Add upgrade-insecure-requests to responseHeaders in HTTP Nowhere Mode

[cschanaj]

cc @Hainish @gloomy-ghost It would be really great to see this in the next release.

TEST CASES

1. CSP Exists, Contains upgrade-insecure-requests directive

• https://www.w3.org/TR/upgrade-insecure-requests/

2. CSP Exists, Not Contains upgrade-insecure-requests directive

• https://www.theguardian.com/international

3. CSP Not Exists, With mixed content

• https://www.gsu.edu/about/

4. CSP Not Exists, Without mixed content

• https://www.mozilla.org/

Follow up: Should we modify the CORS headers as well??

Close #8506 Close cschanaj/https-everywhere/pull/13

[Bisaloo]

Built and tested with http://www.gsu.edu/about/. Good job! 👍

[ghost]

@cschanaj If Content-Security-Policy header already exists, you should modify it instead of adding a new one.

[ghost]

@cschanaj I've created a PR in your repo. Check it out.

[cschanaj]

@epicminecrafting is there any new feature in the code you proposed? since the code is relatively short, I am not going to bother with the coding style here.

[ghost]

@cschanaj Just minor fixes, like appending instead of prepending and simpler and faster case-ignoring comparision.

[Hainish]

@cschanaj I can review as-is or wait for you to merge @epicminecrafting's changes. Please advise.

[cschanaj]

@Hainish Please review this as-is. thanks!

[Hainish]

There may be a wild edge case here. In

https-everywhere/chromium/background.js

Lines 247 to 253 in 2808747

	const shouldCancel = httpNowhereOn &&
	uri.protocol === 'http:' &&
	uri.hostname.slice(-6) !== '.onion' &&
	uri.hostname !== 'localhost' &&
	!/^127(\.[0-9]{1,3}){3}$/.test(canonical_host) &&
	!/^0\.0\.0\.0$/.test(canonical_host) &&
	uri.hostname !== '[::1]';

, we specifically allow .onion URLs that do not have HTTPS in HTTP Nowhere mode. If a .onion URL serves only one page in HTTPS, and the rest are over HTTP, landing on that page with this logic will cause the client to make unreachable upgraded requests over HTTPS.

As I said, it's a pretty far-fetched edge case (why would a .onion site have a TLS configuration like that?) but it is possible, and probably worth accounting for.

[cschanaj]

I agree. Since .onion requests are encrypted anyway, IMHO we shall exclude them from the HTTPS upgrades, see e4b17e8

[ghost]

I think requests to non-.onion websites should still be upgraded if they were made from .onion website.

[ghost]

return {};

[ghost]

Don't use a for loop, use find function.

[cschanaj]

As per the Chrome's docs in https://developer.chrome.com/extensions/webRequest#type-HttpHeaders

... Only return responseHeaders if you really want to modify the headers in order to limit the number of conflicts (only one extension may modify responseHeaders for each request).

@epicminecrafting fixed in 9541530; as for loop vs find, I don't think there is much difference. Thanks anyway.

[ghost]

@cschanaj There is, declarative programming over imperative one.

[Hainish]

Thank you!