1755742 - Unexpected onSecurityChange flag STATE_HTTPS_ONLY_MODE_UPGRADED when restoring tabs via session store

Status: NEW

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

Description

[Emma Zühlcke [:emz]]

When restoring tabs via session store and switching to those lazy loading tabs, we fire an onSecurityChange which flags includes STATE_HTTPS_ONLY_MODE_UPGRADED. This is unexpected, because HTTPS only mode is disabled.

When debugging I found that that we end up with httpsOnlyStatus == 0 here:
https://searchfox.org/mozilla-central/rev/69a482382823f42f482e840f65725218d7654cc4/security/manager/ssl/nsSecureBrowserUI.cpp#94-98 which sets this flag. Is 0 a valid state for httpsOnlyStatus?

I've found this issue while debugging Bug 1746383. In the video attached here https://bugzilla.mozilla.org/show_bug.cgi?id=1746383#c4 you can see that the identity panel is showing the https-only-mode UI. This may be unexpected.

You can reproduce this issue by setting Firefox to restore tabs on startup, and setting a breakpoint here, observing the aState field: https://searchfox.org/mozilla-central/rev/3de1b6791ea569cdf7773f7cd512cf2e6cc1d3f0/browser/base/content/browser.js#5599
and switching to one of the restored tab that wasn't selected initially.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:ckerschb, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Release mgmt bot [:marco/ :calixte] from comment #1)

When debugging I found that that we end up with httpsOnlyStatus == 0 here:
https://searchfox.org/mozilla-central/rev/69a482382823f42f482e840f65725218d7654cc4/security/manager/ssl/nsSecureBrowserUI.cpp#94-98 which sets this flag. Is 0 a valid state for httpsOnlyStatus?

Hey Julian, I assume this can happen when https-only was turned on, then turned off and then we restore tabs, right? Since you are most familiar with the code and state flags in nsSecureBrowserUI. Do you think it makes sense to additionally check if the https-only pref is true before updating mState?

Alternatively I think we need to figure out why the flag is 0, because in the loadinfo we should always init it with HTTPS_ONLY_UNINITIALIZED.

What are your thoughts?

Comment 3

[Julian Gaibler]

Hey, sorry, I had an issue with my Bugzilla mail :(

I chatted about this with Paul when he filed it and this also happens with a fresh profile. I think what happens is that the httpsOnlyStatus just gets completely lost during serialization/deserialization. The state flags for the browser UI are coming from WindowGlobalParent

I think the best option would be to figure out where the flag gets dropped — that might prevent some other bugs with HTTPS-Only/First :) — but I couldn't tell where this is happening.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Julian Gaibler from comment #3)
Thanks Julian.

I think the best option would be to figure out where the flag gets dropped — that might prevent some other bugs with HTTPS-Only/First :) — but I couldn't tell where this is happening.

Tomer, can you try to debug this when you get a chance? Thank you.

Comment 5

[Tomer Yavor]

Sure, I will check it