Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Use upgradeToSecure when possible#15092

Closed
bcyphers wants to merge 1 commit intomasterfrom
add-upgrade-to-secure-support
Closed

Use upgradeToSecure when possible#15092
bcyphers wants to merge 1 commit intomasterfrom
add-upgrade-to-secure-support

Conversation

@bcyphers
Copy link
Copy Markdown
Contributor

@bcyphers bcyphers commented Apr 7, 2018

Closes #49.

In Firefox, cross-origin resource sharing (CORS) requests that are approved for http urls are sometimes
blocked after being rewritten to https. Firefox has recently added a new option to webRequest.BlockingResponse, upgradeToSecure, which performs a simple http --> https rewrite. This branch tries to use that flag instead of a standard rewrite whenever (1) the rewrite is trivial and (2) the browser is Firefox 59+.

Can be tested on this website: http://shop.decatorevista.ro/

With the current version of the extension in Firefox, a few calls to maxcdn.bootstrapcdn.com will be blocked, breaking styles on the page. This branch allows them to be rewritten successfully.

Use Firefox's new upgradeToSecure flag instead of standard URI rewrites
when applying simple rulesets. Should fix issue with CORS blocking
certain HTTPS rewrites (#49).
@bcyphers bcyphers requested a review from Hainish April 7, 2018 01:03
@bcyphers
Copy link
Copy Markdown
Contributor Author

bcyphers commented Apr 7, 2018

Still need to work out the kinks in Chrome -- in the meantime, @Hainish do you have any other feedback?

@ghost
Copy link
Copy Markdown

ghost commented Apr 9, 2018

Warning: This browser detection code is licensed as "all rights reserved" and can't be used.

@ghost
Copy link
Copy Markdown

ghost commented Apr 10, 2018

See #15127.

@bcyphers
Copy link
Copy Markdown
Contributor Author

Oh thanks @epicminecrafting! Still new to the project, and that wouldn't have occurred to me. I'll take a closer look at your PR and try to merge it in.

@Hainish
Copy link
Copy Markdown
Member

Hainish commented Apr 10, 2018

Hey @bcyphers - also, please make pull requests from your own HTTPS Everywhere repository. This is to keep the number of branches listed in the public repo relatively manageable.

Thanks!

@cschanaj
Copy link
Copy Markdown
Collaborator

I guess the browser detection code can be replaced with compile time boolean variable since all we need to know is that whether the browser support the upgradeToSecure property. Chromium and Firefox extensions uses different fIle extensions anyway.

@ghost
Copy link
Copy Markdown

ghost commented Apr 11, 2018

@cschanaj We would still need to detect the Firefox version.

@cschanaj
Copy link
Copy Markdown
Collaborator

cschanaj commented Apr 11, 2018

@epicminecrafting in the case it is known that the browser is Firefox, we could use browser.runtime.getBrowserInfo() instead , I presume? See https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/getBrowserInfo

@Hainish
Copy link
Copy Markdown
Member

Hainish commented Apr 11, 2018

@cschanaj

@epicminecrafting in the case it is known that the browser is Firefox, we could use browser.runtime.getBrowserInfo() instead , I presume? See https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/getBrowserInfo

@bcyphers and I chatted about this, and this is the solution we came up with as well.

@bcyphers
Copy link
Copy Markdown
Contributor Author

I've amended the pull request enough that I'm going to close this one and re-open it from a branch in my own repo.

@bcyphers bcyphers closed this Apr 14, 2018
@bcyphers bcyphers deleted the add-upgrade-to-secure-support branch April 14, 2018 00:15
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.

3 participants