Skip to content

Fix ExternalDocumentRef checksum parsing to accept all algorithms#900

Open
Atishyy27 wants to merge 1 commit into
spdx:mainfrom
Atishyy27:fix/864-external-doc-ref-checksum-algorithms
Open

Fix ExternalDocumentRef checksum parsing to accept all algorithms#900
Atishyy27 wants to merge 1 commit into
spdx:mainfrom
Atishyy27:fix/864-external-doc-ref-checksum-algorithms

Conversation

@Atishyy27

Copy link
Copy Markdown

Fixes #864

Problem

The tag-value parser cannot parse an ExternalDocumentRef whose checksum uses any algorithm other than SHA1:

ExternalDocumentRef: DocumentRef-x https://example.com SHA256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

fails with Couldn't match Checksum, while the identical algorithm is accepted for File and Package checksums.

Root cause

ExternalDocumentRef doesn't go through the lexer's t_CHECKSUM token (lexer.py:140), which already recognises every member of ChecksumAlgorithm. Instead it re-implements checksum matching with its own inline regex (parser.py:288):

external_doc_ref_regex = re.compile(r"(.*)(\s*SHA1:\s*[a-f0-9]{40})")

That hardcodes both the algorithm (SHA1) and the digest length ({40}).

Fix

Widen the regex to the same algorithm set the lexer already accepts.

Behaviour change worth calling out

The new regex matches [a-f0-9]* rather than [a-f0-9]{40}, so digest-length enforcement moves from parse time to validation time.

This is deliberate, and it makes ExternalDocumentRef consistent with how File and Package checksums already behave: length is checked by validation/checksum_validator.py, which holds the algorithm_length table (SHA1 -> 40, SHA256 -> 64, …) and applies ^[0-9a-f]{N}$. Keeping a length in the parser regex would have meant duplicating that table in a second place, and would still have been wrong for the 16 algorithms it didn't cover.

Concretely: SHA1: afded now parses, and is then rejected by the validator rather than by the parser. Because of that, one existing negative test in test_creation_info_parser.py was updated from an unparseable-checksum case to one that is still unparseable under the new regex:

- "ExternalDocumentRef: Document_ref document_uri SHA1: afded"
+ "ExternalDocumentRef: Document_ref document_uri MD7: afded"

MD7 is not a real algorithm, so the case still exercises "parser rejects a malformed checksum". If reviewers would rather keep parse-time length checking, I'm happy to rework this — it just implies deriving the lengths from checksum_validator.algorithm_length instead of hardcoding them.

Tests

Added test_parse_external_document_ref_with_sha256_checksum, using the SHA256 of the empty string (64 hex chars) so the parsed document also passes validate_checksum.

Verified locally on Windows (Python 3.10):

  • new test fails on the parent commit, passes with the fix
  • full suite: 1037 passed, 3 skipped
  • isort, black, flake8 clean on both touched files

The tag-value parser's regex for ExternalDocumentRef checksums only
matched SHA1 with a fixed 40-hex-character length, so documents using
any other algorithm from ChecksumAlgorithm (e.g. SHA256, as produced
by some third-party generators) failed to parse with 'Couldn't match
Checksum'.

Extend the regex to accept the same set of algorithms already
supported for File/Package checksums via the CHECKSUM lexer token.

Fixes spdx#864

Signed-off-by: Atishyy27 <142108881+Atishyy27@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SPDX 2.3 tagvalue parser cannot parse documents with erroneous SHA256 ExternalDocumentRef checksums

1 participant