Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

window.ipfs all_frames? #392

Closed
alanshaw opened this issue Feb 22, 2018 · 3 comments
Closed

window.ipfs all_frames? #392

alanshaw opened this issue Feb 22, 2018 · 3 comments
Labels
kind/discussion Topical discussion; usually not changes to codebase

Comments

@alanshaw
Copy link
Member

Currently the content script that injects window.ipfs is set to inject itself in all_frames. This means that if a page has an iframe on it, it will also get a window.ipfs object.

https://github.com/ipfs-shipyard/ipfs-companion/blob/b968f01358788d9f581cd70b0ec5eb13b5ad3f05/add-on/manifest.json#L64

What is everyone feeling on dialing this down to false before we release, and then potentially dialing it up in the future if it's requested? I'm thinking performance and security might be a concern.

@alanshaw alanshaw added the kind/discussion Topical discussion; usually not changes to codebase label Feb 22, 2018
@RangerMauve
Copy link

How much overhead is there really with injecting it?
Iframes will have security scoped to to the origin within the iframe, right? I don't think there's any security issues there other than the ones that exist for having window.ipfs at all.

@lidel lidel mentioned this issue Feb 23, 2018
27 tasks
@lidel
Copy link
Member

lidel commented Feb 23, 2018

I've seen a short-lived "the webpage is slowing down your browser" warning a few times, but it was after extension was reloaded via R in web-ext in developer mode.

Not sure it happens "in production", but I'd say we should keep "all_frames": true enabled in dev channel for now, so that if performance problem exists, people can report it.

As for disabling it before stable release, I added decision to release checklist at #347

@alanshaw
Copy link
Member Author

roger roger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion Topical discussion; usually not changes to codebase
Projects
None yet
Development

No branches or pull requests

3 participants