Bug 1409210 - Don't prefetch resources with a Vary header. draft
authorNicholas Hurley <hurley@mozilla.com>
Mon, 16 Oct 2017 16:55:46 -0700
changeset 681790 180dcffe8ba71ba6a89b97881f5da85fa59ba5e8
parent 681625 f27105b62753c71ecadad2f8d632ec7e5ac96bbd
child 684099 2fb7a7c7a90fb9c6231f9f5a511b0702f073b8d3
push id84929
push userbmo:hurley@mozilla.com
push dateTue, 17 Oct 2017 19:56:57 +0000
bugs1409210
milestone58.0a1
Bug 1409210 - Don't prefetch resources with a Vary header. Conceivably, we could allow a few more prefetches than this would (based on the headers in the original request matching up to a header listed in the Vary response header), but this is safer in case (for example) future requests of this resource end up sending a cookie that wasn't set on the original request. In practice, the difference is likely to be small enough that this broader stroke won't make a huge impact on the number of things we do or don't prefetch. MozReview-Commit-ID: GhD9mZR6aOX
netwerk/base/Predictor.cpp
netwerk/base/Predictor.h
--- a/netwerk/base/Predictor.cpp
+++ b/netwerk/base/Predictor.cpp
@@ -2604,28 +2604,30 @@ Predictor::UpdateCacheability(nsIURI *so
     PREDICTOR_LOG(("Predictor::UpdateCacheability non-http(s) uri"));
     return;
   }
 
   RefPtr<Predictor> self = sSelf;
   if (self) {
     nsAutoCString method;
     requestHead.Method(method);
+    nsAutoCString vary;
+    Unused << responseHead->GetHeader(nsHttp::Vary, vary);
     self->UpdateCacheabilityInternal(sourceURI, targetURI, httpStatus,
                                      method, *lci->OriginAttributesPtr(),
-                                     isTracking);
+                                     isTracking, !vary.IsEmpty());
   }
 }
 
 void
 Predictor::UpdateCacheabilityInternal(nsIURI *sourceURI, nsIURI *targetURI,
                                       uint32_t httpStatus,
                                       const nsCString &method,
                                       const OriginAttributes& originAttributes,
-                                      bool isTracking)
+                                      bool isTracking, bool couldVary)
 {
   PREDICTOR_LOG(("Predictor::UpdateCacheability httpStatus=%u", httpStatus));
 
   nsresult rv;
 
   if (!mInitialized) {
     PREDICTOR_LOG(("    not initialized"));
     return;
@@ -2653,17 +2655,17 @@ Predictor::UpdateCacheabilityInternal(ns
     return;
   }
 
   uint32_t openFlags = nsICacheStorage::OPEN_READONLY |
                        nsICacheStorage::OPEN_SECRETLY |
                        nsICacheStorage::CHECK_MULTITHREADED;
   RefPtr<Predictor::CacheabilityAction> action =
     new Predictor::CacheabilityAction(targetURI, httpStatus, method, isTracking,
-                                      this);
+                                      couldVary, this);
   nsAutoCString uri;
   targetURI->GetAsciiSpec(uri);
   PREDICTOR_LOG(("    uri=%s action=%p", uri.get(), action.get()));
   cacheDiskStorage->AsyncOpenURI(sourceURI, EmptyCString(), openFlags, action);
 }
 
 NS_IMPL_ISUPPORTS(Predictor::CacheabilityAction,
                   nsICacheEntryOpenCallback,
@@ -2730,17 +2732,18 @@ Predictor::CacheabilityAction::OnCacheEn
                                         flags)) {
       PREDICTOR_LOG(("    failed to parse key=%s value=%s", key, value));
       continue;
     }
 
     if (strTargetURI.Equals(uri)) {
       if (mHttpStatus == 200 && mMethod.EqualsLiteral("GET") &&
           !hasQueryString &&
-          !mIsTracking) {
+          !mIsTracking &&
+          !mCouldVary) {
         PREDICTOR_LOG(("    marking %s cacheable", key));
         flags |= FLAG_PREFETCHABLE;
       } else {
         PREDICTOR_LOG(("    marking %s uncacheable", key));
         flags &= ~FLAG_PREFETCHABLE;
       }
       nsCString newValue;
       MakeMetadataEntry(hitCount, lastHit, flags, newValue);
--- a/netwerk/base/Predictor.h
+++ b/netwerk/base/Predictor.h
@@ -134,32 +134,34 @@ private:
                            , public nsICacheEntryMetaDataVisitor
   {
   public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSICACHEENTRYOPENCALLBACK
     NS_DECL_NSICACHEENTRYMETADATAVISITOR
 
     CacheabilityAction(nsIURI *targetURI, uint32_t httpStatus,
-                       const nsCString &method, bool isTracking,
+                       const nsCString &method, bool isTracking, bool couldVary,
                        Predictor *predictor)
       :mTargetURI(targetURI)
       ,mHttpStatus(httpStatus)
       ,mMethod(method)
       ,mIsTracking(isTracking)
+      ,mCouldVary(couldVary)
       ,mPredictor(predictor)
     { }
 
   private:
     virtual ~CacheabilityAction() { }
 
     nsCOMPtr<nsIURI> mTargetURI;
     uint32_t mHttpStatus;
     nsCString mMethod;
     bool mIsTracking;
+    bool mCouldVary;
     RefPtr<Predictor> mPredictor;
     nsTArray<nsCString> mKeysToCheck;
     nsTArray<nsCString> mValuesToCheck;
   };
 
   class Resetter : public nsICacheEntryOpenCallback,
                    public nsICacheEntryMetaDataVisitor,
                    public nsICacheStorageVisitor
@@ -421,17 +423,17 @@ private:
                           uint32_t &flags);
 
   // Used to update whether a particular URI was cacheable or not.
   // sourceURI and targetURI are the same as the arguments to Learn
   // and httpStatus is the status code we got while loading targetURI.
   void UpdateCacheabilityInternal(nsIURI *sourceURI, nsIURI *targetURI,
                                   uint32_t httpStatus, const nsCString &method,
                                   const OriginAttributes& originAttributes,
-                                  bool isTracking);
+                                  bool isTracking, bool couldVary);
 
   // Make sure our prefs are in their expected range of values
   void SanitizePrefs();
 
   // Our state
   bool mInitialized;
 
   bool mEnabled;