Closed Bug 1680605 Opened 4 years ago Closed 4 years ago

Make sure https upgrade with https rr is really working

Categories

(Core :: Networking: HTTP, task, P1)

Product:
Core
Component:
Networking: HTTP
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
firefox86 --- fixed

People

(Reporter: kershaw, Assigned: kershaw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

Bug 1680605 - Make https upgrade with https rr properly
47 bytes, text/x-phabricator-request
Details | Review

Our current implementation has something missed:

  • The check whether to upgrade to https is only donein the callback of NS_ShouldSecureUpgrade. The other case when willCallback is false is missed.
  • The code that deals with address record and https rr is in the same function nsHttpChannel::OnLookupComplete. We need to separate this into two different functions, since in the failure case, where the nsIDNSRecord is null, we can't tell if this callback is for address resolution or https rr.
Assignee: nobody → kershaw
Status: NEW → ASSIGNED
Blocks: httpssvc
Pushed by kjang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ed5db87da381 Make https upgrade with https rr properly r=necko-reviewers,dragana
Regressions: 1685169

Backed out for failures on test_httpssvc_https_upgrade.js and resource-timing.

backout: https://hg.mozilla.org/integration/autoland/rev/1e323e0a130c4d077551758e4d37e02531b16e7f

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=ed5db87da381a6557160fd0cc78f11682281c1b1&selectedTaskRun=LTUkIMZJQ-2b4F36A3sznw.0

Test failure on test_httpssvc_https_upgrade.js has initially failed on this push ^ as a Test-verify job, but it has failed on tier 1 on: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&searchStr=os%2Cx%2C10.14%2Cwebrender%2Copt%2Cxpcshell%2Ctests%2Ctest-macosx1014-64-qr%2Fopt-xpcshell-e10s%2Cx2&revision=fdbcd1cd62e215d916777f5ff305fbdb6c4da9ef&selectedTaskRun=dxjEIf5jQOq1pPbeZklidQ.0

failure logs:

[task 2021-01-05T20:11:04.515Z] 20:11:04 INFO - TEST-START | netwerk/test/unit/test_httpssvc_https_upgrade.js
[task 2021-01-05T20:11:07.200Z] 20:11:07 WARNING - TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_httpssvc_https_upgrade.js | xpcshell return code: 0
[task 2021-01-05T20:11:07.200Z] 20:11:07 INFO - TEST-INFO took 2680ms
[task 2021-01-05T20:11:07.200Z] 20:11:07 INFO - >>>>>>>
[task 2021-01-05T20:11:07.200Z] 20:11:07 INFO - PID 1546 | [Parent 1546, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp:2916
[task 2021-01-05T20:11:07.200Z] 20:11:07 INFO - TEST-PASS | netwerk/test/unit/test_httpssvc_https_upgrade.js | setup - [setup : 31] "50347" != null

[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - TEST-PASS | /resource-timing/resource_connection_reuse.html | There should be 2 PerformanceEntries
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - TEST-UNEXPECTED-FAIL | /resource-timing/resource_connection_reuse.html | connectStart and fetchStart should be the same - assert_equals: connectStart and fetchStart should be the same expected 48.14 but got 40.14
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - test_equals/<@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:40
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2044:25
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - test@http://web-platform.test:8000/resources/testharness.js:572:30
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - wp_test@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:57:9
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - test_equals@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:12
[task 2021-01-05T23:10:45.491Z] 23:10:45 INFO - onload_test@http://web-platform.test:8000/resource-timing/resource_connection_reuse.html:41:20
[task 2021-01-05T23:10:45.492Z] 23:10:45 INFO -

Flags: needinfo?(kershaw)
Pushed by kjang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b403ea4dbb86 Make https upgrade with https rr properly r=necko-reviewers,dragana

Backed out changeset b403ea4dbb86 (bug 1680605) for resource_connection_reuse.html failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=0db606883e8df09bebd3ddce87b985c0ac446e8e&tochange=7151cfdc78ba55efee78d43852093d1d5b587249&test_paths=resource-timing&selectedTaskRun=T18PS8m7Twi4QnauRbUTHw.0

Backout link: https://hg.mozilla.org/integration/autoland/rev/7151cfdc78ba55efee78d43852093d1d5b587249

Failure log: https://treeherder.mozilla.org/logviewer?job_id=326014985&repo=autoland&lineNumber=13709

[task 2021-01-06T21:42:16.781Z] 21:42:16     INFO - TEST-START | /resource-timing/resource_connection_reuse.html
[task 2021-01-06T21:42:16.782Z] 21:42:16     INFO - Closing window 133
[task 2021-01-06T21:42:16.820Z] 21:42:16     INFO - PID 5304 | ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
[task 2021-01-06T21:42:16.850Z] 21:42:16     INFO - PID 5304 | [Child 7984, Main Thread] WARNING: Extra shutdown CC: 'i < NORMAL_SHUTDOWN_COLLECTIONS', file /b[uiPalrdesnt /9w3o8r8k,er /IchPeckoDuts/gLec Bkoa/cxpckom/bgarseo/nsCundycle]C olleWctorA.cpp:3364R
[task 2021-01-06T21:42:16.851Z] 21:42:16     INFO - PID 5304 | NING: idb: '0.000000s: starting', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.853Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: cache: '0.000000s: starting', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.854Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: sdb: '0.000000s: starting', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.855Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: ls: '0.000000s: starting', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.856Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.000000s: Live database entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.858Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.000000s: gLiveDatabaseHashtable entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.859Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.000000s: Live database entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.860Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.000000s: gLiveDatabaseHashtable entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.862Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.016000s: Live database entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.863Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.016000s: gLiveDatabaseHashtable entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.864Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.016000s: Live database entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.865Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.016000s: Live database entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.866Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.016000s: gLiveDatabaseHashtable entry removed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.867Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: idb: '0.016000s: completed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.869Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Backgroun[Child]d  WA7984RNIN,G Main Thread] WARNING: NS_ENSURE_TRUE(Preferen:ces:: InitcSatche: '0.0a1ticMembe60r00ss(:) ) cfoampilleetde: d'f,il ef i/bleui l/budis/wlodrske/rw/cohecrkkouetrs//gcehckeock/omuotdusl/es/lgiebcpkor/efd/oPrmefe/reqnuceost.cpap:/A4c37t8o
[task 2021-01-06T21:42:16.869Z] 21:42:16     INFO - PID 5304 | rsParent.cpp:4124
[task 2021-01-06T21:42:16.870Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: sdb: '0.016000s: completed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.872Z] 21:42:16     INFO - PID 5304 | [Parent 9388, IPDL Background] WARNING: ls: '0.016000s: completed', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp:4124
[task 2021-01-06T21:42:16.915Z] 21:42:16     INFO - PID 5304 | [Child 8884, Main Thread] WARNING: Extra shutdown CC: 'i < NORMAL_SHUTDOWN_COLLECTIONS', file /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp:3364
[task 2021-01-06T21:42:16.928Z] 21:42:16     INFO - PID 5304 | [Child 8884, Main Thread] WARNING: NS_ENSURE_TRUE(Preferences::InitStaticMembers()) failed: file /builds/worker/checkouts/gecko/modules/libpref/Preferences.cpp:4378
[task 2021-01-06T21:42:17.077Z] 21:42:17     INFO - PID 5304 | [Parent 9388, Main Thread] WARNING: NS_ENSURE_TRUE(mDB) failed: file /builds/worker/checkouts/gecko/netwerk/cache/nsDiskCacheDeviceSQL.cpp:1276
[task 2021-01-06T21:42:17.081Z] 21:42:17     INFO - PID 5304 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
[task 2021-01-06T21:42:17.095Z] 21:42:17     INFO - PID 5304 | [GPU 1236, Main Thread] WARNING: NS_ENSURE_TRUE(Preferences::InitStaticMembers()) failed: file /builds/worker/checkouts/gecko/modules/libpref/Preferences.cpp:4378
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - 
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - TEST-PASS | /resource-timing/resource_connection_reuse.html | window.performance is defined 
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - TEST-PASS | /resource-timing/resource_connection_reuse.html | window.performance.getEntriesByType is defined 
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - TEST-PASS | /resource-timing/resource_connection_reuse.html | There should be 2 PerformanceEntries 
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - TEST-UNEXPECTED-FAIL | /resource-timing/resource_connection_reuse.html | connectStart and fetchStart should be the same - assert_equals: connectStart and fetchStart should be the same expected 31.24 but got 29.44
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - test_equals/<@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:40
[task 2021-01-06T21:42:17.124Z] 21:42:17     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2044:25
[task 2021-01-06T21:42:17.125Z] 21:42:17     INFO - test@http://web-platform.test:8000/resources/testharness.js:572:30
[task 2021-01-06T21:42:17.125Z] 21:42:17     INFO - wp_test@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:57:9
[task 2021-01-06T21:42:17.125Z] 21:42:17     INFO - test_equals@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:12
[task 2021-01-06T21:42:17.125Z] 21:42:17     INFO - onload_test@http://web-platform.test:8000/resource-timing/resource_connection_reuse.html:41:20
[task 2021-01-06T21:42:17.127Z] 21:42:17     INFO - 
[task 2021-01-06T21:42:17.127Z] 21:42:17     INFO - TEST-UNEXPECTED-FAIL | /resource-timing/resource_connection_reuse.html | connectEnd and fetchStart should be the same - assert_equals: connectEnd and fetchStart should be the same expected 31.24 but got 29.44
[task 2021-01-06T21:42:17.127Z] 21:42:17     INFO - test_equals/<@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:40
[task 2021-01-06T21:42:17.127Z] 21:42:17     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2044:25
[task 2021-01-06T21:42:17.127Z] 21:42:17     INFO - test@http://web-platform.test:8000/resources/testharness.js:572:30
[task 2021-01-06T21:42:17.127Z] 21:42:17     INFO - wp_test@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:57:9
[task 2021-01-06T21:42:17.128Z] 21:42:17     INFO - test_equals@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:12
[task 2021-01-06T21:42:17.128Z] 21:42:17     INFO - onload_test@http://web-platform.test:8000/resource-timing/resource_connection_reuse.html:42:20
[task 2021-01-06T21:42:17.130Z] 21:42:17     INFO - 
[task 2021-01-06T21:42:17.130Z] 21:42:17     INFO - TEST-PASS | /resource-timing/resource_connection_reuse.html | secureConnectionStart should be zero 
[task 2021-01-06T21:42:17.130Z] 21:42:17     INFO - TEST-UNEXPECTED-FAIL | /resource-timing/resource_connection_reuse.html | domainLookupStart and fetchStart should be the same - assert_equals: domainLookupStart and fetchStart should be the same expected 30.62 but got 29.44
[task 2021-01-06T21:42:17.130Z] 21:42:17     INFO - test_equals/<@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:40
[task 2021-01-06T21:42:17.130Z] 21:42:17     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2044:25
[task 2021-01-06T21:42:17.130Z] 21:42:17     INFO - test@http://web-platform.test:8000/resources/testharness.js:572:30
[task 2021-01-06T21:42:17.131Z] 21:42:17     INFO - wp_test@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:57:9
[task 2021-01-06T21:42:17.131Z] 21:42:17     INFO - test_equals@http://web-platform.test:8000/resource-timing/resources/webperftestharness.js:136:12
[task 2021-01-06T21:42:17.131Z] 21:42:17     INFO - onload_test@http://web-platform.test:8000/resource-timing/resource_connection_reuse.html:46:20
[task 2021-01-06T21:42:17.132Z] 21:42:17     INFO - 
...
Flags: needinfo?(kershaw)
Pushed by kjang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c6fec8df79ce Make https upgrade with https rr properly r=necko-reviewers,dragana

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

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox86: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: