Skip to content

Commit 08ba71f

Browse files
LoneRifleLoneRifle
committed
Revert "Bugfix: ensure inclusive namespaces written on root node (#163)"
PR #163 modifies the root node, which may be a problem when creating the signature value, which uses a dummy signature wrapper. Revert first, investigate later. This attempts to quick fix #167 and #164 This reverts commit a0de707.
1 parent e0db5f2 commit 08ba71f

File tree

3 files changed

+32
-75
lines changed

3 files changed

+32
-75
lines changed

‎lib/exclusive-canonicalization.js

Lines changed: 11 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -78,19 +78,13 @@ function isPrefixInScope(prefixesInScope, prefix, namespaceURI)
7878
* @return {String}
7979
* @api private
8080
*/
81-
ExclusiveCanonicalization.prototype.renderNs = function(node,
82-
prefixesInScope,
83-
defaultNs,
84-
defaultNsForPrefix,
85-
inclusiveNamespacesPrefixList,
86-
ancestorNamespaces,
87-
topNode
88-
) {
81+
ExclusiveCanonicalization.prototype.renderNs = function(node, prefixesInScope, defaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList) {
8982
var a, i, p, attr
9083
, res = []
9184
, newDefaultNs = defaultNs
9285
, nsListToRender = []
9386
, currNs = node.namespaceURI || "";
87+
9488
//handle the namespaceof the node itself
9589
if (node.prefix) {
9690
if (!isPrefixInScope(prefixesInScope, node.prefix, node.namespaceURI || defaultNsForPrefix[node.prefix])) {
@@ -104,57 +98,23 @@ ExclusiveCanonicalization.prototype.renderNs = function(node,
10498
res.push(' xmlns="', newDefaultNs, '"');
10599
}
106100

107-
if(topNode) {
108-
for(var j = 0; j < ancestorNamespaces.length; j++) {
109-
var ancestorNs = ancestorNamespaces[j];
110-
for(var k = 0; k < inclusiveNamespacesPrefixList.length; k++) {
111-
var inclusiveNs = inclusiveNamespacesPrefixList[k];
112-
if(ancestorNs.prefix === inclusiveNs && !isPrefixInScope(prefixesInScope, ancestorNs.prefix, ancestorNs.namespaceURI)) {
113-
nsListToRender.push({"prefix": ancestorNs.prefix, "namespaceURI": ancestorNs.namespaceURI});
114-
prefixesInScope.push({"prefix": ancestorNs.prefix, "namespaceURI": ancestorNs.namespaceURI});
115-
}
116-
}
117-
}
118-
}
119-
120101
//handle the attributes namespace
121102
if (node.attributes) {
122103
for (i = 0; i < node.attributes.length; ++i) {
123104
attr = node.attributes[i];
124105

125-
126106
//handle all prefixed attributes that are included in the prefix list and where
127107
//the prefix is not defined already
128-
if (attr.prefix &&
129-
!isPrefixInScope(prefixesInScope, attr.localName, attr.value) &&
130-
(inclusiveNamespacesPrefixList.indexOf(attr.localName) >= 0)) {
108+
if (attr.prefix && !isPrefixInScope(prefixesInScope, attr.localName, attr.value) && inclusiveNamespacesPrefixList.indexOf(attr.localName) >= 0) {
131109
nsListToRender.push({"prefix": attr.localName, "namespaceURI": attr.value});
132110
prefixesInScope.push({"prefix": attr.localName, "namespaceURI": attr.value});
133111
}
134112

135113
//handle all prefixed attributes that are not xmlns definitions and where
136114
//the prefix is not defined already
137-
if (attr.prefix && attr.prefix!=="xmlns" && attr.prefix!=="xml") {
138-
var artificiallyIntroduced = false;
139-
if(attr.namespaceURI === undefined) {
140-
//This could mean that the namespace Uri has been reset to "", or it could mean we have artificially
141-
//introduced it because it was in inclusiveNamespacePrefixList
142-
if(inclusiveNamespacesPrefixList.indexOf(attr.prefix) >= 0) {
143-
for(var j = 0; j < ancestorNamespaces.length; j++) {
144-
var ancestorNs = ancestorNamespaces[j];
145-
if(ancestorNs.prefix === attr.prefix) {
146-
artificiallyIntroduced = true;
147-
break;
148-
}
149-
}
150-
}
151-
}
152-
if(!artificiallyIntroduced) {
153-
if(!isPrefixInScope(prefixesInScope, attr.prefix, attr.namespaceURI)) {
154-
nsListToRender.push({"prefix": attr.prefix, "namespaceURI": attr.namespaceURI|| defaultNsForPrefix[attr.prefix]});
155-
prefixesInScope.push({"prefix": attr.prefix, "namespaceURI": attr.namespaceURI|| defaultNsForPrefix[attr.prefix]});
156-
}
157-
}
115+
if (attr.prefix && !isPrefixInScope(prefixesInScope, attr.prefix, attr.namespaceURI) && attr.prefix!="xmlns" && attr.prefix!="xml") {
116+
nsListToRender.push({"prefix": attr.prefix, "namespaceURI": attr.namespaceURI});
117+
prefixesInScope.push({"prefix": attr.prefix, "namespaceURI": attr.namespaceURI});
158118
}
159119
}
160120
}
@@ -172,23 +132,18 @@ ExclusiveCanonicalization.prototype.renderNs = function(node,
172132
return {"rendered": res.join(""), "newDefaultNs": newDefaultNs};
173133
};
174134

175-
ExclusiveCanonicalization.prototype.processInner = function(node,
176-
prefixesInScope,
177-
defaultNs,
178-
defaultNsForPrefix,
179-
inclusiveNamespacesPrefixList,
180-
ancestorNamespaces,
181-
topNode) {
135+
ExclusiveCanonicalization.prototype.processInner = function(node, prefixesInScope, defaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList) {
136+
182137
if (node.nodeType === 8) { return this.renderComment(node); }
183138
if (node.data) { return utils.encodeSpecialCharactersInText(node.data); }
184139

185140
var i, pfxCopy
186-
, ns = this.renderNs(node, prefixesInScope, defaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList, ancestorNamespaces, topNode)
141+
, ns = this.renderNs(node, prefixesInScope, defaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList)
187142
, res = ["<", node.tagName, ns.rendered, this.renderAttrs(node, ns.newDefaultNs), ">"];
188143

189144
for (i = 0; i < node.childNodes.length; ++i) {
190145
pfxCopy = prefixesInScope.slice(0);
191-
res.push(this.processInner(node.childNodes[i], pfxCopy, ns.newDefaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList, ancestorNamespaces, false));
146+
res.push(this.processInner(node.childNodes[i], pfxCopy, ns.newDefaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList));
192147
}
193148

194149
res.push("</", node.tagName, ">");
@@ -242,10 +197,9 @@ ExclusiveCanonicalization.prototype.process = function(node, options) {
242197
var inclusiveNamespacesPrefixList = options.inclusiveNamespacesPrefixList || [];
243198
var defaultNs = options.defaultNs || "";
244199
var defaultNsForPrefix = options.defaultNsForPrefix || {};
245-
var ancestorNamespaces = options.ancestorNamespaces || [];
246200
if (!(inclusiveNamespacesPrefixList instanceof Array)) { inclusiveNamespacesPrefixList = inclusiveNamespacesPrefixList.split(' '); }
247201

248-
var res = this.processInner(node, [], defaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList, ancestorNamespaces, true);
202+
var res = this.processInner(node, [], defaultNs, defaultNsForPrefix, inclusiveNamespacesPrefixList);
249203
return res;
250204
};
251205

‎lib/signed-xml.js

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,9 +373,9 @@ SignedXml.prototype.validateSignatureValue = function(doc) {
373373
throw new Error("When canonicalization method is non-exclusive, whole xml dom must be provided as an argument");
374374
}
375375

376+
ancestorNamespaces = findAncestorNs(doc, "//*[local-name()='SignedInfo']");
376377
}
377-
ancestorNamespaces = findAncestorNs(doc, "//*[local-name()='SignedInfo']");
378-
378+
379379
var c14nOptions = {
380380
ancestorNamespaces: ancestorNamespaces
381381
};
@@ -449,12 +449,25 @@ SignedXml.prototype.validateReferences = function(doc) {
449449
}
450450

451451
/**
452-
* Search for ancestor namespaces before validating references. Ancestor namespaces are needed
453-
* even for exclusive canonicalization because they may be needed for namespaces that are on the
454-
* inclusive namespace prefix list.
452+
* When canonicalization algorithm is non-exclusive, search for ancestor namespaces
453+
* before validating references.
455454
*/
456455
if(Array.isArray(ref.transforms)){
457-
ref.ancestorNamespaces = findAncestorNs(doc, elemXpath);
456+
var hasNonExcC14nTransform = false;
457+
for(var t in ref.transforms){
458+
if(!ref.transforms.hasOwnProperty(t)) continue;
459+
460+
if(ref.transforms[t] === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"
461+
|| ref.transforms[t] === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments")
462+
{
463+
hasNonExcC14nTransform = true;
464+
break;
465+
}
466+
}
467+
468+
if(hasNonExcC14nTransform){
469+
ref.ancestorNamespaces = findAncestorNs(doc, elemXpath);
470+
}
458471
}
459472

460473
var c14nOptions = {

‎test/canonicalization-unit-tests.js

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@ var ExclusiveCanonicalization = require("../lib/exclusive-canonicalization").Exc
44
, SignedXml = require('../lib/signed-xml.js').SignedXml
55

66

7-
var compare = function(test, xml, xpath, expected, inclusiveNamespacesPrefixList, defaultNsForPrefix, ancestorNamespaces) {
7+
var compare = function(test, xml, xpath, expected, inclusiveNamespacesPrefixList, defaultNsForPrefix) {
88
test.expect(1)
99
var doc = new Dom().parseFromString(xml)
1010
var elem = select(xpath, doc)[0]
1111
var can = new ExclusiveCanonicalization()
1212
var result = can.process(elem, {
1313
inclusiveNamespacesPrefixList: inclusiveNamespacesPrefixList,
14-
defaultNsForPrefix: defaultNsForPrefix,
15-
ancestorNamespaces: ancestorNamespaces
14+
defaultNsForPrefix: defaultNsForPrefix
1615
}).toString()
1716

1817
test.equal(expected, result)
@@ -443,14 +442,5 @@ module.exports = {
443442
'//*',
444443
'<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:InclusiveNamespaces xmlns:ds="http://www.w3.org/2001/10/xml-exc-c14n#"></ds:InclusiveNamespaces></ds:Signature>',
445444
'ds')
446-
},
447-
448-
"An ancestor namespace, which is included in the inclusiveNamespacePrefixList should be added to the top level element": function (test) {
449-
compare(test, '<root><child inclusive:attr="value"></child></root>',
450-
'//*',
451-
'<root xmlns:inclusive="urn:inclusive"><child inclusive:attr="value"></child></root>',
452-
"inclusive",
453-
{},
454-
[{"prefix": "inclusive", "namespaceURI": "urn:inclusive"}]);
455445
}
456446
}

0 commit comments

Comments
 (0)