diff options
| author | toasted-nutbread <toasted-nutbread@users.noreply.github.com> | 2021-05-23 12:29:54 -0400 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2021-05-23 12:29:54 -0400 |
| commit | 41c0132c59a31c6d8bcc711b94b0859349e88f9b (patch) | |
| tree | f00e3a56021dc1634f30c79fa3ea71071039e0a6 | |
| parent | ce340d7a19f2d60a952820aac9e7c942f884eb98 (diff) | |
Improve support for sandboxed iframes (#1704)
* Add more tests
* Improve handling of errors from setupFrame
* Passively handle errors when contentDocument is null
| -rw-r--r-- | ext/js/app/popup.js | 29 | ||||
| -rw-r--r-- | ext/js/comm/frame-client.js | 7 | ||||
| -rw-r--r-- | 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 @@ -25,6 +25,9 @@ bottom: 0; right: 0; } +.danger { + color: #c83c28; +} </style> <body> @@ -108,7 +111,20 @@ <y-test> <y-description><iframe> element with srcdoc.</y-description> - <iframe id="iframe-with-srcdoc" allowfullscreen="true" class="container"></iframe> + <iframe allowfullscreen="true" class="iframe-with-srcdoc container"></iframe> + </y-test> + + <y-test> + <y-description><iframe> element with srcdoc and <code>sandbox="allow-same-origin allow-scripts"</code>.</y-description> + <iframe allowfullscreen="true" class="iframe-with-srcdoc container" sandbox="allow-same-origin allow-scripts"></iframe> + </y-test> + + <y-test> + <y-description> + <iframe> element with srcdoc and <code>sandbox="allow-scripts"</code>.<br> + <span class="danger">This element is expected to not work.</span> + </y-description> + <iframe allowfullscreen="true" class="iframe-with-srcdoc container" sandbox="allow-scripts"></iframe> </y-test> <y-test> @@ -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; + } })(); </script> |