Skip to content

Commit

Permalink
Fix security error mixing cross-origin frames and promises in old bro…
Browse files Browse the repository at this point in the history
…wsers

Fix an issue where the sidebar failed to appear in Safari 11 and Chrome
63 when returning a cross-origin Window from a Promise `then` callback.
In these browsers an exception is triggered when the browser tries to
test if the return value is a Promise that can be unwrapped.

The issue was resolved in more recent browsers by adding an undefined
`then` property to cross origin objects. [1]

[1] whatwg/dom#536
  • Loading branch information
robertknight committed Sep 7, 2021
1 parent 223f329 commit b5e13ae
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
8 changes: 4 additions & 4 deletions src/annotator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ function init() {
sidebar = new Sidebar(document.body, eventBus, guest, getConfig('sidebar'));

// Expose sidebar window reference for use by same-origin guest frames.
window_.__hypothesis.sidebarWindow = sidebar.ready.then(
() => sidebar.iframe.contentWindow
);
window_.__hypothesis.sidebarWindow = sidebar.ready.then(() => [
sidebar.iframe.contentWindow,
]);
}

// Clear `annotations` value from the notebook's config to prevent direct-linked
Expand All @@ -93,7 +93,7 @@ function init() {

if (sidebarWindow) {
const sidebarOrigin = new URL(sidebarLinkElement.href).origin;
sidebarWindow.then(frame =>
sidebarWindow.then(([frame]) =>
guest.crossframe.connectToSidebar(frame, sidebarOrigin)
);
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/types/annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,14 @@
* @prop {import('./pdfjs').PDFViewerApplication} [PDFViewerApplication] -
* PDF.js entry point. If set, triggers loading of PDF rather than HTML integration.
* @prop {object} [__hypothesis] - Internal data related to supporting guests in iframes
* @prop {Promise<Window>} [__hypothesis.sidebarWindow] -
* @prop {Promise<[Window]>} [__hypothesis.sidebarWindow] -
* The sidebar window that is active in this frame. This resolves once the sidebar
* application has started and is ready to connect to guests.
*
* The `Window` object is wrapped in an array to avoid issues with
* combining promises with cross-origin objects in old browsers
* (eg. Safari 11, Chrome <= 63) which trigger an exception when trying to
* test if the object has a `then` method. See https://github.com/whatwg/dom/issues/536.
*/

/**
Expand Down

0 comments on commit b5e13ae

Please sign in to comment.