Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing SignedXml.CheckSignature for enveloped signature with #xpointer(/) Reference #95404

Merged
merged 18 commits into from
Mar 5, 2024

Conversation

m0sa
Copy link
Contributor

@m0sa m0sa commented Nov 29, 2023

Adds tests for #95390 and implements a fix.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 29, 2023
@ghost
Copy link

ghost commented Nov 29, 2023

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Started poking at #95390

Author: m0sa
Assignees: -
Labels:

area-System.Security

Milestone: -

@m0sa m0sa changed the title [WIP] Fixing SignedXml.CheckSignature for enveloped signature with #xpointer(/) Reference Fixing SignedXml.CheckSignature for enveloped signature with #xpointer(/) Reference Nov 29, 2023
@m0sa m0sa marked this pull request as ready for review November 29, 2023 20:50
m0sa and others added 5 commits November 29, 2023 22:45
@m0sa m0sa requested a review from vcsjones December 6, 2023 06:39
@vcsjones vcsjones requested review from krwq and bartonjs December 8, 2023 22:13
@bartonjs bartonjs added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 3, 2024
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 5, 2024
@m0sa m0sa requested a review from bartonjs January 11, 2024 08:35
@m0sa m0sa requested a review from vcsjones February 15, 2024 15:37
Copy link
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

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

I'm slightly displeased at how the null URI got included in this change sort of just-because (i.e. it wasn't part of the original problem description); but there's not really reason to break it apart at this point. (Except that the PR / final commit needs a better title.)

_uri values that are non-empty and don't start with # will now take a different path through the code; but it ends up having the same effect.

@bartonjs bartonjs merged commit b5b290f into dotnet:main Mar 5, 2024
111 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants