Skip to content
New issue

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

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

Already on GitHub? # to your account

Enhance derToPem to support XML pretty-print #439

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

cjbarth
Copy link
Contributor

@cjbarth cjbarth commented Jan 8, 2024

When pulling this library to node-saml, it was discovered that there was a missing case in derToPem. This case is not being handled correctly and a test has been added for it.

Copy link

codecov bot commented Jan 8, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7410d2e) 73.17% compared to head (e2656c7) 73.17%.
Report is 2 commits behind head on master.

Files Patch % Lines
src/signed-xml.ts 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #439   +/-   ##
=======================================
  Coverage   73.17%   73.17%           
=======================================
  Files           9        9           
  Lines         902      902           
  Branches      239      239           
=======================================
  Hits          660      660           
  Misses        143      143           
  Partials       99       99           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cjbarth
Copy link
Contributor Author

cjbarth commented Jan 8, 2024

@LoneRifle , this should be a simple change to review. It simply removes spaces from a pretty-printed XML payload so that the cert can be properly normalized.

Copy link
Collaborator

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

lgtm, good catch!

@cjbarth cjbarth merged commit 6e95c60 into node-saml:master Jan 9, 2024
8 of 9 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants