Skip to content

fix: Upgrade @xmldom/xmldom to 0.9.x#414

Open
dbetteridge wants to merge 1 commit into
node-saml:masterfrom
dbetteridge:fix/upgrade-xmldom
Open

fix: Upgrade @xmldom/xmldom to 0.9.x#414
dbetteridge wants to merge 1 commit into
node-saml:masterfrom
dbetteridge:fix/upgrade-xmldom

Conversation

@dbetteridge

@dbetteridge dbetteridge commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • Upgrades @xmldom/xmldom from ^0.8.10 to ^0.9.10 to address flagged CVE's and bugs https://security.snyk.io/package/npm/%40xmldom%2Fxmldom/0.8.10
  • Adapts parseDomFromString to the new 0.9.x DOMParser API (onError callback replacing errorHandler, xmlns option replacing implicit namespace handling, locator: true replacing locator: {})
  • Updates test expectations to match minor error message wording changes in xmldom 0.9 (HierarchyRequestError vs Hierarchy request error)
  • Fixes a test that relied on undeclared namespace prefixes now that xmldom 0.9 is stricter about namespace resolution

Test plan

  • npm test passes with the updated dependency
  • Verify no regressions in SAML response parsing with real-world payloads
  • Confirm xml-crypto and xml-encryption continue to work correctly (they pin their own xmldom 0.8.x internally)

Summary by CodeRabbit

  • Bug Fixes

    • Improved XML parser error reporting with detailed line and column information for easier troubleshooting.
    • Fixed XML namespace handling for SAML assertions to ensure attributes are correctly interpreted and validated.

  • Chores

    • Updated XML processing dependency to version 0.9.10 for enhanced compatibility.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Upgrades @xmldom/xmldom from ^0.8.10 to ^0.9.10. Adapts parseDomFromString in src/xml.ts to the new DOMParser API using locator: true, an onError callback, and a samlNamespaces map. Adds a formatXmlError helper. Updates two test files to match changed error message text and SAML namespace prefix behavior.

Changes

xmldom 0.9 upgrade and adaptation

Layer / File(s) Summary
Dependency bump and XML helper infrastructure
package.json, src/xml.ts		 Bumps @xmldom/xmldom to ^0.9.10, adds samlNamespaces URI map for the new xmlns parser option, and adds formatXmlError to normalize parse error objects with optional line/column location.
parseDomFromString rewritten for xmldom 0.9 API
src/xml.ts		 Replaces the old errorHandler with locator: true and an onError callback that captures the first error/fatalError. Wraps parsing in try/catch; rejects with formatXmlError output for recorded parse errors or an "element parse error" message for thrown exceptions. Removes the prior documentElement presence check.
Test fixture and error message updates
test/test-signatures.spec.ts, test/tests.spec.ts		 Updates XMLDOM_ERROR to the new HierarchyRequestError casing from xmldom 0.9. Changes the assertion issuer prefix from saml: to saml2: and adds explicit xmlns:xs/xmlns:xsi declarations to the attribute value element in the conflicting-profile-fields fixture.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hippity-hoppity, xmldom's new!
Namespaces mapped and errors formatted too,
onError now catches what parsers let slip,
saml2: prefix secures every trip.
The bunny upgrades with a confident hop —
version nine-point-ten, and we just won't stop! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: upgrading @xmldom/xmldom to 0.9.x, which is the primary objective of this pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively addresses the template requirements: summarizes changes clearly, explains the use-case (CVE/bug fixes with dependency upgrade), includes tests and test plan, and provides detailed context.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@RGlanville

RGlanville commented Jun 17, 2026

Copy link
Copy Markdown

@ahacker1-securesaml / @cjbarth - Can you review this patch to xmldom?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@JoshTighe-Vault JoshTighe-Vault JoshTighe-Vault approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dbetteridge @RGlanville @JoshTighe-Vault