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

fix: escapeTag correctly works with booleans #543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DamianCSpendesk
Copy link

In relation to #538, the issue boils down to the code in escapeTag, where we call

xmlEscape(text || "")

This fails for "falsey" values, in our case for AllowCreate being false. Even passing true, we end up with an error as the xml-escape lib used actually expects a string.

This PR tests for undefined and null, where it'll pass an empty string. For all other values, it will convert to a string. As a result, our AllowCreate gets correctly set

...
<samlp:NameIDPolicy Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" AllowCreate="false"/></samlp:AuthnRequest>

@DamianCSpendesk
Copy link
Author

@tngan Sorry for the ping - for whatever reason, I can't add you as a reviewer

@adrai
Copy link

adrai commented Aug 7, 2024

@tngan can this be merged?

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.

2 participants