Fix native HTTPS RR resolution
Categories
(Core :: Networking: DNS, defect, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox124 | --- | fixed |
People
(Reporter: valentin, Assigned: valentin)
References
Details
(Whiteboard: [necko-triaged])
Attachments
(3 files)
A native HTTPS RR resolution may be removed from the queue but not be dispatched - which leads to the request hanging forever.
if (mActiveAnyThreadCount < MaxResolverThreadsAnyPriority()) {
rec = mQueue.Dequeue(false, lock);
RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
if (addrRec) {
MOZ_ASSERT(IsMediumPriority(addrRec->flags) ||
IsLowPriority(addrRec->flags));
mActiveAnyThreadCount++;
addrRec->StoreUsingAnyThread(true);
SET_GET_TTL(addrRec, true);
addrRec.forget(result);
return true;
}
// We don't need to QI to AddrHostRecord, since ByType records can also be dispatched.
|
Assignee | |
Comment 1•4 months ago
|
||
The purpose of this pref is to avoid breaking any tests that don't expect a
HTTPS record to be present. For example, if you're loading http://domain.com
in a test, and don't expect a HTTPS upgrade to happen, if that domain suddenly
adds a HTTPS record we might end up upgrading to HTTPS even in automation.
This pref does an early return with NS_ERROR_UNKNOWN_DOMAIN just before doing
a call to the system API. This means we're still waiting in the DNS queue to
resolve the domain (simulating the same waiting times we might see
on users' computers), but we never actually use native DNS to resolve a HTTPS
record in automation.
|
|
Assignee | |
Comment 2•4 months ago
|
||
Before this change, we would have removed the record from the queue,
it would have not QI'd to a AddrHostRecord so we would not have returned it.
That meant the DNS resolution never completed, and blocked the request
forever.
|
|
Assignee | |
Comment 3•4 months ago
|
||
This is a workaround for bug 1878505.
The problem is that when speculative connections are enabled, the
SpeculativeTransaction will call FetchHTTPSRR. This ends up resolving after
the real connection has been established, leading to an extra connection.
This seems incorrect.
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9737908480c6 Add pref to make native HTTPS resolution exit early in automation r=necko-reviewers,kershaw https://hg.mozilla.org/integration/autoland/rev/8e10457b7ccc Don't remove DNS record from queue without resolving r=necko-reviewers,kershaw https://hg.mozilla.org/integration/autoland/rev/c5320acccd4a Disable speculative connections in test_verify_traffic.js r=necko-reviewers,kershaw
|
||
Comment 5•4 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/9737908480c6
https://hg.mozilla.org/mozilla-central/rev/8e10457b7ccc
https://hg.mozilla.org/mozilla-central/rev/c5320acccd4a
Description
•