Truncate fractional seconds with a warning

[zbleness]

The dateTimeStamp XML standard type supports fractional second notation: https://www.w3.org/TR/xmlschema11-2/#dateTimeStamp

Before this patch, the tool crashed when fractional timestamps were given in e.g. CreationInfo.
Lets truncate the fractional second part with a warning instead of crashing.

[zbleness]

PR Checks pass, please review.

[armintaenzertng]

Thanks for your contribution, @zbleness!

The SPDX spec is quite explicit with the format of the timestamp and defines YYYY-MM-DDThh:mm:ssZ as the format.
I'd read that such that no fractional seconds are allowed.

[zbleness]

Hi @armintaenzertng !

Afaik. the ISO8601 spec support fractional seconds too.
It is true that the SPDX HTML spec clearly defines a subset of the ISO8601 format for SPDX.
The problem is that it is not reflected in the JSON schemas which makes it hard to get consistent SBOMs from our vendors.

Would you be ok if we fix the schemas (all versions) instead for consistency?

Example (v2.2.1):

• https://github.com/spdx/spdx-spec/blob/development/v2.2.1/schemas/spdx-schema.json#L124
• https://github.com/spdx/spdx-spec/blob/development/v2.2.1/schemas/spdx-schema.json#L193
• https://github.com/spdx/spdx-spec/blob/development/v2.2.1/schemas/spdx-schema.json#L250
• https://github.com/spdx/spdx-spec/blob/development/v2.2.1/schemas/spdx-schema.json#L457
• https://github.com/spdx/spdx-spec/blob/development/v2.2.1/schemas/spdx-schema.json#L661

[armintaenzertng]

Changing the spec is not for me to decide. @goneall, what's your stance on this? Should we allow fractional seconds?

[goneall]

Changing the spec is not for me to decide. @goneall, what's your stance on this? Should we allow fractional seconds?

Changing the spec would involve creating an issue in the spdx-model repo and updating quite a few libraries.

Would you be ok if we fix the schemas (all versions) instead for consistency?

If the proposal is to update the schemas to reflect the SPDX subset of the XML datetimestamp, that seems like a reasonable approach.

[zbleness]

Changing the spec would involve creating an issue in the spdx-model repo and updating quite a few libraries.

Based on the proposed regex in this issue you seem to be adopting fractional seconds format (more ISO8601 compatibility). Do you really want a new issue that would propose quite the opposite (maintaining SPDX 2.3 timestamp => less ISO8601 compatibility)?

If the proposal is to update the schemas to reflect the SPDX subset of the XML datetimestamp, that seems like a reasonable approach.

To be clear, I would incorporate the format restriction from the HTML specs: YYYY-MM-DDThh:mm:ssZ and add some validation.

[goneall]

Based on the proposed regex in spdx/spdx-3-model#865 issue you seem to be adopting fractional seconds format (more ISO8601 compatibility). Do you really want a new issue that would propose quite the opposite (maintaining SPDX 2.3 timestamp => less ISO8601 compatibility)?

Good catch - I totally forgot about this prior suggestion - no need to introduce another issue.

The proposal will likely only impact version 3.1 or later on the spec, so updating the schemas still makes sense for the prior versions.

After thinking about this a bit more, creating a pull request directly in the SPDX spec repo for the 2.X versions would make more sense than creating an issue in the model repo.

For 3.1, we should decide in our weekly tech call if we're going to support the more granular timestamps.

@bact @zvr - any opinions on this?

[zvr]

My strong preference would be to retain the YYYY-MM-DDThh:mm:ssZ format that was defined and used in SPDXv2.

I see little value in having more granularity.

For SPDXv2, we should update unqualified references to "ISO 8601" (in the schemas).

For SPDXv3, we can improve the DateTime dataclass format restriction, as per spdx/spdx-3-model#865.

[zvr]

On this specific patch, the initial description says:

The dateTimeStamp XML standard type supports fractional second notation: https://www.w3.org/TR/xmlschema11-2/#dateTimeStamp
Before this patch, the tool crashed when fractional timestamps were given in e.g. CreationInfo.

This is correct. The specification does not allow arbitrary xsd:dateTimeStamp values, but only of the specific format that is defined in the DateTime class.

Unfortunately, it seems that Datatypes are not (yet) visible in the generated JSON-LD schema.

[bact]

Agree to improve the datetime format in SPDX 3.x.

For SPDX 2.x spec, I tend to think we should not touch it, unless we are quite certain that once we update the SPDX 2.x spec, tools maintainers of major programming languages will update the tools accordingly. If we can make sure that there will be no huge gap in datetime compatibilities between tools, it may be ok.

A middle ground is probably the approach this PR takes: being strict in output but flexible in input. The spec should be strict (no microsecond allowed), but it can also suggest that implementations CAN truncated it WITH warnings and NO microsecond will be recorded in the SBOM. -- This can be written in the spec, along with the update of the description in the schema to say "ISO 8601 but ...".

--

On the implementation side, as this PR increases robustness of the library, I see the value of it. But until the spec got update, it should not truncate microsecond silently. Warning should be raised to clearly communicate that the input is not by the spec. (if needed, this robustness can be toggled).

[zvr]

I think this PR is wrong, since it adds support for microsecond resolution, while the spec does not allow it.

For SPDXv2, the issue (as I understand it) is that, the spec only allows our restricted format (e.g. clearly stated in the Annotation date definition), but the wording copied into the schema simply mentions "ISO 8601 format". My suggestion was to update the wording in the schema to say "subset of ISO 8601 format" or something.

This is essentially for people who do not read the spec and only follow the schema.

Any tool that accepted or generated fractional seconds in SPDXv2 (or v3) is clearly not following the spec.

[zbleness]

Instead of abandoning the PR all together, I took in bact's proposal for your consideration - it seems like an improvement over just crashing.
Also updated the title and description to match the implementation.

I will follow up by updating SPDX2.x specs in a separate PR as it seems to be the consensus.