bug 1441256 - bypass cache when retrying connection without TRR r?valentin,mcmanus draft
authorDaniel Stenberg <daniel@haxx.se>
Tue, 06 Mar 2018 14:50:21 +0100
changeset 764226 5d2486eae400f5a6f8bad93dfa964e38e16642f0
parent 763042 69f9ae4f6e8943d24569a9e4380f6df869f56b73
push id101707
push userbmo:daniel@haxx.se
push dateWed, 07 Mar 2018 13:27:29 +0000
reviewersvalentin, mcmanus
bugs1441256
milestone60.0a1
bug 1441256 - bypass cache when retrying connection without TRR r?valentin,mcmanus 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);
 };