From 41c0132c59a31c6d8bcc711b94b0859349e88f9b Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sun, 23 May 2021 12:29:54 -0400 Subject: [PATCH] Improve support for sandboxed iframes (#1704) * Add more tests * Improve handling of errors from setupFrame * Passively handle errors when contentDocument is null --- ext/js/app/popup.js | 29 ++++++++++++++++++++++++----- ext/js/comm/frame-client.js | 7 ++++++- test/data/html/test-document2.html | 23 ++++++++++++++++++++--- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/ext/js/app/popup.js b/ext/js/app/popup.js index af8645e4..43699735 100644 --- a/ext/js/app/popup.js +++ b/ext/js/app/popup.js @@ -228,20 +228,31 @@ class Popup extends EventDispatcher { _inject() { let injectPromise = this._injectPromise; if (injectPromise === null) { - injectPromise = this._createInjectPromise(); + injectPromise = this._injectInner1(); this._injectPromise = injectPromise; injectPromise.then( () => { if (injectPromise !== this._injectPromise) { return; } this._injectPromiseComplete = true; }, - () => { this._resetFrame(); } + () => {} ); } return injectPromise; } - async _createInjectPromise() { + async _injectInner1() { + try { + await this._injectInner2(); + return true; + } catch (e) { + this._resetFrame(); + if (e.source === this) { return false; } // Passive error + throw e; + } + } + + async _injectInner2() { if (this._options === null) { throw new Error('Options not initialized'); } @@ -255,9 +266,16 @@ class Popup extends EventDispatcher { frame.removeAttribute('srcdoc'); this._observeFullscreen(true); this._onFullscreenChanged(); + const {contentDocument} = frame; + if (contentDocument === null) { + // This can occur when running inside a sandboxed frame without "allow-same-origin" + const error = new Error('Popup not supoprted in this context'); + error.source = this; // Used to detect a passive error which should be ignored + throw error; + } const url = chrome.runtime.getURL('/popup.html'); if (useSecurePopupFrameUrl) { - frame.contentDocument.location.href = url; + contentDocument.location.href = url; } else { frame.setAttribute('src', url); } @@ -366,7 +384,8 @@ class Popup extends EventDispatcher { } async _show(elementRect, writingMode) { - await this._inject(); + const injected = await this._inject(); + if (!injected) { return; } const optionsGeneral = this._options.general; const {popupDisplayMode} = optionsGeneral; diff --git a/ext/js/comm/frame-client.js b/ext/js/comm/frame-client.js index d15cee30..c74a9913 100644 --- a/ext/js/comm/frame-client.js +++ b/ext/js/comm/frame-client.js @@ -156,7 +156,12 @@ class FrameClient { // Prevent unhandled rejections frameLoadedPromise.catch(() => {}); // NOP - setupFrame(frame); + try { + setupFrame(frame); + } catch (e) { + cleanup(); + reject(e); + } }); } diff --git a/test/data/html/test-document2.html b/test/data/html/test-document2.html index bb7bfb97..fb3adb59 100644 --- a/test/data/html/test-document2.html +++ b/test/data/html/test-document2.html @@ -24,6 +24,9 @@ top: 0; bottom: 0; right: 0; +} +.danger { + color: #c83c28; } @@ -108,7 +111,20 @@ <iframe> element with srcdoc. - + + + + + <iframe> element with srcdoc and sandbox="allow-same-origin allow-scripts". + + + + + + <iframe> element with srcdoc and sandbox="allow-scripts".
+ This element is expected to not work. + + @@ -180,9 +196,10 @@ const iframeWithDataUrl = document.querySelector('#iframe-with-data-url'); const iframeWithBlobUrl = document.querySelector('#iframe-with-blob-url'); - const iframeWithSrcdoc = document.querySelector('#iframe-with-srcdoc'); iframeWithBlobUrl.src = URL.createObjectURL(dataUrlToBlob(iframeWithDataUrl.src)); - iframeWithSrcdoc.srcdoc = dataUrlToContent(iframeWithDataUrl.src).content; + for (const iframeWithSrcdoc of document.querySelectorAll('.iframe-with-srcdoc')) { + iframeWithSrcdoc.srcdoc = dataUrlToContent(iframeWithDataUrl.src).content; + } })();