summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortoasted-nutbread <toasted-nutbread@users.noreply.github.com>2021-05-23 12:29:54 -0400
committerGitHub <noreply@github.com>2021-05-23 12:29:54 -0400
commit41c0132c59a31c6d8bcc711b94b0859349e88f9b (patch)
treef00e3a56021dc1634f30c79fa3ea71071039e0a6
parentce340d7a19f2d60a952820aac9e7c942f884eb98 (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.js29
-rw-r--r--ext/js/comm/frame-client.js7
-rw-r--r--test/data/html/test-document2.html23
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>&lt;iframe&gt; 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>&lt;iframe&gt; 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>
+ &lt;iframe&gt; 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>