Skip to content

Commit f11f271

Browse files
committed
Fix exponential DOM issue in unwrap
This fixes an issue in the current implementation where if `sanitize` is called on an HTML with deep nested unallowed elements it creates an exponential number of new elements and can eventually exhaust the available memory. For example, if you have this HTML: <x><y><z>blabla</z></y></x> The code first hits `<x>` and inserts its `.innerHTML` after it: <x><y><z>blabla</z></y></x> <y><z>blabla</z></y> Then it hits the inner `<y>`, and same thing: <x><y><z>blabla</z></y> <z>blabla</z></x> <y><z>blabla</z></y> Then the inner `<z>`: <x><y><z>blabla</z> blabla</y> <z>blabla</z></x> <y><z>blabla</z></y> Then the first `<z>`’s copy, etc. At the end, there are 7 nodes in the document (2^3-1). If you raise the depth from 3 to 20, you get 2^20-1=1M nodes. Note that there is also a performance issue as `.innerHTML` serializes the whole DOM and then `insertAdjacentHTML` unserializes it. This commit fixes both the exponential issue and the serialization/unserialization.
1 parent 86cd196 commit f11f271

1 file changed

Lines changed: 14 additions & 12 deletions

File tree

src/index.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -254,23 +254,25 @@ function sanitizeHtml(
254254
element.setAttribute('target', '_blank');
255255
}
256256
} else {
257-
element.insertAdjacentHTML('afterend', element.innerHTML);
257+
// Disallowed tag: mark, and unwrap only after traversal
258258
toRemove.push(element);
259259
}
260260
}
261261

262-
for (const element of toRemove) {
263-
try {
264-
try {
265-
element.parentNode?.removeChild(element);
266-
} catch {
267-
element.outerHTML = '';
268-
}
269-
} catch {
270-
try {
271-
element.remove();
272-
} catch {}
262+
// Unwrap disallowed elements by moving their child nodes before them
263+
// and then dropping the now-empty element.
264+
// Iterate in reverse to start from the innermost elements
265+
// and limit the size of the trees we move.
266+
for (let i = toRemove.length - 1; i >= 0; i--) {
267+
const element = toRemove[i];
268+
const parent = element.parentNode;
269+
if (!parent) continue; // already removed
270+
// copy each of its children above it
271+
while (element.firstChild) {
272+
parent.insertBefore(element.firstChild, element);
273273
}
274+
// then remove it
275+
parent.removeChild(element);
274276
}
275277

276278
const styleList = doc.querySelectorAll('style');

0 commit comments

Comments
 (0)