Closed Bug 1943551 Opened 20 days ago Closed 15 days ago

Don't run HTTPS-First upgrades when a navigation is triggered by a WebDriver Classic / BiDi navigation API

Categories

(Remote Protocol :: Agent, task, P2)

task
Points:
3

Tracking

(firefox136 fixed)

RESOLVED FIXED
136 Branch
Tracking Status
firefox136 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Regressed 1 open bug)

Details

(Whiteboard: [webdriver:m15])

Attachments

(1 file)

Over the last couple of days I have investigated a couple of failing Playwright tests. As it turned out those were failing because of the experimental HTTPS-First feature that is enabled in Firefox Nightly. Turning it off made tests that eg. require a HTTP connection due to a defined HTTP proxy to pass. But other tests could be affected as well by this feature because a started navigation would be replaced with a new one with the HTTPS protocol, which could have side-effects to our code that is observing navigations via the webprogress listener.

Chrome does not fail those tests and as given by Freddy has it implemented for already a couple of releases. So they may turn it off for automation. As well users of WebDriver BiDi and Marionette would see a difference between Firefox Nightly / DevEdition builds and releases.

While it would be good to test the normal user behavior we should turn this feature off for now and investigate what needs to be done to properly support it. Then various clients can decide if they want to turn it on or off.

As Freddy mentioned there is a flag for the location bar which let us control the behavior. HTTPS-First only kicks in when the entered URL doesn't contain a schema. That means entering http://example.com will end-up on this site. While with example.com Firefox will try to upgrade to HTTPS.

Maybe we only need such a flag for the browsingContext.loadURI API.

I'm leaning towards aligning with other browsers. We aren't exactly the first one to ship this.
We should also make sure to fix more than just playwright (What else is there? Puppeteer? WebDriver?)

Playwright actually turns off this feature for Chrome: https://github.com/microsoft/playwright/blob/f11768436ae0888fa2cc0803eb7a79e0dedb7d5b/packages/playwright-core/src/server/chromium/chromiumSwitches.ts#L44

Given that we haven't heard from anyone else complains about that behavior (it might be that no-one runs Nightly builds for tests) we may consider to disable it for Playwright, and find a solution as referred to in comment 2.

Using schemelessInput: Ci.nsILoadInfo.SchemelessInputTypeSchemeful, as part of the loadInfo flags works like a charm and all the previously failing tests in Playwright now pass. I'll prepare a patch so that we probably do not have to temporarily disable this feature in general for Remote Protocol. Playwright could still do it on their own if they want to have parity.

Note that with the upcoming patch it will not change the behavior of Firefox to actually run HTTPS-First upgraded when a link is clicked or a script with window.open("example.com") is executed.

I'll add some basic wdspec tests as well, because a while ago we moved each and every test to use HTTPS by default, which means that we no longer have coverage for HTTP.

Summary: Temporarily disable the experimental HTTP-First mode for Remote Protocols → Don't run HTTPS-First upgrades when a navigation is triggered by a WebDriver Classic / BiDi navigation API

When writing tests I noticed the following:

Whenever I have opened http://example.org once and trying to load example.org afterward in the same tab or a new one, the page never gets upgraded. The resulting URL will always be http://example.org. Is that expected? Even closing all related tabs and opening a new one doesn't make a difference. Do we store the state somewhere?

Flags: needinfo?(fbraun)

I spoke with Alex and they are not making any changes to CDP/BiDi in case of Chrome. It means that navigation requests to HTTP are going to be upgraded to HTTPS if possible. So far they had only a single report.

As such I wonder if we should go ahead and make that change or do nothing so that there is no different experience for users of WebDriver BiDi and classic. Lets discuss in our triage meeting on Monday.

Whiteboard: [webdriver:triage]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #5)

Whenever I have opened http://example.org once and trying to load example.org afterward in the same tab or a new one, the page never gets upgraded. [...] Do we store the state somewhere?

I mentioned this already on Slack, the answer is yes. We intentionally store exceptions (for a week) to avoid delays on further loads.

Flags: needinfo?(fbraun)

In today’s triage meeting, we decided to align with Firefox’s behavior and not upgrade connections from HTTP to HTTPS when using the WebDriver navigation API. Since this involves a request for a specific URL, the behavior should match what Firefox does when a user enters the URL in the location bar. This ensures consistency for WebDriver users, even though it will differ from Chrome’s implementation of the WebDriver protocol.

Freddy or Simon, as far as I remember you were fine with this approach, right?

(In reply to Simon Friedberger (:simonf) from comment #7)

I mentioned this already on Slack, the answer is yes. We intentionally store exceptions (for a week) to avoid delays on further loads.

Could you please tell where exactly it is stored and if it can be removed via a Gecko API. At least I cannot find it listed in the exceptions under HTTPS settings.

Flags: needinfo?(sfriedberger)
Flags: needinfo?(fbraun)
Whiteboard: [webdriver:triage]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #8)

(In reply to Simon Friedberger (:simonf) from comment #7)

I mentioned this already on Slack, the answer is yes. We intentionally store exceptions (for a week) to avoid delays on further loads.

Could you please tell where exactly it is stored and if it can be removed via a Gecko API. At least I cannot find it listed in the exceptions under HTTPS settings.

Simon helped me on Slack already so we can easily get rid of these exception by just cleaning all permissions with Services.perms.removeAll();

Flags: needinfo?(fbraun)
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Points: --- → 3
Priority: -- → P2
Whiteboard: [webdriver:m15]
Flags: needinfo?(sfriedberger)

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #8)

In today’s triage meeting, we decided to align with Firefox’s behavior [...]

Did you mean to say Chrome here?

No, Chrome folks will still upgrade as of now but we are aligning our behavior to Firefox - so no upgrade with http:// (because URLs need to have a scheme).

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/db90df42d25a [remote] Don't run HTTPS-First upgrades for WebDriver navigation APIs. r=webdriver-reviewers,simonf,Sasha
Status: ASSIGNED → RESOLVED
Closed: 15 days ago
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch
Regressions: 1946669
Regressions: 1947429
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: