1828063 - IsUpgradeDowngradeEndlessLoop might block legitimate redirects

Status: RESOLVED DUPLICATE of bug 1747230

(Core :: DOM: Security, defect, P3)

Description

[Tom Schuster]

In bug 1804684 I added a test where I had to set the pref dom.security.https_only_mode_break_upgrade_downgrade_endless_loop to false. It seems like all redirects with different fragments or querys will be blocked unless they have an user activation.

Comment 1

[Manuel Bucher [:manuel]]

I think this should only return false here and continue the function when true:
https://searchfox.org/mozilla-central/rev/7e1f58e993f362d5d16bd1230a4417ebb2aa07b3/dom/security/nsHTTPSOnlyUtils.cpp#305
Edit: not the case, maybe not the same bug?

Comment 2

[Daniel Veditz [:dveditz]]

The line in comment 1 is CHECK_upgrade, and comment 0 was about BREAK_upgrade. I have no idea why we have two prefs -- it looks like they would both bail out before anything much happens in there. Maybe Kershaw knows since he touched those lines last.

Comment 3

[Kershaw Chang [:kershaw]]

(In reply to Daniel Veditz [:dveditz] from comment #2)

The line in comment 1 is CHECK_upgrade, and comment 0 was about BREAK_upgrade. I have no idea why we have two prefs -- it looks like they would both bail out before anything much happens in there. Maybe Kershaw knows since he touched those lines last.

Sorry for the late reply.
I took a quick look by running this test and it seems that the reason why IsUpgradeDowngradeEndlessLoop returns true at this line, which has nothing to do with the pref mentioned in comment #1.
I think this is because the triggeringPrincipal's URI is http://example.com/browser/dom/security/test/https-only/file_fragment_noscript.html and we think this is an endless loop when redirecting to https://example.com/browser/dom/security/test/https-only/file_fragment_noscript.html#foo.
Not sure how to fix this. Maybe also compare the reference (#foo) of the URI?

Comment 4

[Manuel Bucher [:manuel]]

Attachment: Bug 1828063 - Add Pref to not upgrade same origin redirects in HTTPS-First mode r=freddyb

This reintroduces a pref that affected both HTTPS-First and HTTPS-Only
mode. In HTTPS-Only mode this pref doesn't make sense and got removed.
To keep allowing this behavior in HTTPS-First mode this patch introduces
it again.

Website breakages for websites misconfigured like in Bug 1725646. If
those are more common, we might want to flip the pref.

Or the Pref could be removed in the future if we don't find use for it.

Comment 5

[Phabricator Automation]

Comment on attachment 9380824 [details]
Bug 1828063 - Add Pref to not upgrade same origin redirects in HTTPS-First mode r=freddyb

Revision D202147 was moved to bug 1747230. Setting attachment 9380824 [details] to obsolete.

Comment 6

[Manuel Bucher [:manuel]]

Wrong bug number in patch.

Comment 7

[Manuel Bucher [:manuel]]

Fixed in Bug 1747230. Marking as duplicate now.