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

Add anywhere flag to _find methods. #33

Merged
merged 2 commits into from
Nov 8, 2015
Merged

Add anywhere flag to _find methods. #33

merged 2 commits into from
Nov 8, 2015

Conversation

jmindek
Copy link

@jmindek jmindek commented Nov 2, 2015

Jerry Mindek added 2 commits November 2, 2015 14:43
 - Address Issue #32 Enveloped signature: Signature-placeholder not found when not
   direct child of root
 - Set anywhere=True when searching for Signature.
 - Replace ':' with '.' in XPATH query to find Signature tag.
@kislyuk
Copy link
Member

kislyuk commented Nov 4, 2015

Can you add a test case that exercises this functionality?

@kislyuk
Copy link
Member

kislyuk commented Nov 4, 2015

This expansion highlights the need to better guide users of this package to use best practices (make sure they pass a specific X.509 cert that they expect the signature to use, or another way to ascertain that the signature is authentic; and only trust data returned by verify()).

Since that concern is not really exacerbated by this change, and you seem to be in a rush, I can merge and release this now as long as you promise to add a test case in a later PR.

@jmindek
Copy link
Author

jmindek commented Nov 4, 2015

I respect your desire to have a test case before merging and will add one
soon.

On Wed, Nov 4, 2015 at 10:17 AM Andrey Kislyuk notifications@github.com
wrote:

This expansion highlights the need to better guide users of this package
to use best practices (make sure they pass a specific X.509 cert that they
expect the signature to use, or another way to ascertain that the signature
is authentic; and only trust data returned by verify()).

Since that concern is not really exacerbated by this change, and you seem
to be in a rush, I can merge and release this now as long as you promise to
add a test case in a later PR.


Reply to this email directly or view it on GitHub
#33 (comment).

@kislyuk
Copy link
Member

kislyuk commented Nov 8, 2015

Merging now, will add a test case and release later today.

kislyuk added a commit that referenced this pull request Nov 8, 2015
Add anywhere flag to _find methods.
@kislyuk kislyuk merged commit de1250e into XML-Security:master Nov 8, 2015
@jmindek
Copy link
Author

jmindek commented Nov 8, 2015

Thanks Andrey.
I worked on adding a new method and xml file to test but just haven't had
the time to finish it up.

-Jerry

On Sun, Nov 8, 2015 at 9:13 AM Andrey Kislyuk notifications@github.com
wrote:

Merged #33 #33.


Reply to this email directly or view it on GitHub
#33 (comment).

@kislyuk
Copy link
Member

kislyuk commented Nov 9, 2015

Released in v0.4.5. Please test.

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.

None yet

2 participants