1885893 - HTTPS-First Telemetry is collecting bad "Downgraded on Timer" data

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Malte Jürgens [:maltejur]]

The current "Downgraded on Timer" data for the HTTPS-First telemetry seems wrong. We count "downgraded on timer" more often that the generic "downgraded" (example). That means we sometimes count the "downgraded on timer" case, even when the upgrade was successful and we did not downgrade. I can also confirm that from local testing with httpsfirst.downgraded_on_timer_schemeless.

Comment 1

[Malte Jürgens [:maltejur]]

Attachment: Bug 1885893 - Fix HTTPS-First "Downgraded on Timer" Telemetry r?freddyb

Move code for counting "downgrade on timer" telemetry from
TestHTTPAnswerRunnable::OnStartRequest to
nsHTTPSOnlyUtils::PotentiallyDowngradeHttpsFirstRequest, with a check if the
channel status is NS_ERROR_NET_TIMEOUT_EXTERNAL, which indicates that the
channel has been canceled by the timer. This fixes some cases where the HTTPS
request is actually successful, but we still count a timer downgrade.

Comment 2

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

Set release status flags based on info from the regressing bug 1868380

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5844bfc5579
Fix HTTPS-First "Downgraded on Timer" Telemetry r=freddyb,necko-reviewers,kershaw

Comment 4

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/a5844bfc5579

Comment 5

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

The patch landed in nightly and beta is affected.
:mjurgens, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox125 to wontfix.

For more information, please visit BugBot documentation.

Comment 6

[Malte Jürgens [:maltejur]]

Attachment: Bug 1885893 - Only collect HTTPS-First telemetry on successful request r?freddyb!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c58f1ec47e2e
Only collect HTTPS-First telemetry on successful request r=freddyb

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c58f1ec47e2e

Comment 9

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 10

[Pascal Chevrel:pascalc]

Is that worth uplifting to our last betas or can it ride the 128 train? Thanks

Comment 11

[Malte Jürgens [:maltejur]]

Attachment: Bug 1885893 - Only collect HTTPS-First telemetry on successful request

This patch addresses the problem that we currently collect HTTPS-First telemetry
for sites that are not reachable at all, be it through always causing a error or
through always timing out.

• On a downgrade, do not collect telemetry instantly, but instead save the
  telemetry data in the load state for the downgraded request
• That telemetry data will then be copied over into the document load listener
  of the new request
• On a successful request, if we have downgrade data in the load listener, we
  collect the downgrade telemetry, as the downgrade seems to have been
  successful
• Similar to the downgrade case, we only count the upgrade metric once we
  encounter a successful request annotated with the information that it was
  upgraded by HTTPS-First, instead of counting it instantly on the decision to
  upgrade. This also means the upgrade metric will not include loads that are
  downgraded again anymore
• Add a testcase for a site which is neither reachable via HTTP nor HTTPS, and
  ensure no telemetry is collected

Original Revision: https://phabricator.services.mozilla.com/D210792

Comment 12

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: There will be inaccurate telemetry during the gradual rollout of schemeless HTTPS-First in 127, or we will have to delay the rollout by one release
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: -
• Risk associated with taking this patch: Low
• Explanation of risk level: This patch does not change any HTTPS-First behaviour, it only changes the point in time when HTTPS-First telemetry is being collected
• String changes made/needed: -
• Is Android affected?: no

Comment 13

[Pascal Chevrel:pascalc]

The uplift does not apply cleanly to the beta branch.
Please note that tomorrow we build our last beta at 1PM UTC. I would need to land this before if you want it in 127.

Comment 14

[Pascal Chevrel:pascalc]

S4, patch did not apply to the branch, no answer in the bug and we merge beta to release today to build our RC. Marking as fix-optional in case an updated patch is provided for a potential RC2 later this week or for the planned dot release mid-cycle.

Comment 15

[Malte Jürgens [:maltejur]]

If this could still make it into a potential RC2 or a dot release that would be great, but it is not critical. I have updated the patch, and it should apply cleanly now (I previously had some problems with moz-phab and a outdated local beta branch).

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/b8d39a885c81