bug 1441256 - bypass cache when retrying connection without TRR r=mcmanus,valentin
authorDaniel Stenberg <daniel@haxx.se>
Tue, 06 Mar 2018 14:50:21 +0100
changeset 462145 09b580ee681bd1b67e853e41b2bdb2754c3e9053
parent 462144 7efade306e9e4f4aa9e1ec2e09c4b5641e6e6ff6
child 462146 885b39af0ef1d7ccb47c6ee52a9581e05b816d86
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus, valentin
bugs1441256
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1441256 - bypass cache when retrying connection without TRR r=mcmanus,valentin Otherwise it will just load back the same (problematic) addresses from the cache again the second time. This introduces a new resolver bit (REFRESH_CACHE) that also invalidates the existing cache entry while doing the new resolve. MozReview-Commit-ID: 5Bc2KiAGYYA
netwerk/base/nsISocketTransport.idl
netwerk/base/nsSocketTransport2.cpp
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
netwerk/dns/nsIDNSService.idl
--- a/netwerk/base/nsISocketTransport.idl
+++ b/netwerk/base/nsISocketTransport.idl
@@ -240,16 +240,25 @@ interface nsISocketTransport : nsITransp
     /**
      * If set, do not use TRR for resolving the host name. Intended only for
      * retries or other scenarios when TRR is deemed likely to have returned a
      * wrong adddress.
      */
     const unsigned long DISABLE_TRR = (1 << 8);
 
     /**
+     * Values for the connectionFlags
+     *
+     * When using BYPASS_CACHE, setting this bit will invalidate the existing
+     * cached entry immediately while the new resolve is being done to avoid
+     * other users from using stale content in the mean time.
+     */
+    const unsigned long REFRESH_CACHE = (1 << 9);
+
+    /**
      * An opaque flags for non-standard behavior of the TLS system.
      * It is unlikely this will need to be set outside of telemetry studies
      * relating to the TLS implementation.
      */
     attribute unsigned long tlsFlags;
 
     /**
      * Socket QoS/ToS markings. Valid values are IPTOS_DSCP_AFxx or
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -1101,16 +1101,18 @@ nsSocketTransport::ResolveHost()
     nsCOMPtr<nsIDNSService> dns = do_GetService(kDNSServiceCID, &rv);
     if (NS_FAILED(rv)) return rv;
 
     mResolving = true;
 
     uint32_t dnsFlags = 0;
     if (mConnectionFlags & nsSocketTransport::BYPASS_CACHE)
         dnsFlags = nsIDNSService::RESOLVE_BYPASS_CACHE;
+    if (mConnectionFlags & nsSocketTransport::REFRESH_CACHE)
+        dnsFlags = nsIDNSService::RESOLVE_REFRESH_CACHE;
     if (mConnectionFlags & nsSocketTransport::DISABLE_IPV6)
         dnsFlags |= nsIDNSService::RESOLVE_DISABLE_IPV6;
     if (mConnectionFlags & nsSocketTransport::DISABLE_IPV4)
         dnsFlags |= nsIDNSService::RESOLVE_DISABLE_IPV4;
     if (mConnectionFlags & nsSocketTransport::DISABLE_TRR)
         dnsFlags |= nsIDNSService::RESOLVE_DISABLE_TRR;
 
     NS_ASSERTION(!(dnsFlags & nsIDNSService::RESOLVE_DISABLE_IPV6) ||
@@ -1804,20 +1806,21 @@ nsSocketTransport::RecoverFromError()
                 mState = STATE_CLOSED;
                 mConnectionFlags &= ~(DISABLE_IPV6 | DISABLE_IPV4);
                 tryAgain = true;
             } else if (!(mConnectionFlags & DISABLE_TRR)) {
                 bool trrEnabled;
                 mDNSRecord->IsTRR(&trrEnabled);
                 if (trrEnabled) {
                     // Drop state to closed.  This will trigger a new round of
-                    // DNS resolving.
+                    // DNS resolving. Bypass the cache this time since the
+                    // cached data came from TRR and failed already!
                     SOCKET_LOG(("  failed to connect with TRR enabled, try w/o\n"));
                     mState = STATE_CLOSED;
-                    mConnectionFlags |= DISABLE_TRR;
+                    mConnectionFlags |= DISABLE_TRR | BYPASS_CACHE | REFRESH_CACHE;
                     tryAgain = true;
                 }
             }
         }
     }
 
     // prepare to try again.
     if (tryAgain) {
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -219,16 +219,22 @@ nsHostRecord::Cancel()
     }
     if (mTrrAAAA) {
         mTrrAAAA->Cancel();
         mTrrAAAA = nullptr;
     }
 }
 
 void
+nsHostRecord::Invalidate()
+{
+    mDoomed = true;
+}
+
+void
 nsHostRecord::SetExpiration(const mozilla::TimeStamp& now, unsigned int valid, unsigned int grace)
 {
     mValidStart = now;
     mGraceStart = now + TimeDuration::FromSeconds(valid);
     mValidEnd = now + TimeDuration::FromSeconds(valid + grace);
 }
 
 void
@@ -732,18 +738,19 @@ nsHostResolver::ResolveHost(const char  
                             uint16_t                flags,
                             uint16_t                af,
                             const char             *netInterface,
                             nsResolveHostCallback  *aCallback)
 {
     NS_ENSURE_TRUE(host && *host, NS_ERROR_UNEXPECTED);
     NS_ENSURE_TRUE(netInterface, NS_ERROR_UNEXPECTED);
 
-    LOG(("Resolving host [%s%s%s]%s.\n", LOG_HOST(host, netInterface),
-         flags & RES_BYPASS_CACHE ? " - bypassing cache" : ""));
+    LOG(("Resolving host [%s%s%s]%s%s.\n", LOG_HOST(host, netInterface),
+         flags & RES_BYPASS_CACHE ? " - bypassing cache" : "",
+         flags & RES_REFRESH_CACHE ? " - refresh cache" : ""));
 
     // ensure that we are working with a valid hostname before proceeding.  see
     // bug 304904 for details.
     if (!net_IsValidHostName(nsDependentCString(host)))
         return NS_ERROR_UNKNOWN_HOST;
 
     RefPtr<nsResolveHostCallback> callback(aCallback);
     // if result is set inside the lock, then we need to issue the
@@ -917,16 +924,20 @@ nsHostResolver::ResolveHost(const char  
                     }
                 }
                 // If no valid address was found in the cache or this is an
                 // AF_UNSPEC request, then start a new lookup.
                 if (!result) {
                     LOG(("  No usable address in cache for host [%s%s%s].",
                          LOG_HOST(host, netInterface)));
 
+                    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);
                     Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
                                           METHOD_NETWORK_FIRST);
                     if (NS_FAILED(rv) && callback->isInList()) {
                         callback->remove();
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -138,16 +138,19 @@ public:
     // mValidEnd, and mGraceStart). valid and grace are durations in seconds.
     void SetExpiration(const mozilla::TimeStamp& now, unsigned int valid,
                        unsigned int grace);
     void CopyExpirationTimesAndFlagsFrom(const nsHostRecord *aFromHostRecord);
 
     // Checks if the record is usable (not expired and has a value)
     bool HasUsableResult(const mozilla::TimeStamp& now, uint16_t queryFlags = 0) const;
 
+    // Mark hostrecord as not usable
+    void Invalidate();
+
     // hold addr_info_lock when calling the blacklist functions
     bool   Blacklisted(mozilla::net::NetAddr *query);
     void   ResetBlacklist();
     void   ReportUnusable(mozilla::net::NetAddr *addr);
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
     enum DnsPriority {
@@ -366,17 +369,18 @@ public:
         RES_CANON_NAME = nsIDNSService::RESOLVE_CANONICAL_NAME,
         RES_PRIORITY_MEDIUM = nsIDNSService::RESOLVE_PRIORITY_MEDIUM,
         RES_PRIORITY_LOW = nsIDNSService::RESOLVE_PRIORITY_LOW,
         RES_SPECULATE = nsIDNSService::RESOLVE_SPECULATE,
         //RES_DISABLE_IPV6 = nsIDNSService::RESOLVE_DISABLE_IPV6, // Not used
         RES_OFFLINE = nsIDNSService::RESOLVE_OFFLINE,
         //RES_DISABLE_IPv4 = nsIDNSService::RESOLVE_DISABLE_IPV4, // Not Used
         RES_ALLOW_NAME_COLLISION = nsIDNSService::RESOLVE_ALLOW_NAME_COLLISION,
-        RES_DISABLE_TRR = nsIDNSService::RESOLVE_DISABLE_TRR
+        RES_DISABLE_TRR = nsIDNSService::RESOLVE_DISABLE_TRR,
+        RES_REFRESH_CACHE = nsIDNSService::RESOLVE_REFRESH_CACHE
     };
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
     /**
      * Flush the DNS cache.
      */
     void FlushCache();
--- a/netwerk/dns/nsIDNSService.idl
+++ b/netwerk/dns/nsIDNSService.idl
@@ -234,9 +234,14 @@ interface nsIDNSService : nsISupports
      */
     const unsigned long RESOLVE_ALLOW_NAME_COLLISION = (1 << 8);
 
     /**
      * If set, do not use TRR for resolving the host name.
      */
     const unsigned long RESOLVE_DISABLE_TRR = (1 << 9);
 
+    /**
+     * if set (together with RESOLVE_BYPASS_CACHE), invalidate the DNS
+     * existing cache entry first (if existing) then make a new resolve.
+     */
+    const unsigned long RESOLVE_REFRESH_CACHE = (1 << 10);
 };