From e8bb3ac54b6b6016f98cd6a0ea05bcfbd09dcc99 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:50:41 -0500 Subject: [PATCH] fix: avoid sparse storage for numeric attribute names Store qualified attribute names in a Map so V8 does not treat names such as `2539` as array indexes. Reconstructing formatting elements with those indexed properties could otherwise exhaust an Angular SSR worker's heap. Keep numeric names available through the standard attribute APIs, but do not mirror them onto NamedNodeMap because its numeric properties represent attribute positions. Add regression tests for parsing, mutation, cloning, and formatting reconstruction. Fixes angular/angular#70826 --- lib/Element.js | 80 ++++++++++-------- test/domino.js | 219 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+), 34 deletions(-) diff --git a/lib/Element.js b/lib/Element.js index 92c86e2..097ba75 100644 --- a/lib/Element.js +++ b/lib/Element.js @@ -28,7 +28,8 @@ function Element(doc, localName, namespaceURI, prefix) { this._tagName = undefined; // These properties maintain the set of attributes - this._attrsByQName = Object.create(null); // The qname->Attr map + // Map keeps numeric attribute names out of sparse object-index storage. + this._attrsByQName = new Map(); // The qname->Attr map this._attrsByLName = Object.create(null); // The ns|lname->Attr map this._attrKeys = []; // attr index -> ns|lname } @@ -534,7 +535,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { qname = String(qname); if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); if (!attr) return null; if (Array.isArray(attr)) // If there is more than one @@ -554,7 +555,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { qname = String(qname); if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - return this._attrsByQName[qname] !== undefined; + return this._attrsByQName.has(qname); }}, hasAttributeNS: { value: function hasAttributeNS(ns, lname) { @@ -573,7 +574,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { if (!xml.isValidName(qname)) utils.InvalidCharacterError(); if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - var a = this._attrsByQName[qname]; + var a = this._attrsByQName.get(qname); if (a === undefined) { if (force === undefined || force === true) { this._setAttribute(qname, ''); @@ -594,7 +595,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { // XXX: the spec says that this next search should be done // on the local name, but I think that is an error. // email pending on www-dom about it. - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); var isnew; if (!attr) { attr = this._newattr(qname); @@ -607,7 +608,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { // Now set the attribute value on the new or existing Attr object. // The Attr.value setter method handles mutation events, etc. attr.value = value; - if (this._attributes) this._attributes[qname] = attr; + setNamedProperty(this._attributes, qname, attr); if (isnew && this._newattrhook) this._newattrhook(qname, value); }}, @@ -695,7 +696,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { utils.InUseAttributeError(); } var result = null; - var oldAttrs = this._attrsByQName[attr.name]; + var oldAttrs = this._attrsByQName.get(attr.name); if (oldAttrs) { if (!Array.isArray(oldAttrs)) { oldAttrs = [ oldAttrs ]; } if (oldAttrs.some(function(a) { return a===attr; })) { @@ -734,7 +735,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); if (!attr) return; // If there is more than one match for this qname @@ -745,13 +746,13 @@ Element.prototype = Object.create(ContainerNode.prototype, { attr = attr.shift(); // remove it from the array } else { - this._attrsByQName[qname] = attr[1]; + this._attrsByQName.set(qname, attr[1]); attr = attr[0]; } } else { // only a single match, so remove the qname mapping - this._attrsByQName[qname] = undefined; + this._attrsByQName.delete(qname); } var ns = attr.namespaceURI; @@ -763,7 +764,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { var i = this._attrKeys.indexOf(key); if (this._attributes) { Array.prototype.splice.call(this._attributes, i, 1); - this._attributes[qname] = undefined; + setNamedProperty(this._attributes, qname, undefined); } this._attrKeys.splice(i, 1); @@ -832,20 +833,20 @@ Element.prototype = Object.create(ContainerNode.prototype, { // prefix will never have two matching Attr objects (because // setAttributeNS doesn't allow a non-null namespace with a // null prefix. - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); return attr ? attr.value : null; }}, // The raw version of setAttribute for reflected idl attributes. _setattr: { value: function _setattr(qname, value) { - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); var isnew; if (!attr) { attr = this._newattr(qname); isnew = true; } attr.value = String(value); - if (this._attributes) this._attributes[qname] = attr; + setNamedProperty(this._attributes, qname, attr); if (isnew && this._newattrhook) this._newattrhook(qname, value); }}, @@ -854,7 +855,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { _newattr: { value: function _newattr(qname) { var attr = new Attr(this, qname, null, null); var key = '|' + qname; - this._attrsByQName[qname] = attr; + this._attrsByQName.set(qname, attr); this._attrsByLName[key] = attr; if (this._attributes) { this._attributes[this._attrKeys.length] = attr; @@ -863,52 +864,48 @@ Element.prototype = Object.create(ContainerNode.prototype, { return attr; }}, - // Add a qname->Attr mapping to the _attrsByQName object, taking into + // Add a qname->Attr mapping to the _attrsByQName map, taking into // account that there may be more than one attr object with the // same qname _addQName: { value: function(attr) { var qname = attr.name; - var existing = this._attrsByQName[qname]; + var existing = this._attrsByQName.get(qname); if (!existing) { - this._attrsByQName[qname] = attr; + this._attrsByQName.set(qname, attr); } else if (Array.isArray(existing)) { existing.push(attr); } else { - this._attrsByQName[qname] = [existing, attr]; + this._attrsByQName.set(qname, [existing, attr]); } - if (this._attributes) this._attributes[qname] = attr; + setNamedProperty(this._attributes, qname, attr); }}, - // Remove a qname->Attr mapping to the _attrsByQName object, taking into + // Remove a qname->Attr mapping from the _attrsByQName map, taking into // account that there may be more than one attr object with the // same qname _removeQName: { value: function(attr) { var qname = attr.name; - var target = this._attrsByQName[qname]; + var target = this._attrsByQName.get(qname); if (Array.isArray(target)) { var idx = target.indexOf(attr); utils.assert(idx !== -1); // It must be here somewhere if (target.length === 2) { - this._attrsByQName[qname] = target[1-idx]; - if (this._attributes) { - this._attributes[qname] = this._attrsByQName[qname]; - } + this._attrsByQName.set(qname, target[1-idx]); + setNamedProperty(this._attributes, qname, this._attrsByQName.get(qname)); } else { target.splice(idx, 1); if (this._attributes && this._attributes[qname] === attr) { - this._attributes[qname] = target[0]; + setNamedProperty(this._attributes, qname, target[0]); } } } else { utils.assert(target === attr); // If only one, it must match - this._attrsByQName[qname] = undefined; - if (this._attributes) { - this._attributes[qname] = undefined; - } + this._attrsByQName.delete(qname); + setNamedProperty(this._attributes, qname, undefined); } }}, @@ -1090,15 +1087,30 @@ Attr.prototype = Object.create(Object.prototype, { // Sneakily export this class for use by Document.createAttribute() Element._Attr = Attr; +// WebIDL reserves array indices for indexed access, even outside the list. +// Mirroring numeric names would also create sparse object-index storage. +function isArrayIndex(qname) { + if (qname.length > 10) { return false; } + var index = qname >>> 0; + // Reject noncanonical spellings and 2^32-1, which is not an array index. + return index !== 0xFFFFFFFF && String(index) === qname; +} + +// Mirror a qname->Attr mapping onto an already-created NamedNodeMap. +function setNamedProperty(attributes, qname, attr) { + if (attributes && !isArrayIndex(qname)) { attributes[qname] = attr; } +} + // The attributes property of an Element will be an instance of this class. // This class is really just a dummy, though. It only defines a length // property and an item() method. The AttrArrayProxy that // defines the public API just uses the Element object itself. function AttributesArray(elt) { NamedNodeMap.call(this, elt); - for (var name in elt._attrsByQName) { - this[name] = elt._attrsByQName[name]; - } + var self = this; + elt._attrsByQName.forEach(function(attr, qname) { + setNamedProperty(self, qname, attr); + }); for (var i = 0; i < elt._attrKeys.length; i++) { this[i] = elt._attrsByLName[elt._attrKeys[i]]; } diff --git a/test/domino.js b/test/domino.js index bf0b33b..e8d0636 100644 --- a/test/domino.js +++ b/test/domino.js @@ -1,4 +1,5 @@ 'use strict'; +var assert = require('assert'); var domino = require('../lib'); var fs = require('fs'); var html = fs.readFileSync(__dirname + '/fixture/doc.html', 'utf8'); @@ -260,6 +261,224 @@ exports.attributes2 = function() { (div.attributes.onclick === undefined).should.be.true(); }; +var indexNames = ['0', '1', '7', '42', '4294967294']; +var otherNames = [ + '00', '01', '-0', '+1', '-1', '1.0', '1.5', '1e2', '0x10', + '4294967295', '4294967296', '10000000000', 'nan', 'infinity' +]; + +function indexedKeys(object) { + return Object.getOwnPropertyNames(object).filter(function(name) { + return /^(0|[1-9][0-9]*)$/.test(name) && Number(name) < 0xFFFFFFFF; + }); +} + +function checkStorage(element) { + var names = element.getAttributeNames(); + assert.ok(element._attrsByQName instanceof Map); + assert.strictEqual(element._attrsByQName.size, new Set(names).size); + assert.deepStrictEqual(indexedKeys(element._attrsByQName), []); + assert.deepStrictEqual(indexedKeys(element._attrsByLName), []); + var attrs = element.attributes; + assert.strictEqual(attrs, element.attributes); + assert.strictEqual(attrs.length, names.length); + assert.deepStrictEqual(indexedKeys(attrs), names.map(function(_, i) { return String(i); })); + names.forEach(function(name, i) { + assert.strictEqual(attrs[i], attrs.item(i)); + assert.strictEqual(attrs[i].name, name); + assert.strictEqual(attrs[i].ownerElement, element); + assert.ok(element._attrsByQName.has(name)); + }); + assert.strictEqual(attrs[names.length], undefined); + assert.strictEqual(attrs.item(names.length), null); + assert.deepStrictEqual(Array.from(attrs), names.map(function(_, i) { return attrs[i]; })); +} + +function parseElement(html) { + return domino.createDocument(html).body.firstChild; +} + +exports.numericAttributes = { + 'stores parsed numeric names without sparse property keys': function() { + var element = parseElement('
'); + checkStorage(element); + assert.strictEqual(element.getAttribute('999'), 'x'); + assert.strictEqual(element.getAttribute('2539'), 'y'); + assert.strictEqual(element.hasAttribute('2540'), false); + assert.strictEqual(element.attributes.getNamedItem('2539').value, 'y'); + assert.strictEqual(element.outerHTML, '
'); + }, + + 'distinguishes array indices from numeric-looking names': function() { + indexNames.concat(otherNames).forEach(function(name) { + var element = parseElement('
'); + checkStorage(element); + var attr = element.getAttributeNode(name); + assert.strictEqual(attr.value, 'value'); + assert.strictEqual(element.attributes.getNamedItem(name), attr); + if (indexNames.indexOf(name) !== -1) { + assert.strictEqual(element.attributes[name], element.attributes.item(Number(name)) || undefined); + } else { + assert.strictEqual(element.attributes[name], attr); + } + }); + }, + + 'keeps dense indices when attributes are added or updated after access': function() { + var element = parseElement('
'); + var attrs = element.attributes; + element._setAttribute('1', 'second'); + element._setAttribute('42', 'last'); + element._setAttribute('0', 'updated'); + element.id = 'reflected'; + checkStorage(element); + assert.strictEqual(element.attributes, attrs); + assert.deepStrictEqual(element.getAttributeNames(), ['title', '0', '1', '42', 'id']); + assert.strictEqual(attrs[0].name, 'title'); + assert.strictEqual(attrs.getNamedItem('0').value, 'updated'); + assert.strictEqual(attrs.id.value, 'reflected'); + }, + + 'keeps DOM setter validation unchanged': function() { + var element = domino.createDocument().createElement('div'); + assert.throws(function() { element.setAttribute('7', 'value'); }, {name: 'InvalidCharacterError'}); + assert.throws(function() { element.setAttributeNS(null, '7', 'value'); }, {name: 'InvalidCharacterError'}); + assert.throws(function() { element.toggleAttribute('7'); }, {name: 'InvalidCharacterError'}); + checkStorage(element); + }, + + 'preserves clone and import storage before and after attributes access': function() { + [false, true].forEach(function(materialized) { + var element = parseElement('
'); + if (materialized) { checkStorage(element); } + var target = domino.createDocument(); + [element.cloneNode(true), target.importNode(element, true)].forEach(function(copy) { + assert.strictEqual(copy.outerHTML, element.outerHTML); + checkStorage(copy); + checkStorage(copy.firstChild); + assert.notStrictEqual(copy.getAttributeNode('0'), element.getAttributeNode('0')); + copy.getAttributeNode('0').value = 'changed'; + assert.strictEqual(element.getAttribute('0'), 'first'); + }); + }); + }, + + 'transfers and replaces parsed Attr nodes on a live NamedNodeMap': function() { + ['setAttributeNode', 'setAttributeNodeNS'].forEach(function(method) { + var source = parseElement('
'); + var target = parseElement('
'); + checkStorage(source); + checkStorage(target); + var attr = source.removeAttributeNode(source.getAttributeNode('0')); + var old = target[method](attr); + assert.strictEqual(old.value, 'old'); + assert.strictEqual(old.ownerElement, null); + assert.strictEqual(attr.ownerElement, target); + assert.strictEqual(target.getAttribute('0'), 'new'); + checkStorage(source); + checkStorage(target); + }); + }, + + 'retains namespace aliases until the last qualified-name match is removed': function() { + var element = domino.createDocument().createElement('div'); + checkStorage(element); + ['urn:first', 'urn:second', 'urn:third'].forEach(function(ns, i) { + element.setAttributeNS(ns, 'p:name', String(i)); + }); + checkStorage(element); + assert.strictEqual(element.getAttribute('p:name'), '0'); + element.removeAttributeNS('urn:second', 'name'); + checkStorage(element); + element.removeAttribute('p:name'); + assert.strictEqual(element.getAttribute('p:name'), '2'); + assert.strictEqual(element.hasAttribute('p:name'), true); + checkStorage(element); + element.removeAttributeNode(element.getAttributeNode('p:name')); + assert.strictEqual(element.hasAttribute('p:name'), false); + checkStorage(element); + }, + + 'preserves Map-method names and ordinary reflected attributes': function() { + var element = parseElement('
'); + checkStorage(element); + element.setAttribute('get', 'updated'); + element.className = 'example'; + assert.strictEqual(element.getAttribute('get'), 'updated'); + assert.strictEqual(element.attributes.get.value, 'updated'); + assert.strictEqual(element.toggleAttribute('hidden'), true); + assert.strictEqual(element.toggleAttribute('hidden'), false); + checkStorage(element); + }, + + 'preserves numeric attributes in foreign content and templates': function() { + var doc = domino.createDocument('' + + ''); + ['svg', 'g', 'math'].forEach(function(tag) { checkStorage(doc.querySelector(tag)); }); + var child = doc.querySelector('template').content.firstChild; + assert.strictEqual(child.getAttribute('7'), 'template'); + checkStorage(child); + }, + + 'preserves attributes through small formatting reconstruction and reparsing': function() { + var input = '

one

two'; + var expected = '

one

' + + '

two

'; + var doc = domino.createDocument(input); + assert.strictEqual(doc.body.innerHTML, expected); + Array.from(doc.querySelectorAll('b')).forEach(checkStorage); + doc.body.innerHTML = doc.body.innerHTML; + assert.strictEqual(doc.body.innerHTML, expected); + Array.from(doc.querySelectorAll('b')).forEach(checkStorage); + var template = doc.createElement('template'); + template.innerHTML = input; + assert.strictEqual(template.innerHTML, expected); + Array.from(template.content.querySelectorAll('b')).forEach(checkStorage); + }, + + 'preserves numeric names across incremental parser chunk boundaries': function() { + var input = '
'; + for (var split = 0; split <= input.length; split++) { + var parser = domino.createIncrementalHTMLParser(); + parser.write(input.slice(0, split)); + parser.process(); + parser.end(input.slice(split)); + assert.strictEqual(parser.process(), false); + var element = parser.document().body.firstChild; + assert.strictEqual(element.outerHTML, '
'); + checkStorage(element); + } + } +}; + +var removers = { + removeAttribute: function(element, name) { element.removeAttribute(name); }, + removeAttributeNS: function(element, name) { element.removeAttributeNS(null, name); }, + removeAttributeNode: function(element, name) { element.removeAttributeNode(element.getAttributeNode(name)); }, + removeNamedItem: function(element, name) { element.attributes.removeNamedItem(name); }, + removeNamedItemNS: function(element, name) { element.attributes.removeNamedItemNS(null, name); } +}; +Object.keys(removers).forEach(function(method) { + exports.numericAttributes[method + ' releases names and keeps indices dense'] = function() { + [false, true].forEach(function(materialized) { + var element = parseElement('
'); + if (materialized) { checkStorage(element); } + ['0', '42', '1', 'title'].forEach(function(name) { + var attr = element.getAttributeNode(name); + removers[method](element, name); + assert.strictEqual(attr.ownerElement, null); + assert.strictEqual(element.hasAttribute(name), false); + assert.strictEqual(element.getAttribute(name), null); + assert.strictEqual(element._attrsByQName.has(name), false); + checkStorage(element); + }); + element._setAttribute('0', 'again'); + checkStorage(element); + assert.strictEqual(element.getAttribute('0'), 'again'); + }); + }; +}); + // exports.jquery1_9 = function() { // var window = createWindow(html); // var f = __dirname + '/fixture/jquery-1.9.1.js';