1860038 - Enable network.dns.port_prefixed_qname_https_rr

Status: NEW

(Core :: Networking: DNS, defect, P2)

Description

[Valentin Gosu [:valentin] (he/him)]

When resolving port prefixed HTTPS records we use the port in the additional info, but that is not part of the record key.

https://searchfox.org/mozilla-central/rev/e37662380b7b5507732a4f37a17da1a2f7802c04/netwerk/protocol/http/HTTPSRecordResolver.cpp#52-53

dns->NewAdditionalInfo(""_ns, mConnInfo->OriginPort(),
                       getter_AddRefs(info));

Ideally, the caller would add the port to the name here and here instead.

Comment 1

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

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

:kershaw, since you are the author of the regressor, bug 1755902, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

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

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

Comment 3

[Valentin Gosu [:valentin] (he/him)]

I'll try to post a fix soon.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

I was looking at writing a test for this and realized that this is actually preffed off.
We'll turn it on in this bug instead.

Comment 5

[Valentin Gosu [:valentin] (he/him)]

I later realized that we change the host to include the port, and that is being used for keying, so there's no issue with

https://searchfox.org/mozilla-central/rev/d6576544301cacc0e393fbc919c53e4e6b0d46ec/netwerk/dns/nsHostResolver.cpp#468-474

if (StaticPrefs::network_dns_port_prefixed_qname_https_rr() &&
    type == nsIDNSService::RESOLVE_TYPE_HTTPSSVC && aPort != -1 &&
    aPort != 443) {
  originHost = Some(host);
  host = nsPrintfCString("_%d._https.%s", aPort, host.get());
  LOG(("  Using port prefixed host name [%s]", host.get()));
}
[...]
    nsHostKey key(host, aTrrServer, type, flags, af,
                  (aOriginAttributes.mPrivateBrowsingId > 0), originSuffix);

Where I think there might be issues is when cancelling the request for example here we should also pass the port.

I'll keep this bug open to flip the pref and enable the feature.