From 93ec4f0381e5a30076a465aec3d18c05f3c9b1d6 Mon Sep 17 00:00:00 2001 From: Mihai Alexandru Michis Date: Thu, 19 Mar 2020 17:12:32 +0200 Subject: [PATCH] Backed out changeset f01596089356 (bug 1220810) for causing crashes in test_performance_attributes_exist_in_object.html CLOSED TREE --- dom/base/test/unit/test_error_codes.js | 3 - dom/media/tests/mochitest/mochitest.ini | 2 - dom/performance/PerformanceTiming.cpp | 12 - dom/security/nsMixedContentBlocker.cpp | 8 +- dom/security/nsMixedContentBlocker.h | 5 - dom/security/test/gtest/TestSecureContext.cpp | 24 +- .../test_isOriginPotentiallyTrustworthy.js | 1 - .../browser/browser_webauthn_ipaddress.js | 1 - modules/libpref/init/StaticPrefList.yaml | 6 - modules/libpref/init/all.js | 2 + netwerk/base/nsProtocolProxyService.cpp | 17 +- netwerk/base/nsProtocolProxyService.h | 1 + netwerk/dns/DNS.cpp | 47 +- netwerk/dns/DNS.h | 4 - netwerk/dns/nsHostResolver.cpp | 542 ++++++++---------- netwerk/dns/nsHostResolver.h | 8 - netwerk/test/unit/test_about_networking.js | 4 - netwerk/test/unit/test_dns_offline.js | 3 - .../test/unit/test_dns_originAttributes.js | 5 - .../test/unit/test_ping_aboutnetworking.js | 2 - netwerk/test/unit/test_trr.js | 2 - 21 files changed, 277 insertions(+), 422 deletions(-) diff --git a/dom/base/test/unit/test_error_codes.js b/dom/base/test/unit/test_error_codes.js index 2a963d6aa5e6..a06ebfd9aace 100644 --- a/dom/base/test/unit/test_error_codes.js +++ b/dom/base/test/unit/test_error_codes.js @@ -38,8 +38,6 @@ function run_test_pt1() { } catch (e) {} Services.io.offline = true; prefs.setBoolPref("network.dns.offline-localhost", false); - // We always resolve localhost as it's hardcoded without the following pref: - prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); gExpectedStatus = Cr.NS_ERROR_OFFLINE; gNextTestFunc = run_test_pt2; @@ -51,7 +49,6 @@ function run_test_pt1() { function run_test_pt2() { Services.io.offline = false; prefs.clearUserPref("network.dns.offline-localhost"); - prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); gExpectedStatus = Cr.NS_ERROR_CONNECTION_REFUSED; gNextTestFunc = end_test; diff --git a/dom/media/tests/mochitest/mochitest.ini b/dom/media/tests/mochitest/mochitest.ini index 428ae46a54f1..4dd01c8bb29b 100644 --- a/dom/media/tests/mochitest/mochitest.ini +++ b/dom/media/tests/mochitest/mochitest.ini @@ -2,8 +2,6 @@ tags = mtg webrtc subsuite = media scheme = https -prefs = - network.proxy.allow_hijacking_localhost=true support-files = head.js dataChannel.js diff --git a/dom/performance/PerformanceTiming.cpp b/dom/performance/PerformanceTiming.cpp index 43a4a1f49963..30694e769f87 100644 --- a/dom/performance/PerformanceTiming.cpp +++ b/dom/performance/PerformanceTiming.cpp @@ -87,18 +87,6 @@ PerformanceTiming::PerformanceTiming(Performance* aPerformance, : nsRFPService::ReduceTimePrecisionAsMSecs( aZeroTime, aPerformance->GetRandomTimelineSeed()))); -#ifdef DEBUG - if (mTimingData->ResponseStartHighRes(aPerformance) - - mTimingData->ZeroTime() < - 0) { - MOZ_CRASH_UNSAFE_PRINTF( - "Heisenbug Reproduced: Please file line in 1436778. %s %f - %f (%f)", - (aPerformance->IsSystemPrincipal() ? "System" : "Not-System"), - mTimingData->ResponseStartHighRes(aPerformance), - mTimingData->ZeroTime(), aZeroTime); - } -#endif - // Non-null aHttpChannel implies that this PerformanceTiming object is being // used for subresources, which is irrelevant to this probe. if (!aHttpChannel && StaticPrefs::dom_enable_performance() && diff --git a/dom/security/nsMixedContentBlocker.cpp b/dom/security/nsMixedContentBlocker.cpp index d49a812c28ff..c4b67750f47f 100644 --- a/dom/security/nsMixedContentBlocker.cpp +++ b/dom/security/nsMixedContentBlocker.cpp @@ -379,7 +379,8 @@ nsMixedContentBlocker::ShouldLoad(nsIURI* aContentLocation, bool nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackHost( const nsACString& aAsciiHost) { - if (mozilla::net::IsLoopbackHostname(aAsciiHost)) { + if (aAsciiHost.EqualsLiteral("::1") || + aAsciiHost.EqualsLiteral("localhost")) { return true; } @@ -399,8 +400,9 @@ bool nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackHost( // https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy says // we should only consider [::1]/128 as a potentially trustworthy IPv6 // address, whereas for IPv4 127.0.0.1/8 are considered as potentially - // trustworthy. - return IsLoopBackAddressWithoutIPv6Mapping(&addr); + // trustworthy. We already handled "[::1]" above, so all that's remained to + // handle here are IPv4 loopback addresses. + return IsIPAddrV4(&addr) && IsLoopBackAddress(&addr); } bool nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackURL(nsIURI* aURL) { diff --git a/dom/security/nsMixedContentBlocker.h b/dom/security/nsMixedContentBlocker.h index 54e97fe6607a..9997a4cf2a24 100644 --- a/dom/security/nsMixedContentBlocker.h +++ b/dom/security/nsMixedContentBlocker.h @@ -34,11 +34,6 @@ enum MixedContentTypes { using mozilla::OriginAttributes; class nsILoadInfo; // forward declaration -namespace mozilla { -namespace net { -class nsProtocolProxyService; // forward declaration -} -} // namespace mozilla class nsMixedContentBlocker : public nsIContentPolicy, public nsIChannelEventSink { diff --git a/dom/security/test/gtest/TestSecureContext.cpp b/dom/security/test/gtest/TestSecureContext.cpp index 3156139fa064..f971963d9e7d 100644 --- a/dom/security/test/gtest/TestSecureContext.cpp +++ b/dom/security/test/gtest/TestSecureContext.cpp @@ -24,29 +24,12 @@ struct TestExpectations { bool expectedResult; }; -class MOZ_RAII AutoRestoreBoolPref final { - public: - AutoRestoreBoolPref(const char* aPref, bool aValue) : mPref(aPref) { - Preferences::GetBool(mPref, &mOldValue); - Preferences::SetBool(mPref, aValue); - } - - ~AutoRestoreBoolPref() { Preferences::SetBool(mPref, mOldValue); } - - private: - const char* mPref = nullptr; - bool mOldValue = false; -}; - // ============================= TestDirectives ======================== TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) { // boolean isOriginPotentiallyTrustworthy(in nsIPrincipal aPrincipal); - AutoRestoreBoolPref savedPref("network.proxy.allow_hijacking_localhost", - false); - static const TestExpectations uris[] = { {"http://example.com/", false}, {"https://example.com/", true}, @@ -56,9 +39,7 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) {"ftp://example.com", false}, {"about:config", false}, {"http://localhost", true}, - {"http://localhost.localhost", true}, - {"http://a.b.c.d.e.localhost", true}, - {"http://xyzzy.localhost", true}, + {"http://xyzzy.localhost", false}, {"http://127.0.0.1", true}, {"http://127.0.0.2", true}, {"http://127.1.0.1", true}, @@ -91,8 +72,7 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) bool isPotentiallyTrustworthy = false; rv = prin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy); ASSERT_EQ(NS_OK, rv); - ASSERT_EQ(isPotentiallyTrustworthy, uris[i].expectedResult) - << uris[i].uri << uris[i].expectedResult; + ASSERT_EQ(isPotentiallyTrustworthy, uris[i].expectedResult); } } diff --git a/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js index 9286470f82ab..427543a108a3 100644 --- a/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js +++ b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js @@ -36,7 +36,6 @@ add_task(async function test_isOriginPotentiallyTrustworthy() { ["http://example.com/", false], ["https://example.com/", true], ["http://localhost/", true], - ["http://localhost.localhost/", true], ["http://127.0.0.1/", true], ["file:///", true], ["resource:///", true], diff --git a/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js b/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js index a7d85ba8ac59..c19e372e37d6 100644 --- a/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js +++ b/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js @@ -21,7 +21,6 @@ add_task(function test_setup() { "security.webauth.webauthn_enable_usbtoken", false ); - Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); }); registerCleanupFunction(async function() { diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 9b0b31dd8eee..bc9cc3075498 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -7231,12 +7231,6 @@ value: true mirror: always -# Set true to allow resolving proxy for localhost -- name: network.proxy.allow_hijacking_localhost - type: RelaxedAtomicBool - value: false - mirror: always - # Allow CookieJarSettings to be unblocked for channels without a document. # This is for testing only. - name: network.cookieJarSettings.unblocked_for_testing diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index c5d79e333ca5..b911c1200c89 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1978,6 +1978,8 @@ pref("network.proxy.socks_port", 0); pref("network.proxy.socks_version", 5); pref("network.proxy.proxy_over_tls", true); pref("network.proxy.no_proxies_on", ""); +// Set true to allow resolving proxy for localhost +pref("network.proxy.allow_hijacking_localhost", false); pref("network.proxy.failover_timeout", 1800); // 30 minutes pref("network.online", true); //online/offline diff --git a/netwerk/base/nsProtocolProxyService.cpp b/netwerk/base/nsProtocolProxyService.cpp index 97961eaac47f..f163d522a98a 100644 --- a/netwerk/base/nsProtocolProxyService.cpp +++ b/netwerk/base/nsProtocolProxyService.cpp @@ -36,9 +36,7 @@ #include "nsISystemProxySettings.h" #include "nsINetworkLinkService.h" #include "nsIHttpChannelInternal.h" -#include "mozilla/dom/nsMixedContentBlocker.h" #include "mozilla/Logging.h" -#include "mozilla/StaticPrefs_network.h" #include "mozilla/Tokenizer.h" #include "mozilla/Unused.h" @@ -772,6 +770,7 @@ nsProtocolProxyService::nsProtocolProxyService() mSOCKSProxyRemoteDNS(false), mProxyOverTLS(true), mWPADOverDHCPEnabled(false), + mAllowHijackingLocalhost(false), mPACMan(nullptr), mSessionStart(PR_Now()), mFailedProxyTimeout(30 * 60) // 30 minute default @@ -1019,6 +1018,11 @@ void nsProtocolProxyService::PrefsChanged(nsIPrefBranch* prefBranch, reloadPAC = reloadPAC || mProxyConfig == PROXYCONFIG_WPAD; } + if (!pref || !strcmp(pref, PROXY_PREF("allow_hijacking_localhost"))) { + proxy_GetBoolPref(prefBranch, PROXY_PREF("allow_hijacking_localhost"), + mAllowHijackingLocalhost); + } + if (!pref || !strcmp(pref, PROXY_PREF("failover_timeout"))) proxy_GetIntPref(prefBranch, PROXY_PREF("failover_timeout"), mFailedProxyTimeout); @@ -1092,12 +1096,9 @@ bool nsProtocolProxyService::CanUseProxy(nsIURI* aURI, int32_t defaultPort) { // Don't use proxy for local hosts (plain hostname, no dots) if ((!is_ipaddr && mFilterLocalHosts && !host.Contains('.')) || - // This method detects if we have network.proxy.allow_hijacking_localhost - // pref enabled. If it's true then this method will always return false - // otherwise it returns true if the host matches an address that's - // hardcoded to the loopback address. - (!StaticPrefs::network_proxy_allow_hijacking_localhost() && - nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackHost(host))) { + (!mAllowHijackingLocalhost && + (host.EqualsLiteral("127.0.0.1") || host.EqualsLiteral("::1") || + host.EqualsLiteral("localhost")))) { LOG(("Not using proxy for this local host [%s]!\n", host.get())); return false; // don't allow proxying } diff --git a/netwerk/base/nsProtocolProxyService.h b/netwerk/base/nsProtocolProxyService.h index e52c0d34600b..feab7132a7a7 100644 --- a/netwerk/base/nsProtocolProxyService.h +++ b/netwerk/base/nsProtocolProxyService.h @@ -387,6 +387,7 @@ class nsProtocolProxyService final : public nsIProtocolProxyService2, bool mSOCKSProxyRemoteDNS; bool mProxyOverTLS; bool mWPADOverDHCPEnabled; + bool mAllowHijackingLocalhost; RefPtr mPACMan; // non-null if we are using PAC nsCOMPtr mSystemProxySettings; diff --git a/netwerk/dns/DNS.cpp b/netwerk/dns/DNS.cpp index 0c2126c8e7f6..fad9d4f48abb 100644 --- a/netwerk/dns/DNS.cpp +++ b/netwerk/dns/DNS.cpp @@ -6,11 +6,9 @@ #include "mozilla/net/DNS.h" -#include "mozilla/ArrayUtils.h" #include "mozilla/Assertions.h" #include "mozilla/mozalloc.h" -#include "mozilla/StaticPrefs_network.h" -#include "nsContentUtils.h" +#include "mozilla/ArrayUtils.h" #include "nsString.h" #include @@ -141,46 +139,21 @@ bool NetAddrToString(const NetAddr* addr, char* buf, uint32_t bufSize) { } bool IsLoopBackAddress(const NetAddr* addr) { - if (IsLoopBackAddressWithoutIPv6Mapping(addr)) { - return true; - } - if (addr->raw.family != AF_INET6) { - return false; - } - - return IPv6ADDR_IS_V4MAPPED(&addr->inet6.ip) && - IPv6ADDR_V4MAPPED_TO_IPADDR(&addr->inet6.ip) == htonl(INADDR_LOOPBACK); -} - -bool IsLoopBackAddressWithoutIPv6Mapping(const NetAddr* addr) { if (addr->raw.family == AF_INET) { // Consider 127.0.0.1/8 as loopback uint32_t ipv4Addr = ntohl(addr->inet.ip); return (ipv4Addr >> 24) == 127; } - - if (addr->raw.family == AF_INET6 && IPv6ADDR_IS_LOOPBACK(&addr->inet6.ip)) { - return true; + if (addr->raw.family == AF_INET6) { + if (IPv6ADDR_IS_LOOPBACK(&addr->inet6.ip)) { + return true; + } + if (IPv6ADDR_IS_V4MAPPED(&addr->inet6.ip) && + IPv6ADDR_V4MAPPED_TO_IPADDR(&addr->inet6.ip) == + htonl(INADDR_LOOPBACK)) { + return true; + } } - - return false; -} - -bool IsLoopbackHostname(const nsACString& aAsciiHost) { - // If the user has configured to proxy localhost addresses don't consider them - // to be secure - if (StaticPrefs::network_proxy_allow_hijacking_localhost()) { - return false; - } - - nsAutoCString host; - nsContentUtils::ASCIIToLower(aAsciiHost, host); - - if (host.EqualsLiteral("localhost") || - StringEndsWith(host, NS_LITERAL_CSTRING(".localhost"))) { - return true; - } - return false; } diff --git a/netwerk/dns/DNS.h b/netwerk/dns/DNS.h index 0867ec8099ca..b81b5e44fdd8 100644 --- a/netwerk/dns/DNS.h +++ b/netwerk/dns/DNS.h @@ -182,10 +182,6 @@ bool NetAddrToString(const NetAddr* addr, char* buf, uint32_t bufSize); bool IsLoopBackAddress(const NetAddr* addr); -bool IsLoopBackAddressWithoutIPv6Mapping(const NetAddr* addr); - -bool IsLoopbackHostname(const nsACString& aAsciiHost); - bool IsIPAddrAny(const NetAddr* addr); bool IsIPAddrV4(const NetAddr* addr); diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp index 98f0f241436f..7edba72f429f 100644 --- a/netwerk/dns/nsHostResolver.cpp +++ b/netwerk/dns/nsHostResolver.cpp @@ -784,7 +784,11 @@ nsresult nsHostResolver::GetHostRecord(const nsACString& host, RefPtr& entry = mRecordDB.GetOrInsert(key); if (!entry) { - entry = InitRecord(key); + if (IS_ADDR_TYPE(type)) { + entry = new AddrHostRecord(key); + } else { + entry = new TypeHostRecord(key); + } } RefPtr rec = entry; @@ -803,48 +807,6 @@ nsresult nsHostResolver::GetHostRecord(const nsACString& host, return NS_OK; } -nsHostRecord* nsHostResolver::InitRecord(const nsHostKey& key) { - if (IS_ADDR_TYPE(key.type)) { - return new AddrHostRecord(key); - } - return new TypeHostRecord(key); -} - -already_AddRefed nsHostResolver::InitLoopbackRecord( - const nsHostKey& key, nsresult* aRv) { - MOZ_ASSERT(aRv); - MOZ_ASSERT(IS_ADDR_TYPE(key.type)); - - *aRv = NS_ERROR_FAILURE; - RefPtr rec = InitRecord(key); - - RefPtr addrRec = do_QueryObject(rec); - MutexAutoLock lock(addrRec->addr_info_lock); - - PRNetAddr prAddr; - - if (key.af == PR_AF_INET) { - MOZ_RELEASE_ASSERT(PR_StringToNetAddr("127.0.0.1", &prAddr) == PR_SUCCESS); - } else { - MOZ_RELEASE_ASSERT(PR_StringToNetAddr("::1", &prAddr) == PR_SUCCESS); - } - - RefPtr ai; - *aRv = GetAddrInfo(rec->host, rec->af, addrRec->flags, getter_AddRefs(ai), - addrRec->mGetTtl); - if (NS_WARN_IF(NS_FAILED(*aRv))) { - return nullptr; - } - - addrRec->addr_info = ai; - addrRec->SetExpiration(TimeStamp::NowLoRes(), mDefaultCacheLifetime, - mDefaultGracePeriod); - addrRec->negative = false; - - *aRv = NS_OK; - return rec.forget(); -} - nsresult nsHostResolver::ResolveHost(const nsACString& aHost, const nsACString& aTrrServer, uint16_t type, @@ -890,276 +852,268 @@ nsresult nsHostResolver::ResolveHost(const nsACString& aHost, MutexAutoLock lock(mLock); if (mShutdown) { - return NS_ERROR_NOT_INITIALIZED; - } + rv = NS_ERROR_NOT_INITIALIZED; + } else { + // check to see if there is already an entry for this |host| + // in the hash table. if so, then check to see if we can't + // just reuse the lookup result. otherwise, if there are + // any pending callbacks, then add to pending callbacks queue, + // and return. otherwise, add ourselves as first pending + // callback, and proceed to do the lookup. + nsAutoCString originSuffix; + aOriginAttributes.CreateSuffix(originSuffix); - // check to see if there is already an entry for this |host| - // in the hash table. if so, then check to see if we can't - // just reuse the lookup result. otherwise, if there are - // any pending callbacks, then add to pending callbacks queue, - // and return. otherwise, add ourselves as first pending - // callback, and proceed to do the lookup. - nsAutoCString originSuffix; - aOriginAttributes.CreateSuffix(originSuffix); + if (gTRRService && gTRRService->IsExcludedFromTRR(host)) { + flags |= RES_DISABLE_TRR; - if (gTRRService && gTRRService->IsExcludedFromTRR(host)) { - flags |= RES_DISABLE_TRR; - - if (!aTrrServer.IsEmpty()) { - return NS_ERROR_UNKNOWN_HOST; - } - } - - nsHostKey key(host, aTrrServer, type, flags, af, - (aOriginAttributes.mPrivateBrowsingId > 0), originSuffix); - - // Check if we have a localhost domain, if so hardcode to loopback - if (IS_ADDR_TYPE(type) && IsLoopbackHostname(host)) { - nsresult rv; - RefPtr result = InitLoopbackRecord(key, &rv); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - MOZ_ASSERT(result); - aCallback->OnResolveHostComplete(this, result, NS_OK); - return NS_OK; - } - - RefPtr& entry = mRecordDB.GetOrInsert(key); - if (!entry) { - entry = InitRecord(key); - } - - RefPtr rec = entry; - RefPtr addrRec = do_QueryObject(rec); - MOZ_ASSERT(rec, "Record should not be null"); - MOZ_ASSERT((IS_ADDR_TYPE(type) && rec->IsAddrRecord() && addrRec) || - (IS_OTHER_TYPE(type) && !rec->IsAddrRecord())); - - if (!(flags & RES_BYPASS_CACHE) && - rec->HasUsableResult(TimeStamp::NowLoRes(), flags)) { - LOG((" Using cached record for host [%s].\n", host.get())); - // put reference to host record on stack... - result = rec; - if (IS_ADDR_TYPE(type)) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); - } - - // For entries that are in the grace period - // or all cached negative entries, use the cache but start a new - // lookup in the background - ConditionallyRefreshRecord(rec, host); - - if (rec->negative) { - LOG((" Negative cache entry for host [%s].\n", host.get())); - if (IS_ADDR_TYPE(type)) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, - METHOD_NEGATIVE_HIT); + if (!aTrrServer.IsEmpty()) { + return NS_ERROR_UNKNOWN_HOST; } - status = NS_ERROR_UNKNOWN_HOST; } - // Check whether host is a IP address for A/AAAA queries. - // For by-type records we have already checked at the beginning of - // this function. - } else if (addrRec && addrRec->addr) { - // if the host name is an IP address literal and has been - // parsed, go ahead and use it. - LOG((" Using cached address for IP Literal [%s].\n", host.get())); - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); - result = rec; - } else if (addrRec && - PR_StringToNetAddr(host.get(), &tempAddr) == PR_SUCCESS) { - // try parsing the host name as an IP address literal to short - // circuit full host resolution. (this is necessary on some - // platforms like Win9x. see bug 219376 for more details.) - LOG((" Host is IP Literal [%s].\n", host.get())); - - // ok, just copy the result into the host record, and be - // done with it! ;-) - addrRec->addr = MakeUnique(); - PRNetAddrToNetAddr(&tempAddr, addrRec->addr.get()); - // put reference to host record on stack... - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); - result = rec; - - // Check if we have received too many requests. - } else if (mPendingCount >= MAX_NON_PRIORITY_REQUESTS && - !IsHighPriority(flags) && !rec->mResolving) { - LOG( - (" Lookup queue full: dropping %s priority request for " - "host [%s].\n", - IsMediumPriority(flags) ? "medium" : "low", host.get())); - if (IS_ADDR_TYPE(type)) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_OVERFLOW); + nsHostKey key(host, aTrrServer, type, flags, af, + (aOriginAttributes.mPrivateBrowsingId > 0), originSuffix); + RefPtr& entry = mRecordDB.GetOrInsert(key); + if (!entry) { + if (IS_ADDR_TYPE(type)) { + entry = new AddrHostRecord(key); + } else { + entry = new TypeHostRecord(key); + } } - // This is a lower priority request and we are swamped, so refuse it. - rv = NS_ERROR_DNS_LOOKUP_QUEUE_FULL; - // Check if the offline flag is set. - } else if (flags & RES_OFFLINE) { - LOG((" Offline request for host [%s]; ignoring.\n", host.get())); - rv = NS_ERROR_OFFLINE; + RefPtr rec = entry; + RefPtr addrRec = do_QueryObject(rec); + MOZ_ASSERT(rec, "Record should not be null"); + MOZ_ASSERT((IS_ADDR_TYPE(type) && rec->IsAddrRecord() && addrRec) || + (IS_OTHER_TYPE(type) && !rec->IsAddrRecord())); - // We do not have a valid result till here. - // A/AAAA request can check for an alternative entry like AF_UNSPEC. - // Otherwise we need to start a new query. - } else if (!rec->mResolving) { - // If this is an IPV4 or IPV6 specific request, check if there is - // an AF_UNSPEC entry we can use. Otherwise, hit the resolver... - if (addrRec && !(flags & RES_BYPASS_CACHE) && - ((af == PR_AF_INET) || (af == PR_AF_INET6))) { - // Check for an AF_UNSPEC entry. + // Check if the entry is vaild. + if (!(flags & RES_BYPASS_CACHE) && + rec->HasUsableResult(TimeStamp::NowLoRes(), flags)) { + LOG((" Using cached record for host [%s].\n", host.get())); + // put reference to host record on stack... + result = rec; + if (IS_ADDR_TYPE(type)) { + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); + } - const nsHostKey unspecKey( - host, aTrrServer, nsIDNSService::RESOLVE_TYPE_DEFAULT, flags, - PR_AF_UNSPEC, (aOriginAttributes.mPrivateBrowsingId > 0), - originSuffix); - RefPtr unspecRec = mRecordDB.Get(unspecKey); + // For entries that are in the grace period + // or all cached negative entries, use the cache but start a new + // lookup in the background + ConditionallyRefreshRecord(rec, host); - TimeStamp now = TimeStamp::NowLoRes(); - if (unspecRec && unspecRec->HasUsableResult(now, flags)) { - MOZ_ASSERT(unspecRec->IsAddrRecord()); - - RefPtr addrUnspecRec = do_QueryObject(unspecRec); - MOZ_ASSERT(addrUnspecRec); - MOZ_ASSERT(addrUnspecRec->addr_info || addrUnspecRec->negative, - "Entry should be resolved or negative."); - - LOG((" Trying AF_UNSPEC entry for host [%s] af: %s.\n", host.get(), - (af == PR_AF_INET) ? "AF_INET" : "AF_INET6")); - - // We need to lock in case any other thread is reading - // addr_info. - MutexAutoLock lock(addrRec->addr_info_lock); - - addrRec->addr_info = nullptr; - addrRec->addr_info_gencnt++; - if (unspecRec->negative) { - rec->negative = unspecRec->negative; - rec->CopyExpirationTimesAndFlagsFrom(unspecRec); - } else if (addrUnspecRec->addr_info) { - // Search for any valid address in the AF_UNSPEC entry - // in the cache (not blacklisted and from the right - // family). - NetAddrElement* addrIter = - addrUnspecRec->addr_info->mAddresses.getFirst(); - while (addrIter) { - if ((af == addrIter->mAddress.inet.family) && - !addrUnspecRec->Blacklisted(&addrIter->mAddress)) { - if (!addrRec->addr_info) { - addrRec->addr_info = - new AddrInfo(addrUnspecRec->addr_info->mHostName, - addrUnspecRec->addr_info->mCanonicalName, - addrUnspecRec->addr_info->IsTRR()); - addrRec->addr_info_gencnt++; - rec->CopyExpirationTimesAndFlagsFrom(unspecRec); - } - addrRec->addr_info->AddAddress(new NetAddrElement(*addrIter)); - } - addrIter = addrIter->getNext(); - } - } - // Now check if we have a new record. - if (rec->HasUsableResult(now, flags)) { - result = rec; - if (rec->negative) { - status = NS_ERROR_UNKNOWN_HOST; - } - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); - ConditionallyRefreshRecord(rec, host); - } else if (af == PR_AF_INET6) { - // For AF_INET6, a new lookup means another AF_UNSPEC - // lookup. We have already iterated through the - // AF_UNSPEC addresses, so we mark this record as - // negative. - LOG( - (" No AF_INET6 in AF_UNSPEC entry: " - "host [%s] unknown host.", - host.get())); - result = rec; - rec->negative = true; - status = NS_ERROR_UNKNOWN_HOST; + if (rec->negative) { + LOG((" Negative cache entry for host [%s].\n", host.get())); + if (IS_ADDR_TYPE(type)) { Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_NEGATIVE_HIT); } - } - } - - // If this is a by-type request or if no valid record was found - // in the cache or this is an AF_UNSPEC request, then start a - // new lookup. - if (!result) { - LOG((" No usable record in cache for host [%s] type %d.", host.get(), - type)); - - if (flags & RES_REFRESH_CACHE) { - rec->Invalidate(); + status = NS_ERROR_UNKNOWN_HOST; } - // Add callback to the list of pending callbacks. - rec->mCallbacks.insertBack(callback); - rec->flags = flags; - rv = NameLookup(rec); + // Check whether host is a IP address for A/AAAA queries. + // For by-type records we have already checked at the beginning of + // this function. + } else if (addrRec && addrRec->addr) { + // if the host name is an IP address literal and has been + // parsed, go ahead and use it. + LOG((" Using cached address for IP Literal [%s].\n", host.get())); + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); + result = rec; + } else if (addrRec && + PR_StringToNetAddr(host.get(), &tempAddr) == PR_SUCCESS) { + // try parsing the host name as an IP address literal to short + // circuit full host resolution. (this is necessary on some + // platforms like Win9x. see bug 219376 for more details.) + LOG((" Host is IP Literal [%s].\n", host.get())); + + // ok, just copy the result into the host record, and be + // done with it! ;-) + addrRec->addr = MakeUnique(); + PRNetAddrToNetAddr(&tempAddr, addrRec->addr.get()); + // put reference to host record on stack... + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); + result = rec; + + // Check if we have received too many requests. + } else if (mPendingCount >= MAX_NON_PRIORITY_REQUESTS && + !IsHighPriority(flags) && !rec->mResolving) { + LOG( + (" Lookup queue full: dropping %s priority request for " + "host [%s].\n", + IsMediumPriority(flags) ? "medium" : "low", host.get())); if (IS_ADDR_TYPE(type)) { + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_OVERFLOW); + } + // This is a lower priority request and we are swamped, so refuse it. + rv = NS_ERROR_DNS_LOOKUP_QUEUE_FULL; + + // Check if the offline flag is set. + } else if (flags & RES_OFFLINE) { + LOG((" Offline request for host [%s]; ignoring.\n", host.get())); + rv = NS_ERROR_OFFLINE; + + // We do not have a valid result till here. + // A/AAAA request can check for an alternative entry like AF_UNSPEC. + // Otherwise we need to start a new query. + } else if (!rec->mResolving) { + // If this is an IPV4 or IPV6 specific request, check if there is + // an AF_UNSPEC entry we can use. Otherwise, hit the resolver... + if (addrRec && !(flags & RES_BYPASS_CACHE) && + ((af == PR_AF_INET) || (af == PR_AF_INET6))) { + // Check for an AF_UNSPEC entry. + + const nsHostKey unspecKey( + host, aTrrServer, nsIDNSService::RESOLVE_TYPE_DEFAULT, flags, + PR_AF_UNSPEC, (aOriginAttributes.mPrivateBrowsingId > 0), + originSuffix); + RefPtr unspecRec = mRecordDB.Get(unspecKey); + + TimeStamp now = TimeStamp::NowLoRes(); + if (unspecRec && unspecRec->HasUsableResult(now, flags)) { + MOZ_ASSERT(unspecRec->IsAddrRecord()); + + RefPtr addrUnspecRec = do_QueryObject(unspecRec); + MOZ_ASSERT(addrUnspecRec); + MOZ_ASSERT(addrUnspecRec->addr_info || addrUnspecRec->negative, + "Entry should be resolved or negative."); + + LOG((" Trying AF_UNSPEC entry for host [%s] af: %s.\n", host.get(), + (af == PR_AF_INET) ? "AF_INET" : "AF_INET6")); + + // We need to lock in case any other thread is reading + // addr_info. + MutexAutoLock lock(addrRec->addr_info_lock); + + addrRec->addr_info = nullptr; + addrRec->addr_info_gencnt++; + if (unspecRec->negative) { + rec->negative = unspecRec->negative; + rec->CopyExpirationTimesAndFlagsFrom(unspecRec); + } else if (addrUnspecRec->addr_info) { + // Search for any valid address in the AF_UNSPEC entry + // in the cache (not blacklisted and from the right + // family). + NetAddrElement* addrIter = + addrUnspecRec->addr_info->mAddresses.getFirst(); + while (addrIter) { + if ((af == addrIter->mAddress.inet.family) && + !addrUnspecRec->Blacklisted(&addrIter->mAddress)) { + if (!addrRec->addr_info) { + addrRec->addr_info = + new AddrInfo(addrUnspecRec->addr_info->mHostName, + addrUnspecRec->addr_info->mCanonicalName, + addrUnspecRec->addr_info->IsTRR()); + addrRec->addr_info_gencnt++; + rec->CopyExpirationTimesAndFlagsFrom(unspecRec); + } + addrRec->addr_info->AddAddress(new NetAddrElement(*addrIter)); + } + addrIter = addrIter->getNext(); + } + } + // Now check if we have a new record. + if (rec->HasUsableResult(now, flags)) { + result = rec; + if (rec->negative) { + status = NS_ERROR_UNKNOWN_HOST; + } + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); + ConditionallyRefreshRecord(rec, host); + } else if (af == PR_AF_INET6) { + // For AF_INET6, a new lookup means another AF_UNSPEC + // lookup. We have already iterated through the + // AF_UNSPEC addresses, so we mark this record as + // negative. + LOG( + (" No AF_INET6 in AF_UNSPEC entry: " + "host [%s] unknown host.", + host.get())); + result = rec; + rec->negative = true; + status = NS_ERROR_UNKNOWN_HOST; + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, + METHOD_NEGATIVE_HIT); + } + } + } + + // If this is a by-type request or if no valid record was found + // in the cache or this is an AF_UNSPEC request, then start a + // new lookup. + if (!result) { + LOG((" No usable record in cache for host [%s] type %d.", host.get(), + type)); + + if (flags & RES_REFRESH_CACHE) { + rec->Invalidate(); + } + + // Add callback to the list of pending callbacks. + rec->mCallbacks.insertBack(callback); + rec->flags = flags; + rv = NameLookup(rec); + if (IS_ADDR_TYPE(type)) { + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, + METHOD_NETWORK_FIRST); + } + if (NS_FAILED(rv) && callback->isInList()) { + callback->remove(); + } else { + LOG( + (" DNS lookup for host [%s] blocking " + "pending 'getaddrinfo' or trr query: " + "callback [%p]", + host.get(), callback.get())); + } + } + + } else if (addrRec && addrRec->mDidCallbacks) { + // This is only for A/AAAA query. + // record is still pending more (TRR) data; make the callback + // at once + result = rec; + // make it count as a hit + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); + + LOG((" Host [%s] re-using early TRR resolve data\n", host.get())); + } else { + LOG( + (" Host [%s] is being resolved. Appending callback " + "[%p].", + host.get(), callback.get())); + + rec->mCallbacks.insertBack(callback); + + // Only A/AAAA records are place in a queue. The queues are for + // the native resolver, therefore by-type request are never put + // into a queue. + if (addrRec && addrRec->onQueue) { Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, - METHOD_NETWORK_FIRST); - } - if (NS_FAILED(rv) && callback->isInList()) { - callback->remove(); - } else { - LOG( - (" DNS lookup for host [%s] blocking " - "pending 'getaddrinfo' or trr query: " - "callback [%p]", - host.get(), callback.get())); - } - } + METHOD_NETWORK_SHARED); - } else if (addrRec && addrRec->mDidCallbacks) { - // This is only for A/AAAA query. - // record is still pending more (TRR) data; make the callback - // at once - result = rec; - // make it count as a hit - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); + // Consider the case where we are on a pending queue of + // lower priority than the request is being made at. + // In that case we should upgrade to the higher queue. - LOG((" Host [%s] re-using early TRR resolve data\n", host.get())); - } else { - LOG( - (" Host [%s] is being resolved. Appending callback " - "[%p].", - host.get(), callback.get())); - - rec->mCallbacks.insertBack(callback); - - // Only A/AAAA records are place in a queue. The queues are for - // the native resolver, therefore by-type request are never put - // into a queue. - if (addrRec && addrRec->onQueue) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, - METHOD_NETWORK_SHARED); - - // Consider the case where we are on a pending queue of - // lower priority than the request is being made at. - // In that case we should upgrade to the higher queue. - - if (IsHighPriority(flags) && !IsHighPriority(rec->flags)) { - // Move from (low|med) to high. - NS_ASSERTION(addrRec->onQueue, - "Moving Host Record Not Currently Queued"); - rec->remove(); - mHighQ.insertBack(rec); - rec->flags = flags; - ConditionallyCreateThread(rec); - } else if (IsMediumPriority(flags) && IsLowPriority(rec->flags)) { - // Move from low to med. - NS_ASSERTION(addrRec->onQueue, - "Moving Host Record Not Currently Queued"); - rec->remove(); - mMediumQ.insertBack(rec); - rec->flags = flags; - mIdleTaskCV.Notify(); + if (IsHighPriority(flags) && !IsHighPriority(rec->flags)) { + // Move from (low|med) to high. + NS_ASSERTION(addrRec->onQueue, + "Moving Host Record Not Currently Queued"); + rec->remove(); + mHighQ.insertBack(rec); + rec->flags = flags; + ConditionallyCreateThread(rec); + } else if (IsMediumPriority(flags) && IsLowPriority(rec->flags)) { + // Move from low to med. + NS_ASSERTION(addrRec->onQueue, + "Moving Host Record Not Currently Queued"); + rec->remove(); + mMediumQ.insertBack(rec); + rec->flags = flags; + mIdleTaskCV.Notify(); + } } } } diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h index 440cff7c55b0..b5b549c37e82 100644 --- a/netwerk/dns/nsHostResolver.h +++ b/netwerk/dns/nsHostResolver.h @@ -440,14 +440,6 @@ class nsHostResolver : public nsISupports, public AHostResolver { uint16_t flags, uint16_t af, nsResolveHostCallback* callback); - nsHostRecord* InitRecord(const nsHostKey& key); - - /** - * return a resolved hard coded loopback dns record for the specified key - */ - already_AddRefed InitLoopbackRecord(const nsHostKey& key, - nsresult* aRv); - /** * removes the specified callback from the nsHostRecord for the given * hostname, originAttributes, flags, and address family. these parameters diff --git a/netwerk/test/unit/test_about_networking.js b/netwerk/test/unit/test_about_networking.js index c423cd23d45f..ed015bfc18d8 100644 --- a/netwerk/test/unit/test_about_networking.js +++ b/netwerk/test/unit/test_about_networking.js @@ -89,9 +89,6 @@ function run_test() { true ); - // We always resolve localhost as it's hardcoded without the following pref: - Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); - let ioService = Cc["@mozilla.org/network/io-service;1"].getService( Ci.nsIIOService ); @@ -106,7 +103,6 @@ function run_test() { channel.open(); gServerSocket.init(-1, true, -1); - Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); run_next_test(); } diff --git a/netwerk/test/unit/test_dns_offline.js b/netwerk/test/unit/test_dns_offline.js index a82f08040767..15abfa193932 100644 --- a/netwerk/test/unit/test_dns_offline.js +++ b/netwerk/test/unit/test_dns_offline.js @@ -43,8 +43,6 @@ const defaultOriginAttributes = {}; function run_test() { do_test_pending(); prefs.setBoolPref("network.dns.offline-localhost", false); - // We always resolve localhost as it's hardcoded without the following pref: - prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); ioService.offline = true; try { dns.asyncResolve( @@ -99,5 +97,4 @@ function test3Continued() { function cleanup() { prefs.clearUserPref("network.dns.offline-localhost"); - prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); } diff --git a/netwerk/test/unit/test_dns_originAttributes.js b/netwerk/test/unit/test_dns_originAttributes.js index a2857bc7402c..b87b6b44fecb 100644 --- a/netwerk/test/unit/test_dns_originAttributes.js +++ b/netwerk/test/unit/test_dns_originAttributes.js @@ -2,9 +2,6 @@ var dns = Cc["@mozilla.org/network/dns-service;1"].getService(Ci.nsIDNSService); var threadManager = Cc["@mozilla.org/thread-manager;1"].getService( Ci.nsIThreadManager ); -var prefs = Cc["@mozilla.org/preferences-service;1"].getService( - Ci.nsIPrefBranch -); var mainThread = threadManager.currentThread; var listener1 = { @@ -67,7 +64,6 @@ function test2() { // for this originAttributes. function test3() { do_test_pending(); - prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); try { dns.asyncResolve( "localhost", @@ -78,7 +74,6 @@ function test3() { ); } catch (e) { Assert.equal(e.result, Cr.NS_ERROR_OFFLINE); - prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); do_test_finished(); } } diff --git a/netwerk/test/unit/test_ping_aboutnetworking.js b/netwerk/test/unit/test_ping_aboutnetworking.js index 5808a1fbd419..c72900716a7b 100644 --- a/netwerk/test/unit/test_ping_aboutnetworking.js +++ b/netwerk/test/unit/test_ping_aboutnetworking.js @@ -49,8 +49,6 @@ function run_test() { // disable network changed events to avoid the the risk of having the dns // cache getting flushed behind our back ps.setBoolPref("network.notify.changed", false); - // Localhost is hardcoded to loopback and isn't cached, disable that with this pref - ps.setBoolPref("network.proxy.allow_hijacking_localhost", true); registerCleanupFunction(function() { ps.clearUserPref("network.notify.changed"); diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js index e2093abb480a..3e5d96296853 100644 --- a/netwerk/test/unit/test_trr.js +++ b/netwerk/test/unit/test_trr.js @@ -1010,7 +1010,6 @@ add_task(async function test24k() { // resolver if it's not in the excluded domains add_task(async function test25() { dns.clearCache(true); - Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); // Disable localhost hardcoding Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only Services.prefs.setCharPref("network.trr.excluded-domains", ""); Services.prefs.setCharPref("network.trr.builtin-excluded-domains", ""); @@ -1020,7 +1019,6 @@ add_task(async function test25() { ); await new DNSListener("localhost", "192.192.192.192", true); - Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); }); // TRR-only check that localhost goes directly to native lookup when in the excluded-domains