Skip to content

Backport SHR p-claim percent-encoding hex case handling to dev8x - #3578

Closed
debchoudhury-id4s wants to merge 1 commit into
AzureAD:dev8xfrom
debchoudhury-id4s:debchoudhury/shr-p-claim-percent-encoding-backport
Closed

debchoudhury-id4s wants to merge 1 commit into
AzureAD:dev8xfrom
debchoudhury-id4s:debchoudhury/shr-p-claim-percent-encoding-backport

Conversation

@debchoudhury-id4s

Copy link
Copy Markdown
Contributor

Summary

Backports #3561 to the dev8x branch.

  • treats hexadecimal letters inside valid percent-encoded triplets as case-insensitive during SHR p claim validation
  • preserves the 8.x default for literal path casing through UseCaseSensitivePClaimComparison
  • keeps encoded characters distinct from literal delimiters and preserves p claim creation output
  • includes the corresponding validation and creation tests

Testing

  • Targeted ValidatePClaim and CreatePClaim tests pass on .NET 6, 8, 9, and 10 (66 tests per target)
  • .NET Framework targets are skipped locally because delay-signed assemblies cannot pass strong-name verification

)

* Handle SHR p-claim percent-encoding hex case

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 58d6002)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Backport of SHR p-claim validation behavior to the dev8x line so that percent-encoded triplets compare hex letters case-insensitively (per RFC 3986), while preserving the existing 8.x default behavior for literal path casing via UseCaseSensitivePClaimComparison and keeping p-claim creation output unchanged.

Changes:

  • Updated SignedHttpRequestHandler.ValidatePClaim to avoid allocations via span-based trimming/comparison and added %XX-aware ordinal comparison logic for hex-letter casing.
  • Expanded validation test coverage with a comparison matrix and exception-message casing checks.
  • Added creation tests to ensure percent-encoding hex casing and double-encoding are preserved in output; documented the change in the changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/Microsoft.IdentityModel.Protocols.SignedHttpRequest.Tests/SignedHttpRequestValidationTests.cs Adds targeted tests covering hex-case equivalence in %XX triplets, switch behavior, and exception-message casing.
test/Microsoft.IdentityModel.Protocols.SignedHttpRequest.Tests/SignedHttpRequestCreationTests.cs Adds tests verifying p claim creation preserves percent-encoding casing (lower/upper) and double-encoding.
src/Microsoft.IdentityModel.Protocols.SignedHttpRequest/SignedHttpRequestHandler.cs Implements allocation-free trimming/comparison and %XX-aware equality for ordinal comparisons.
CHANGELOG.md Notes the backported bug fix in the release notes.

Comment thread CHANGELOG.md
@debchoudhury-id4s

Copy link
Copy Markdown
Contributor Author

Superseded by #3579, which uses a branch directly in the AzureAD repository and includes the dev8x conflict resolution.

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.

3 participants