bump xmldom to 0.9 #501
bump xmldom to 0.9 #501
Conversation
| const signatureNode = doc.documentElement; | ||
|
|
||
| if (!signatureNode) { | ||
| throw new Error("signatureNode is not defined"); |
There was a problem hiding this comment.
Out of curiosity: why xmldom version bump required this change to signature-unit-tests.spec.ts?
There was a problem hiding this comment.
Linting/eslint complaining that documentElement can now be null. So either a bunch of ignore statements or assert it actually exists. Tried doing it with expect but wasn't enough to make linter happy.
| @@ -54,7 +54,7 @@ describe("Canonicalization unit tests", function () { | |||
|
|
|||
| it("Exclusive canonicalization works with default namespace for prefix", function () { | |||
| compare( | |||
| '<ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:SignedInfo>', | |||
| '<ds:SignedInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:SignedInfo>', | |||
There was a problem hiding this comment.
Why version bump required addition of xmlns definition to input xml?
I guess that new xmldom version refuses to understand that particular input xml anymore when it is parsed at the beginning of compare function...but is this test case ("Exclusive canonicalization works with default namespace for prefix") still testing what it claims to to test. Line 61 configures
{ ds: "http://www.w3.org/2000/09/xmldsig#" }as parameter for compare functions defaultNsForPrefix which is passed as parameter for canonicalization process. I did not spend time to try to understand how canonicalization function should work but parameter name implicates that canonixalization function was previously instructed to use aforementioned NS for prefix ds (if/when it was missing from input xml).
Disclaimer: I am not xml dsig or canonicalization expert. This change just popped up from the mass when I quickly scrolled through changes out of curiosity.
There was a problem hiding this comment.
Line 61 configures
as parameter for compare functions defaultNsForPrefix which is passed as parameter for canonicalization process.
These happen after parsing the DOM, so they are unrelated/ignored by xmldom.
It considers it invalid payload since it was never defined. The excerpt itself
<ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:SignedInfo> is invalid in any XML validator I punch it into. (I'm def no expert on XML either 😅 ).
Seems xmldom is becoming more strict overall.
There was a problem hiding this comment.
These happen after parsing the DOM, so they are unrelated/ignored by xmldom
Yes...canonicalzation's process implementation is tested at compare after inputxml is parsed with xmldom and after SignedInfo is extracted from DOM produced by xmldom and yes xmldom is more stricter now, but:
Under what conditions canonicalization was invoked with Element which did/might not have namespace uri available / defined for prefix (in this case for ds). Dunno why such test case and functionality (defaultNsForPrefix') would exists without some reason. Should such functionality be preserved with tests that actually test it(?)
Unfortunately there isn't much info available at these related commits:
|
There is a lot of work done here, but ultimately, [email protected] isn't ready for this project. I've already been through this. See xmldom/xmldom#812 (comment) |
Fixes #108 and as an alternative to: #492