Fix copy-pasted pattern in OCSPCertID hash algorithm normalization - #311
Open
ZayanKhan-12 wants to merge 1 commit into
Open
Fix copy-pasted pattern in OCSPCertID hash algorithm normalization#311ZayanKhan-12 wants to merge 1 commit into
ZayanKhan-12 wants to merge 1 commit into
Conversation
The sha512 normalization case in OCSPCertID's DER parser matched ".sha512, .sha1UsingNil" instead of ".sha512, .sha512UsingNil" - an apparent copy-paste from the sha1 case. The second pattern was dead (already matched by the sha1 case above), and .sha512UsingNil inputs happened to be preserved by the default case, so behavior was accidentally correct; the pattern now states the intent directly. Adds a test pinning the parameter normalization of all four supported hash algorithms. Fixes apple#273. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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
1 participant
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 #273 (@Lukasa invited a PR there and the reporter never followed up, so picking it up).
Motivation
The sha512 case of
OCSPCertID's hash-algorithm normalization switch reads:The
.sha1UsingNilpattern is a copy-paste from the sha1 case — and it's dead code, since.sha1UsingNilis already matched bycase .sha1, .sha1UsingNil:above.To be candid about impact: behavior is accidentally correct today.
.sha512still hits the first pattern and normalizes properly, and.sha512UsingNilfalls through todefault, which returns it unchanged — the same result the intended pattern produces. So this is a correctness-of-intent fix, not a behavior change; the danger was latent (e.g. if thedefaultcase ever changed).Modifications
case .sha512, .sha1UsingNil:→case .sha512, .sha512UsingNil:testCertIDNormalizesHashAlgorithmParameters, pinning the absent-parameters → NULL-parameters normalization (and the already-normalized spellings) for all four supported hash algorithms, so the intended mapping is now under test.Result
The switch states its intent; normalization behavior for every supported algorithm is covered by tests. Full suite: 578 tests pass; changed files clean under
swift format lint.🤖 Generated with Claude Code