diff --git a/src/c14n-canonicalization.ts b/src/c14n-canonicalization.ts index dd9d7788..a77bc219 100644 --- a/src/c14n-canonicalization.ts +++ b/src/c14n-canonicalization.ts @@ -96,18 +96,36 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg const nsListToRender: { prefix: string; namespaceURI: string }[] = []; const currNs = node.namespaceURI || ""; - //handle the namespace of the node itself - if (node.prefix) { - if (prefixesInScope.indexOf(node.prefix) === -1) { - nsListToRender.push({ - prefix: node.prefix, - namespaceURI: node.namespaceURI || defaultNsForPrefix[node.prefix], - }); - prefixesInScope.push(node.prefix); + if (node.prefix && prefixesInScope.indexOf(node.prefix) === -1) { + nsListToRender.push({ + prefix: node.prefix, + namespaceURI: node.namespaceURI || defaultNsForPrefix[node.prefix], + }); + prefixesInScope.push(node.prefix); + } + + // The default namespace is independent of a prefixed element's namespaceURI. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + let localDefaultNs: string | null = null; + if (node.attributes) { + for (i = 0; i < node.attributes.length; ++i) { + if (node.attributes[i].name === "xmlns") { + localDefaultNs = node.attributes[i].value; + } } - } else if (defaultNs !== currNs) { - //new default ns - newDefaultNs = node.namespaceURI || ""; + } + + let nodeDefaultNs: string; + if (localDefaultNs !== null) { + nodeDefaultNs = localDefaultNs; + } else if (node.prefix) { + nodeDefaultNs = defaultNs; + } else { + nodeDefaultNs = currNs; + } + + if (nodeDefaultNs !== defaultNs) { + newDefaultNs = nodeDefaultNs; res.push(' xmlns="', newDefaultNs, '"'); } @@ -152,6 +170,9 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg if (!alreadyListed) { nsListToRender.push(ancestorNamespace); + if (!ancestorNamespace.prefix) { + newDefaultNs = ancestorNamespace.namespaceURI; + } } } } diff --git a/src/utils.ts b/src/utils.ts index 466b252e..dc2dbf46 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -221,15 +221,19 @@ function collectAncestorNamespaces( return collectAncestorNamespaces(parent, nsArray); } -function findNSPrefix(subset) { +function findSubsetNSPrefixes(subset: Element): Set { + const prefixes = new Set(); const subsetAttributes = subset.attributes; for (let k = 0; k < subsetAttributes.length; k++) { const nodeName = subsetAttributes[k].nodeName; - if (nodeName.search(/^xmlns:?/) !== -1) { - return nodeName.replace(/^xmlns:?/, ""); + if (nodeName === "xmlns" || nodeName.startsWith("xmlns:")) { + prefixes.add(nodeName.replace(/^xmlns:?/, "")); } } - return subset.prefix || ""; + // C14N already renders the element's own namespace; hoisting it would duplicate the declaration. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + prefixes.add(subset.prefix || ""); + return prefixes; } function isElementSubset(docSubset: Node[]): docSubset is Element[] { @@ -264,28 +268,19 @@ export function findAncestorNs( throw new Error("Document subset must be list of elements"); } - // Remove duplicate on ancestor namespace const ancestorNs = collectAncestorNamespaces(docSubset[0]); const ancestorNsWithoutDuplicate: NamespacePrefix[] = []; - for (let i = 0; i < ancestorNs.length; i++) { - let notOnTheList = true; - for (const v in ancestorNsWithoutDuplicate) { - if (ancestorNsWithoutDuplicate[v].prefix === ancestorNs[i].prefix) { - notOnTheList = false; - break; - } - } - - if (notOnTheList) { - ancestorNsWithoutDuplicate.push(ancestorNs[i]); + for (const ns of ancestorNs) { + const isDuplicate = ancestorNsWithoutDuplicate.some((seen) => seen.prefix === ns.prefix); + if (!isDuplicate) { + ancestorNsWithoutDuplicate.push(ns); } } - // Remove namespaces which are already declared in the subset with the same prefix const returningNs: NamespacePrefix[] = []; - const subsetNsPrefix = findNSPrefix(docSubset[0]); + const subsetNsPrefixes = findSubsetNSPrefixes(docSubset[0]); for (const ancestorNs of ancestorNsWithoutDuplicate) { - if (ancestorNs.prefix !== subsetNsPrefix) { + if (!subsetNsPrefixes.has(ancestorNs.prefix)) { returningNs.push(ancestorNs); } } diff --git a/test/c14n-non-exclusive-unit-tests.spec.ts b/test/c14n-non-exclusive-unit-tests.spec.ts index ee7f2ba4..a5d308ba 100644 --- a/test/c14n-non-exclusive-unit-tests.spec.ts +++ b/test/c14n-non-exclusive-unit-tests.spec.ts @@ -1,15 +1,22 @@ import { expect } from "chai"; -import { C14nCanonicalization } from "../src/c14n-canonicalization"; +import { + C14nCanonicalization, + C14nCanonicalizationWithComments, +} from "../src/c14n-canonicalization"; import * as xmldom from "@xmldom/xmldom"; import * as xpath from "xpath"; import * as utils from "../src/utils"; import * as isDomNode from "@xmldom/is-dom-node"; -const test_C14nCanonicalization = function (xml, xpathArg, expected) { +const test_C14nCanonicalization = function ( + xml: string, + xpathArg: string, + expected: string, + can = new C14nCanonicalization(), +) { const doc = new xmldom.DOMParser().parseFromString(xml); const node = xpath.select1(xpathArg, doc); - const can = new C14nCanonicalization(); isDomNode.assertIsNodeLike(node); const result = can @@ -117,6 +124,37 @@ describe("C14N non-exclusive canonicalization tests", function () { test_findAncestorNs(xml, xpath, expected); }); + it("findAncestorNs: Should not hoist default namespace when subset also declares a prefixed namespace", function () { + // The element's own default namespace must be rendered only once. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + const xml = ''; + const xpath = "//*[local-name()='child2']"; + const expected = []; + + test_findAncestorNs(xml, xpath, expected); + }); + + it("findAncestorNs: Should hoist non-default ancestor namespaces when subset is in default namespace", function () { + // Inclusive C14N retains ancestor bindings even when the subset does not visibly use them. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#DataModel + const xml = + ''; + const xpath = "//*[local-name()='child2']"; + const expected = [{ prefix: "aaa", namespaceURI: "urn:aaa" }]; + + test_findAncestorNs(xml, xpath, expected); + }); + + it("findAncestorNs: Should not suppress ancestor namespace for non-namespace attribute starting with 'xmlns'", function () { + // Only xmlns and xmlns:* declare namespaces; xmlnsfoo must not hide an inherited binding. + // https://www.w3.org/TR/REC-xml-names/#ns-decl + const xml = ''; + const xpath = "//*[local-name()='child2']"; + const expected = [{ prefix: "foo", namespaceURI: "urn:foo" }]; + + test_findAncestorNs(xml, xpath, expected); + }); + // Tests for c14nCanonicalization it("C14n: Correctly picks up root ancestor namespace", function () { const xml = ""; @@ -210,4 +248,97 @@ describe("C14N non-exclusive canonicalization tests", function () { test_C14nCanonicalization(xml, xpath, expected); }); + + for (const Canonicalization of [C14nCanonicalization, C14nCanonicalizationWithComments]) { + describe(`${Canonicalization.name}: subset namespace declarations`, function () { + it("does not duplicate the default namespace when the subset declares a prefixed namespace", function () { + // Render the inherited default namespace exactly once on the subset root. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + test_C14nCanonicalization( + '', + "//*[local-name()='child2']", + '', + new Canonicalization(), + ); + }); + + it("does not duplicate the default namespace for the reported #538 document", function () { + // Literal reproduction from https://github.com/node-saml/xml-crypto/issues/538: + // a subset root inheriting a default namespace while declaring a prefixed one, + // with a prefixed child that must resolve against the local declaration. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + test_C14nCanonicalization( + '' + + '' + + "x", + "//*[local-name()='Body']", + '' + + "x", + new Canonicalization(), + ); + }); + + it("retains unused ancestor prefixes alongside the default and local namespaces", function () { + // Inclusive C14N preserves every in-scope namespace, including unused ancestor bindings. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#DataModel + test_C14nCanonicalization( + '', + "//*[local-name()='child2']", + '', + new Canonicalization(), + ); + }); + + for (const declarations of [ + 'xmlns:a="urn:local-a" xmlns:b="urn:local-b"', + 'xmlns:b="urn:local-b" xmlns:a="urn:local-a"', + ]) { + it(`uses both local prefix bindings with ${declarations}`, function () { + // Local declarations override ancestor bindings regardless of declaration order. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#DataModel + test_C14nCanonicalization( + '' + + ``, + "//*[local-name()='child2']", + '', + new Canonicalization(), + ); + }); + } + + it("renders the default namespace on a prefixed apex that redeclares it", function () { + // Every namespace in scope at the apex is rendered there, including the default one. + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + test_C14nCanonicalization( + '', + "//*[local-name()='child2']", + '', + new Canonicalization(), + ); + }); + + it("omits a descendant default namespace already hoisted onto the subset root", function () { + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + test_C14nCanonicalization( + '' + + '', + "//*[local-name()='target']", + '', + new Canonicalization(), + ); + }); + + it("does not restore a default namespace explicitly cleared on a prefixed root", function () { + // An empty default declaration removes the inherited binding; no reset is needed at the apex. + // https://www.w3.org/TR/REC-xml-names/#defaulting + // https://www.w3.org/TR/2001/REC-xml-c14n-20010315#ProcessingModel + test_C14nCanonicalization( + '', + "//*[local-name()='child2']", + '', + new Canonicalization(), + ); + }); + }); + } }); diff --git a/test/canonicalization-unit-tests.spec.ts b/test/canonicalization-unit-tests.spec.ts index 4bce8a6f..227ae529 100644 --- a/test/canonicalization-unit-tests.spec.ts +++ b/test/canonicalization-unit-tests.spec.ts @@ -1,9 +1,12 @@ import { expect } from "chai"; -import { ExclusiveCanonicalization } from "../src/exclusive-canonicalization"; +import { + ExclusiveCanonicalization, + ExclusiveCanonicalizationWithComments, +} from "../src/exclusive-canonicalization"; import * as xmldom from "@xmldom/xmldom"; import * as xpath from "xpath"; -import { SignedXml } from "../src/index"; +import { findAncestorNs, SignedXml } from "../src/index"; import * as isDomNode from "@xmldom/is-dom-node"; const compare = function ( @@ -28,6 +31,50 @@ const compare = function ( }; describe("Canonicalization unit tests", function () { + for (const Canonicalization of [ + ExclusiveCanonicalization, + ExclusiveCanonicalizationWithComments, + ]) { + describe(`${Canonicalization.name}: configured inclusive namespaces`, function () { + const xml = + '' + + ''; + const selector = "//*[local-name()='target']"; + + it("retains local and inherited bindings requested by the caller", function () { + // PrefixList includes QName-value bindings without replacing local declarations with ancestors. + // https://www.w3.org/TR/xml-exc-c14n/#sec-Specification + const doc = new xmldom.DOMParser().parseFromString(xml); + const node = xpath.select1(selector, doc); + isDomNode.assertIsElementNode(node); + + const result = new Canonicalization().process(node, { + ancestorNamespaces: findAncestorNs(doc, selector), + inclusiveNamespacesPrefixList: ["b", "c"], + }); + + expect(result).to.equal( + '', + ); + }); + + it("omits non-visible bindings when the caller supplies an empty PrefixList", function () { + // Prefixes used only in attribute values are not visibly utilized in exclusive C14N. + // https://www.w3.org/TR/xml-exc-c14n/#def-visibly-utilizes + const doc = new xmldom.DOMParser().parseFromString(xml); + const node = xpath.select1(selector, doc); + isDomNode.assertIsElementNode(node); + + const result = new Canonicalization().process(node, { + ancestorNamespaces: findAncestorNs(doc, selector), + inclusiveNamespacesPrefixList: [], + }); + + expect(result).to.equal(''); + }); + }); + } + it("Exclusive canonicalization works on xml with no namespaces", function () { compare("123", "//*", "123"); });