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: Add space after xmlns:wsu to prevent xmldom warning #1215

Merged
merged 1 commit into from
May 1, 2024

Conversation

scagood
Copy link
Contributor

@scagood scagood commented Mar 21, 2023

This removes the following xmldom warning:

[xmldom warning]        attribute space is required"xmlns:wsu"!!
  at DOMHandler.warning (<root>/node_modules/@xmldom/xmldom/lib/dom-parser.js:251:29)
  at parseElementStartPart (<root>/node_modules/@xmldom/xmldom/lib/sax.js:398:19)
  at parse (<root>/node_modules/@xmldom/xmldom/lib/sax.js:167:15)
  at XMLReader.parse (<root>/node_modules/@xmldom/xmldom/lib/sax.js:47:3)
  at DOMParser.parseFromString (<root>/node_modules/@xmldom/xmldom/lib/dom-parser.js:96:7)
  at SignedXml.computeSignature (<root>/node_modules/xml-crypto/lib/signed-xml.js:721:23)
  at WSSecurityCert.postProcess (<root>/node_modules/soap/lib/security/WSSecurityCert.js:141:21)
  at Client._invoke (<root>/node_modules/soap/lib/client.js:357:33)
  at <root>/node_modules/soap/lib/client.js:187:18

This was because there was no space between the following attributes in the Security header:

<wsse:Security xmlns:wsu="docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd"soap:mustUnderstand="1">

This is now:

<wsse:Security xmlns:wsu="docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd" soap:mustUnderstand="1">

@scagood
Copy link
Contributor Author

scagood commented Mar 22, 2023

🤔 I am not sure why the tests are failing, they look unrelated to the change I made.

The tests also look to be passing on my local machine 👀

@scagood
Copy link
Contributor Author

scagood commented May 7, 2023

The required fix can be found in #1206 I will rebase once that is merged.

@efreibe
Copy link

efreibe commented Sep 29, 2023

Hello, I am receiving the same error, this merge will resolve the issue.

@w666
Copy link
Collaborator

w666 commented May 1, 2024

I don't see any problem with this change, tried locally and all tests passed.

@w666 w666 merged commit 4972946 into vpulim:master May 1, 2024
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