Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Use upgradeToSecure when possible#15157

Merged
Hainish merged 2 commits into
EFForg:masterEFForg/https-everywhere:masterfrom
bcyphers:add-upgrade-to-secure-supportbcyphers/https-everywhere:add-upgrade-to-secure-supportCopy head branch name to clipboard
Apr 28, 2018
Merged

Use upgradeToSecure when possible#15157
Hainish merged 2 commits into
EFForg:masterEFForg/https-everywhere:masterfrom
bcyphers:add-upgrade-to-secure-supportbcyphers/https-everywhere:add-upgrade-to-secure-supportCopy head branch name to clipboard

Conversation

@bcyphers

Copy link
Copy Markdown
Contributor

Closes #49.

Builds on #15092.

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. Turn on logging in the background page and look for "onBeforeRequest returning upgradeToSecure: true" messages, and filter outgoing requests to cdnjs.cloudflare.com to make sure they're being rewritten to https.

This PR uses the browser.runtime.getBrowserInfo API to determine the type and version of the browser. Since browser.* APIs aren't in chrome, it first checks whether browser is defined, then checks the browser name and version number accordingly. Unfortunately, since all browser.runtime.* APIs are implemented as promises, I had to add a new initialization function to the background page that figures out whether onBeforeRequest is available on load and saves it as a global(ish) variable.

Use Firefox's new upgradeToSecure flag instead of standard URI rewrites
when applying simple rulesets. Should fix issue with CORS blocking
certain HTTPS rewrites (EFForg#49). Use browser.runtime.getBrowserInfo, a
firefox-only API, to determine browser compatibility.
@cschanaj

Copy link
Copy Markdown
Collaborator

/cc @Hainish

@Giltyhub

Giltyhub commented Apr 23, 2018

Copy link
Copy Markdown
Contributor

@Hainish Also don't forget to close all CORS bugs issues and CORS bugs pull requests when this is merged!! CORS-bugs

@Hainish

Hainish commented Apr 23, 2018

Copy link
Copy Markdown
Member

I'll review this tomorrow.

@Hainish Hainish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

otherwise looks good!

Comment thread chromium/background-scripts/background.js Outdated
function getUpgradeToSecureAvailable() {
if (typeof browser !== 'undefined') {
return browser.runtime.getBrowserInfo().then(function(info) {
var version = info.version.match(/^(\d+)/)[1];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

Comment thread chromium/background-scripts/background.js Outdated
Comment thread chromium/background-scripts/background.js Outdated
@Hainish Hainish merged commit 5f6cec7 into EFForg:master Apr 28, 2018
@Hainish

Hainish commented Apr 28, 2018

Copy link
Copy Markdown
Member

Thank you!

@gloomy-ghost

Copy link
Copy Markdown
Collaborator

verified using a master build at 5f6cec7, I'm going to do a mass close for issues tagged as CORS.

@bardiharborow

Copy link
Copy Markdown
Contributor

@gloomy-ghost can you confirm that a mass close is appropriate given lack of Chrome support at this time?

@gloomy-ghost

Copy link
Copy Markdown
Collaborator

@bardiharborow see #49 (comment), it has been solved in Chrome for a long time

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.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.