Fix ExternalDocumentRef checksum parsing to accept all algorithms#900
Open
Atishyy27 wants to merge 1 commit into
Open
Fix ExternalDocumentRef checksum parsing to accept all algorithms#900Atishyy27 wants to merge 1 commit into
Atishyy27 wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #864
Problem
The tag-value parser cannot parse an
ExternalDocumentRefwhose checksum uses any algorithm other than SHA1:fails with
Couldn't match Checksum, while the identical algorithm is accepted forFileandPackagechecksums.Root cause
ExternalDocumentRefdoesn't go through the lexer'st_CHECKSUMtoken (lexer.py:140), which already recognises every member ofChecksumAlgorithm. Instead it re-implements checksum matching with its own inline regex (parser.py:288):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
ExternalDocumentRefconsistent with howFileandPackagechecksums already behave: length is checked byvalidation/checksum_validator.py, which holds thealgorithm_lengthtable (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: afdednow parses, and is then rejected by the validator rather than by the parser. Because of that, one existing negative test intest_creation_info_parser.pywas updated from an unparseable-checksum case to one that is still unparseable under the new regex:MD7is 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 fromchecksum_validator.algorithm_lengthinstead 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 passesvalidate_checksum.Verified locally on Windows (Python 3.10):
1037 passed, 3 skippedisort,black,flake8clean on both touched files