alex/yomichan
1
Fork 0
Code Releases Activity

Improve support for sandboxed iframes (#1704)

Browse Source 
* Add more tests

* Improve handling of errors from setupFrame

* Passively handle errors when contentDocument is null
This commit is contained in:
toasted-nutbread 2021-05-23 12:29:54 -04:00 committed by GitHub
parent ce340d7a19
commit 41c0132c59
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 50 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
ext/js/app/popup.js
View File

@ -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;

5
ext/js/comm/frame-client.js
View File

@ -156,7 +156,12 @@ class FrameClient {
// Prevent unhandled rejections
frameLoadedPromise.catch(() => {}); // NOP
try {
setupFrame(frame);
} catch (e) {
cleanup();
reject(e);
}
});
}

21
test/data/html/test-document2.html
View File

@ -24,6 +24,9 @@
top: 0;
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));
for (const iframeWithSrcdoc of document.querySelectorAll('.iframe-with-srcdoc')) {
iframeWithSrcdoc.srcdoc = dataUrlToContent(iframeWithDataUrl.src).content;
}
})();
</script>