Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

Fix opener access errors #133

Merged
merged 4 commits into from
Jul 10, 2018

Conversation

steponas
Copy link
Contributor

@steponas steponas commented Jul 9, 2018

Currently, if a page with this plugin is opened after clicking a link from an external domain, an error is thrown during app initialisation:

DOMException: Blocked a frame with origin "https://<my_domain>" from accessing a cross-origin frame.

This is due to Chrome preventing any (even read) access to window.opener in such cases.

This PR provides a test case + fixes the issue by adding a try/catch around the event binding. I possibly could have gone with a known-blacklist but IMO this is a safe, future-proof approach.

@codecov
Copy link

codecov bot commented Jul 9, 2018

Codecov Report

Merging #133 into master will increase coverage by 2.46%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
+ Coverage      86%   88.46%   +2.46%     
==========================================
  Files           2        2              
  Lines          50       52       +2     
  Branches       16       16              
==========================================
+ Hits           43       46       +3     
+ Misses          6        5       -1     
  Partials        1        1
Impacted Files Coverage Δ
src/client.js 86.2% <100%> (+4.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 938d082...d6abaad. Read the comment docs.

@dennisgl
Copy link
Contributor

dennisgl commented Jul 9, 2018

On Chrome 69, window.opener wasn't throwing on the getter of the property itself, but when trying to access window.opener.prototype. I didn't dig up a documentation or commit but this is what we learned during testing.

dennisgl
dennisgl previously approved these changes Jul 9, 2018
Copy link
Contributor

@dennisgl dennisgl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine to mute the errors here for now. Error reporting in the setup itself is always tricky...


app.register(ErrorHandlingPlugin);

getSimulator(app);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

t.doesNotThrow(() => getSimulator(app));

@steponas
Copy link
Contributor Author

@dennisgl thanks for the quick review. I've pushed a commit with the explicit exception check.

@dennisgl
Copy link
Contributor

!merge

@old-fusion-bot old-fusion-bot bot merged commit 6316317 into fusionjs:master Jul 10, 2018
@steponas steponas deleted the fix-opener-access-errors branch July 11, 2018 09:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants