From 3390e7a4fdbcc9fafd8ee8ee370a5c1ed2018dbd Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <apaseltiner@chromium.org>
Date: Wed, 29 Apr 2026 13:30:00 -0700
Subject: [PATCH] [DOM Distiller] Fix unhandled exception in post-processing

An unhandled exception in fillYouTubePlaceholders could halt the
post-processing sequence, bypassing the security-critical sanitizeLinks
call.

This CL:

1. Wraps each post-processing step in a try-catch block in
   postProcessElement to ensure subsequent steps always run.
2. Fixes fillYouTubePlaceholders to attach the new iframe to the DOM
   before further processing, ensuring it has a parentElement.
3. Switches fillYouTubePlaceholders to use querySelectorAll with
   specific attribute selectors (data-type, data-id) and values() for
   stable, forward iteration.

Fixed: 501760514
Change-Id: I2c43b1ec06787ea641e8fd7123b7629d07d2d456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7803098
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@google.com>
Cr-Commit-Position: refs/heads/main@{#1622685}
---

diff --git a/components/dom_distiller/core/javascript/content_processing.js b/components/dom_distiller/core/javascript/content_processing.js
index 928ba0d1..2445f16 100644
--- a/components/dom_distiller/core/javascript/content_processing.js
+++ b/components/dom_distiller/core/javascript/content_processing.js
@@ -86,20 +86,24 @@
  * @param {HTMLElement} element The element to search for placeholders in.
  */
 function fillYouTubePlaceholders(element) {
-  const placeholders = element.getElementsByClassName('embed-placeholder');
-  for (let i = 0; i < placeholders.length; i++) {
-    if (!placeholders[i].hasAttribute('data-type') ||
-        placeholders[i].getAttribute('data-type') !== 'youtube' ||
-        !placeholders[i].hasAttribute('data-id')) {
-      continue;
-    }
+  const placeholders = element.querySelectorAll(
+      '.embed-placeholder[data-type="youtube"][data-id]');
+  for (const placeholder of placeholders.values()) {
     const embed = document.createElement('iframe');
-    const url = 'http://www.youtube.com/embed/' +
-        placeholders[i].getAttribute('data-id');
+    const url =
+        'http://www.youtube.com/embed/' + placeholder.getAttribute('data-id');
     embed.setAttribute('src', url);
     embed.setAttribute('type', 'text/html');
     embed.setAttribute('frameborder', '0');
-    embedYoutubeIFrame(embed);
+
+    // The placeholder must be replaced with the new element before it is
+    // passed to embedYoutubeIFrame, which expects the element to have a
+    // parent.
+    const parent = placeholder.parentElement;
+    if (parent) {
+      parent.replaceChild(embed, placeholder);
+      embedYoutubeIFrame(embed);
+    }
   }
 }
 
diff --git a/components/dom_distiller/core/javascript/dom_distiller_viewer_main.js b/components/dom_distiller/core/javascript/dom_distiller_viewer_main.js
index 9d48a1de..f3c6290 100644
--- a/components/dom_distiller/core/javascript/dom_distiller_viewer_main.js
+++ b/components/dom_distiller/core/javascript/dom_distiller_viewer_main.js
@@ -31,23 +31,33 @@
  * @param {HTMLElement} element The container element of the article.
  */
 function postProcessElement(element) {
+  const tryStep = (fn) => {
+    try {
+      fn(element);
+    } catch (e) {
+      // Use a generic error message to avoid leaking potentially sensitive
+      // information from the DOM.
+      console.error('Post-processing step failed.');
+    }
+  };
+
   // Remove surviving extraneous elements first so they don't interfere with
   // downstream classifiers and processing.
-  removeExtraneousElementsFrom(element);
+  tryStep(removeExtraneousElementsFrom);
 
   // Wrap tables to make them scrollable.
-  wrapTables(element);
+  tryStep(wrapTables);
 
   // Readability will leave iframes around, but they need the proper structure
   // and classes to be styled correctly.
-  addClassesToYoutubeIFrames(element);
+  tryStep(addClassesToYoutubeIFrames);
   // DomDistiller will leave placeholders, which need to be replaced with
   // actual iframes.
-  fillYouTubePlaceholders(element);
-  sanitizeLinks(element);
-  identifyEmptySVGs(element);
-  ImageClassifier.processImagesIn(element);
-  ListClassifier.processListsIn(element);
+  tryStep(fillYouTubePlaceholders);
+  tryStep(sanitizeLinks);
+  tryStep(identifyEmptySVGs);
+  tryStep((el) => ImageClassifier.processImagesIn(el));
+  tryStep((el) => ListClassifier.processListsIn(el));
 }
 
 function addToPage(html) {
diff --git a/components/test/data/dom_distiller/viewer_js_tester.js b/components/test/data/dom_distiller/viewer_js_tester.js
index 4ab8aa5..ed38e5e 100644
--- a/components/test/data/dom_distiller/viewer_js_tester.js
+++ b/components/test/data/dom_distiller/viewer_js_tester.js
@@ -22,4 +22,73 @@
         '<a href="http://example.com" target="_blank">good link</a>' +
             'bad linkanother bad link');
   });
+
+  test('PostProcessElementCatchesExceptions', async function() {
+    const {assert} = await import('./index.js');
+
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+
+    // Save original functions.
+    const originalRemove = window.removeExtraneousElementsFrom;
+    const originalConsoleError = console.error;
+
+    let errorLogged = false;
+
+    // Mock functions.
+    window.removeExtraneousElementsFrom = () => {
+      throw new Error('Test Error');
+    };
+
+    console.error = (msg) => {
+      if (msg === 'Post-processing step failed.') {
+        errorLogged = true;
+      }
+    };
+
+    try {
+      postProcessElement(container);
+    } finally {
+      // Restore original functions.
+      window.removeExtraneousElementsFrom = originalRemove;
+      console.error = originalConsoleError;
+    }
+
+    assert.isTrue(errorLogged, 'Error should be logged to console');
+  });
+
+  test('PostProcessElementContinuesOnFailure', async function() {
+    const {assert} = await import('./index.js');
+
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+
+    // Save original functions.
+    const originalRemove = window.removeExtraneousElementsFrom;
+    const originalWrap = window.wrapTables;
+    const originalConsoleError = console.error;
+
+    let wrapCalled = false;
+
+    // Mock functions.
+    window.removeExtraneousElementsFrom = () => {
+      throw new Error('Test Error');
+    };
+    window.wrapTables = () => {
+      wrapCalled = true;
+    };
+    // Suppress console.error during this test to avoid polluting output.
+    console.error = () => {};
+
+    try {
+      postProcessElement(container);
+    } finally {
+      // Restore original functions.
+      window.removeExtraneousElementsFrom = originalRemove;
+      window.wrapTables = originalWrap;
+      console.error = originalConsoleError;
+    }
+
+    assert.isTrue(wrapCalled, 'Subsequent steps should still run');
+  });
 });

diff --git a/components/test/data/dom_distiller/viewer_js_tester.js b/components/test/data/dom_distiller/viewer_js_tester.js
index 4ab8aa5..ed38e5e 100644
--- a/components/test/data/dom_distiller/viewer_js_tester.js
+++ b/components/test/data/dom_distiller/viewer_js_tester.js
@@ -22,4 +22,73 @@
         '<a href="http://example.com" target="_blank">good link</a>' +
             'bad linkanother bad link');
   });
+
+  test('PostProcessElementCatchesExceptions', async function() {
+    const {assert} = await import('./index.js');
+
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+
+    // Save original functions.
+    const originalRemove = window.removeExtraneousElementsFrom;
+    const originalConsoleError = console.error;
+
+    let errorLogged = false;
+
+    // Mock functions.
+    window.removeExtraneousElementsFrom = () => {
+      throw new Error('Test Error');
+    };
+
+    console.error = (msg) => {
+      if (msg === 'Post-processing step failed.') {
+        errorLogged = true;
+      }
+    };
+
+    try {
+      postProcessElement(container);
+    } finally {
+      // Restore original functions.
+      window.removeExtraneousElementsFrom = originalRemove;
+      console.error = originalConsoleError;
+    }
+
+    assert.isTrue(errorLogged, 'Error should be logged to console');
+  });
+
+  test('PostProcessElementContinuesOnFailure', async function() {
+    const {assert} = await import('./index.js');
+
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+
+    // Save original functions.
+    const originalRemove = window.removeExtraneousElementsFrom;
+    const originalWrap = window.wrapTables;
+    const originalConsoleError = console.error;
+
+    let wrapCalled = false;
+
+    // Mock functions.
+    window.removeExtraneousElementsFrom = () => {
+      throw new Error('Test Error');
+    };
+    window.wrapTables = () => {
+      wrapCalled = true;
+    };
+    // Suppress console.error during this test to avoid polluting output.
+    console.error = () => {};
+
+    try {
+      postProcessElement(container);
+    } finally {
+      // Restore original functions.
+      window.removeExtraneousElementsFrom = originalRemove;
+      window.wrapTables = originalWrap;
+      console.error = originalConsoleError;
+    }
+
+    assert.isTrue(wrapCalled, 'Subsequent steps should still run');
+  });
 });