From 49855e3dd424f04fecc878f422f313c4d2603572 Mon Sep 17 00:00:00 2001
From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com>
Date: Mon, 21 Sep 2026 22:14:00 -0500
Subject: [PATCH] fix: build the id index only when an element is looked up by
id
Skip id-index work until the first lookup. Angular SSR creates transient
documents while parsing innerHTML and sanitizing untrusted markup, so
eager indexing makes attacker-controlled ids spend time and heap in
documents that are never queried.
Build the index from currently rooted nodes on demand and maintain
it after that. Use Maps for ids and duplicate-id buckets to avoid sparse
storage for numeric keys. Coerce lookup arguments before building and
read only unnamespaced id attributes so early and late lookups agree.
Add regression coverage for lazy construction, mutation, duplicate
ids, document order, numeric keys, coercion, and namespaces.
---
lib/Document.js | 102 +++++++++++++++++-------
test/domino.js | 205 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 277 insertions(+), 30 deletions(-)
diff --git a/lib/Document.js b/lib/Document.js
index 8f9cbe5..23e301b 100644
--- a/lib/Document.js
+++ b/lib/Document.js
@@ -58,7 +58,18 @@ function Document(isHTML, address) {
// We may need to update this mapping every time a node is rooted
// or uprooted, and any time an attribute is added, removed or changed
// on a rooted element.
- this.byId = Object.create(null);
+ //
+ // The mapping is built lazily: it stays null until something actually
+ // looks an element up by id, and only then is it populated from the
+ // nodes currently rooted in this document. After that it is kept up to
+ // date by root()/uproot() and by the id attribute change handler, just as
+ // it always was. Documents that are never queried by id -- in particular
+ // the private documents the fragment parser creates and throws away for
+ // every innerHTML assignment -- no longer pay for an index nobody reads.
+ //
+ // A Map keeps ids such as `1023` out of sparse object-index storage, the
+ // same reason Element uses one for qualified attribute names.
+ this._byId = null;
// This property holds a monotonically increasing value akin to
// a timestamp used to record the last modification time of nodes
@@ -326,7 +337,9 @@ Document.prototype = Object.create(ContainerNode.prototype, {
}},
getElementById: { value: function(id) {
- var n = this.byId[id];
+ // Coercion can mutate the document, so do it before building the index.
+ id = String(id);
+ var n = this._idIndex().get(id);
if (!n) return null;
if (n instanceof MultiId) { // there was more than one element with this id
return n.getFirst();
@@ -336,7 +349,29 @@ Document.prototype = Object.create(ContainerNode.prototype, {
_hasMultipleElementsWithId: { value: function(id) {
// Used internally by querySelectorAll optimization
- return (this.byId[id] instanceof MultiId);
+ return (this._idIndex().get(id) instanceof MultiId);
+ }},
+
+ // The id -> element map, materialized on demand. Everything that reads the
+ // index goes through here; everything that writes it (addId/delId) leaves it
+ // alone while it is absent.
+ _idIndex: { value: function _idIndex() {
+ var byId = this._byId;
+ if (byId !== null) return byId;
+ byId = this._byId = new Map();
+ // Populate from the elements rooted in this document right now. _nodes
+ // is the registry root() and uproot() maintain, so walking it -- rather
+ // than the tree -- gives exactly the entries the index would hold had it
+ // been kept up to date all along, and needs no traversal state.
+ var nodes = this._nodes;
+ for (var nid = 1, end = this._nextnid; nid < end; nid++) {
+ var n = nodes[nid];
+ if (n !== undefined && n.nodeType === Node.ELEMENT_NODE) {
+ var id = n.getAttributeNS(null, 'id');
+ if (id) this.addId(id, n);
+ }
+ }
+ return byId;
}},
// Just copy this method from the Element prototype
@@ -631,16 +666,18 @@ Document.prototype = Object.create(ContainerNode.prototype, {
// Add a mapping from id to n for n.ownerDocument
addId: { value: function addId(id, n) {
- var val = this.byId[id];
+ var byId = this._byId;
+ if (byId === null) return; // no index yet; it will pick n up when built
+ var val = byId.get(id);
if (!val) {
- this.byId[id] = n;
+ byId.set(id, n);
}
else {
// TODO: Add a way to opt-out console warnings
//console.warn('Duplicate element id ' + id);
if (!(val instanceof MultiId)) {
val = new MultiId(val);
- this.byId[id] = val;
+ byId.set(id, val);
}
val.add(n);
}
@@ -648,17 +685,19 @@ Document.prototype = Object.create(ContainerNode.prototype, {
// Delete the mapping from id to n for n.ownerDocument
delId: { value: function delId(id, n) {
- var val = this.byId[id];
+ var byId = this._byId;
+ if (byId === null) return; // no index yet; nothing to remove n from
+ var val = byId.get(id);
utils.assert(val);
if (val instanceof MultiId) {
val.del(n);
if (val.length === 1) { // convert back to a single node
- this.byId[id] = val.downgrade();
+ byId.set(id, val.downgrade());
}
}
else {
- this.byId[id] = undefined;
+ byId.delete(id);
}
}},
@@ -774,8 +813,10 @@ function root(n) {
n.ownerDocument._nodes[n._nid] = n;
// Manage id to element mapping
if (n.nodeType === Node.ELEMENT_NODE) {
- var id = n.getAttribute('id');
- if (id) n.ownerDocument.addId(id, n);
+ if (n.ownerDocument._byId !== null) {
+ var id = n.getAttributeNS(null, 'id');
+ if (id) n.ownerDocument.addId(id, n);
+ }
// Script elements need to know when they're inserted
// into the document
@@ -785,8 +826,8 @@ function root(n) {
function uproot(n) {
// Manage id to element mapping
- if (n.nodeType === Node.ELEMENT_NODE) {
- var id = n.getAttribute('id');
+ if (n.nodeType === Node.ELEMENT_NODE && n.ownerDocument._byId !== null) {
+ var id = n.getAttributeNS(null, 'id');
if (id) n.ownerDocument.delId(id, n);
}
n.ownerDocument._nodes[n._nid] = undefined;
@@ -829,18 +870,23 @@ function recursivelySetOwner(node, owner) {
recursivelySetOwner(kid, owner);
}
-// A class for storing multiple nodes with the same ID
+// A class for storing multiple nodes with the same ID.
+//
+// A Map keeps the node ids used as keys out of sparse object-index storage:
+// a plain object keyed by integers gets an elements backing store sized to
+// the largest key, which for node ids means one as large as the document for
+// every id that is duplicated.
function MultiId(node) {
- this.nodes = Object.create(null);
- this.nodes[node._nid] = node;
+ this.nodes = new Map();
+ this.nodes.set(node._nid, node);
this.length = 1;
this.firstNode = undefined;
}
// Add a node to the list, with O(1) time
MultiId.prototype.add = function(node) {
- if (!this.nodes[node._nid]) {
- this.nodes[node._nid] = node;
+ if (!this.nodes.has(node._nid)) {
+ this.nodes.set(node._nid, node);
this.length++;
this.firstNode = undefined;
}
@@ -848,8 +894,7 @@ MultiId.prototype.add = function(node) {
// Remove a node from the list, with O(1) time
MultiId.prototype.del = function(node) {
- if (this.nodes[node._nid]) {
- delete this.nodes[node._nid];
+ if (this.nodes.delete(node._nid)) {
this.length--;
this.firstNode = undefined;
}
@@ -861,13 +906,13 @@ MultiId.prototype.del = function(node) {
MultiId.prototype.getFirst = function() {
/* jshint bitwise: false */
if (!this.firstNode) {
- var nid;
- for (nid in this.nodes) {
- if (this.firstNode === undefined ||
- this.firstNode.compareDocumentPosition(this.nodes[nid]) & Node.DOCUMENT_POSITION_PRECEDING) {
- this.firstNode = this.nodes[nid];
+ var self = this;
+ this.nodes.forEach(function(node) {
+ if (self.firstNode === undefined ||
+ self.firstNode.compareDocumentPosition(node) & Node.DOCUMENT_POSITION_PRECEDING) {
+ self.firstNode = node;
}
- }
+ });
}
return this.firstNode;
};
@@ -875,10 +920,7 @@ MultiId.prototype.getFirst = function() {
// If there is only one node left, return it. Otherwise return "this".
MultiId.prototype.downgrade = function() {
if (this.length === 1) {
- var nid;
- for (nid in this.nodes) {
- return this.nodes[nid];
- }
+ return this.nodes.values().next().value;
}
return this;
};
diff --git a/test/domino.js b/test/domino.js
index 00cc051..04d7695 100644
--- a/test/domino.js
+++ b/test/domino.js
@@ -1984,3 +1984,208 @@ exports.clonedTemplateContentMaintainsHostReference = function () {
);
};
+exports.idIndexIsLazy = function() {
+ // The id -> element index is only built when something actually looks an
+ // element up by id. Parsing, mutating and serializing a document must not
+ // build one: the fragment parser creates (and throws away) a private
+ // document for every innerHTML assignment, and indexing those is pure cost.
+ var doc = domino.createDocument('', true);
+ (doc._byId === null).should.be.true();
+
+ var div = doc.getElementsByTagName('div')[0];
+ div.innerHTML = 'y';
+ div.setAttribute('id', 'renamed');
+ doc.body.appendChild(doc.createElement('i')).setAttribute('id', 'e');
+ doc.body.serialize();
+ (doc._byId === null).should.be.true();
+
+ // ...and the index built on the first lookup reflects every one of those
+ // mutations.
+ doc.getElementById('c').tagName.should.equal('SPAN');
+ (doc._byId === null).should.be.false();
+ doc.getElementById('d').tagName.should.equal('B');
+ doc.getElementById('e').tagName.should.equal('I');
+ doc.getElementById('renamed').tagName.should.equal('DIV');
+ (doc.getElementById('a') === null).should.be.true();
+ (doc.getElementById('b') === null).should.be.true();
+};
+
+exports.idIndexBuiltLateMatchesIndexBuiltEarly = function() {
+ // Same mutations, two documents: one queried before the mutations (so the
+ // index is maintained incrementally) and one queried only afterwards (so the
+ // index is built in one pass). They must agree.
+ function mutate(doc) {
+ var body = doc.body;
+ var a = doc.createElement('a'); a.setAttribute('id', 'x'); body.appendChild(a);
+ var b = doc.createElement('b'); b.setAttribute('id', 'x'); body.appendChild(b);
+ var c = doc.createElement('c'); c.setAttribute('id', 'y'); body.appendChild(c);
+ body.removeChild(a);
+ c.setAttribute('id', 'x');
+ var d = doc.createElement('d'); d.setAttribute('id', 'z'); body.appendChild(d);
+ d.removeAttribute('id');
+ body.insertBefore(a, b);
+ return body;
+ }
+ function report(doc) {
+ return ['x', 'y', 'z'].map(function(id) {
+ var el = doc.getElementById(id);
+ return id + '=' + (el ? el.tagName : 'null') +
+ '/' + doc._hasMultipleElementsWithId(id) +
+ '/' + doc.querySelectorAll('#' + id).length;
+ }).join(' ');
+ }
+ var early = domino.createDocument('', true);
+ early.getElementById('nothing-yet'); // force the index to exist up front
+ mutate(early);
+
+ var late = domino.createDocument('', true);
+ mutate(late);
+
+ report(late).should.equal(report(early));
+ report(late).should.equal('x=A/true/3 y=null/false/0 z=null/false/0');
+};
+
+exports.idIndexAfterLazyBuildTracksRemovals = function() {
+ // duplicateID, but with the first lookup deferred until after the elements
+ // are in place, so the whole MultiId bucket comes from the one-pass build.
+ var doc = domino.createDocument('');
+ var root = doc.documentElement;
+ var kids = ['a', 'b', 'c', 'd'].map(function(name) {
+ var elt = doc.createElement(name);
+ elt.setAttribute('id', 'x');
+ return root.appendChild(elt);
+ });
+ // first lookup happens here, with all four already in the tree
+ assert.strictEqual(doc.getElementById('x'), kids[0]);
+ root.removeChild(kids[0]);
+ assert.strictEqual(doc.getElementById('x'), kids[1]);
+ root.removeChild(kids[2]);
+ assert.strictEqual(doc.getElementById('x'), kids[1]);
+ root.removeChild(kids[1]);
+ assert.strictEqual(doc.getElementById('x'), kids[3]);
+ root.removeChild(kids[3]);
+ assert.strictEqual(doc.getElementById('x'), null);
+};
+
+exports.idIndexReturnsFirstInDocumentOrder = function() {
+ // getFirst() compares document positions, so the bucket's internal key order
+ // must not matter: insert the duplicates back to front.
+ var doc = domino.createDocument('');
+ var root = doc.documentElement;
+ var first = null;
+ for (var i = 0; i < 8; i++) {
+ var elt = doc.createElement('n' + i);
+ elt.setAttribute('id', 'x');
+ root.insertBefore(elt, root.firstChild);
+ first = elt;
+ }
+ assert.strictEqual(doc.getElementById('x'), first);
+ assert.strictEqual(doc.getElementById('x'), root.firstChild);
+ doc.querySelectorAll('#x').should.have.length(8);
+};
+
+exports.idIndexHandlesIdsThatLookLikeArrayIndexes = function() {
+ var doc = domino.createDocument('', true);
+ var first = doc.getElementById('a');
+ first.setAttribute('id', '1023');
+ assert.strictEqual(doc.getElementById('1023'), first);
+ assert.strictEqual(doc.getElementById('a'), null);
+ assert.strictEqual(doc.getElementById('b'), doc.querySelectorAll('div')[1]);
+ doc.body.removeChild(first);
+ assert.strictEqual(doc.getElementById('1023'), null);
+};
+
+exports.idIndexCoercesLookupArguments = function() {
+ var values = [
+ 42, 0, true, false, null, undefined, NaN,
+ new String('42'), ['42'],
+ { toString: function() { return '42'; } }
+ ];
+ [false, true].forEach(function(early) {
+ values.forEach(function(value) {
+ var doc = domino.createDocument('', true);
+ if (early) doc.getElementById('nothing-yet');
+ var element = doc.body.appendChild(doc.createElement('div'));
+ element.id = String(value);
+ assert.ok(doc.getElementById(value) === element,
+ 'Expected lookup to coerce ' + String(value) + ' to a string');
+ });
+ });
+};
+
+exports.idIndexCoercesBeforeLazyBuild = function() {
+ var doc = domino.createDocument('', true);
+ var element = doc.body.firstChild;
+ assert.throws(function() {
+ doc.getElementById(Object.create(null));
+ }, TypeError);
+ assert.strictEqual(doc._byId, null);
+
+ var calls = 0;
+ var id = {
+ toString: function() {
+ calls++;
+ assert.strictEqual(doc._byId, null);
+ element.id = 'renamed';
+ return 'renamed';
+ }
+ };
+ assert.ok(doc.getElementById(id) === element);
+ assert.strictEqual(calls, 1);
+ assert.strictEqual(doc.getElementById('original'), null);
+};
+
+exports.idIndexIgnoresNamespacedIds = function() {
+ [false, true].forEach(function(early) {
+ var doc = domino.createDocument('', true);
+ if (early) doc.getElementById('nothing-yet');
+ var element = doc.body.appendChild(doc.createElement('div'));
+ element.setAttributeNS('urn:test', 'id', 'foreign');
+ assert.ok(doc.getElementById('foreign') === null);
+ assert.strictEqual(doc.querySelectorAll('#foreign').length, 0);
+
+ doc.body.removeChild(element);
+ doc.body.appendChild(element);
+ assert.ok(doc.getElementById('foreign') === null);
+ element.setAttributeNS('urn:test', 'id', 'changed');
+ assert.ok(doc.getElementById('changed') === null);
+ doc.body.removeChild(element);
+ });
+};
+
+exports.idIndexSeparatesIdNamespaces = function() {
+ [false, true].forEach(function(early) {
+ [false, true].forEach(function(attached) {
+ [false, true].forEach(function(xml) {
+ var doc = xml ?
+ domino.createDOMImplementation().createDocument(null, 'root', null) :
+ domino.createDocument('', true);
+ var parent = xml ? doc.documentElement : doc.body;
+ if (early) doc.getElementById('nothing-yet');
+ var element = doc.createElement('div');
+ if (attached) parent.appendChild(element);
+ element.setAttributeNS('urn:test', 'id', 'foreign');
+ element.setAttributeNS(null, 'id', 'real');
+ if (!attached) parent.appendChild(element);
+
+ assert.ok(doc.getElementById('real') === element);
+ assert.ok(doc.getElementById('foreign') === null);
+ assert.ok(doc.querySelector('#real') === element);
+ assert.strictEqual(doc.querySelectorAll('#foreign').length, 0);
+
+ element.getAttributeNodeNS(null, 'id').value = 'renamed';
+ assert.strictEqual(doc.getElementById('real'), null);
+ assert.ok(doc.getElementById('renamed') === element);
+ parent.removeChild(element);
+ assert.strictEqual(doc.getElementById('renamed'), null);
+ parent.appendChild(element);
+ assert.ok(doc.getElementById('renamed') === element);
+
+ element.removeAttributeNS(null, 'id');
+ assert.strictEqual(doc.getElementById('renamed'), null);
+ assert.ok(doc.getElementById('foreign') === null);
+ parent.removeChild(element);
+ });
+ });
+ });
+};