Bug 1434662 - Reset Safe Browsing V4 tables that fail to update. r=gcp
authorFrancois Marier <francois@mozilla.com>
Thu, 12 Apr 2018 10:11:30 -0700
changeset 468814 7bbbf41e92104a15cad390a799d55f89ddc3ae9b
parent 468813 6e691c7ff5dd229d4ad84177e52d51e512bb0a92
child 468815 e1f41b9f1d44fef53b5c717e3bc67dca6bdce845
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1434662
milestone61.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 1434662 - Reset Safe Browsing V4 tables that fail to update. r=gcp This is a generalization of the reset code that's used in pver2 to reset all tables when a `pleasereset` command is received. MozReview-Commit-ID: LF4RegQHqoT
toolkit/components/url-classifier/ProtocolParser.cpp
toolkit/components/url-classifier/ProtocolParser.h
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
--- a/toolkit/components/url-classifier/ProtocolParser.cpp
+++ b/toolkit/components/url-classifier/ProtocolParser.cpp
@@ -108,17 +108,16 @@ ProtocolParser::GetTableUpdate(const nsA
   return update;
 }
 
 ///////////////////////////////////////////////////////////////////////
 // ProtocolParserV2
 
 ProtocolParserV2::ProtocolParserV2()
   : mState(PROTOCOL_STATE_CONTROL)
-  , mResetRequested(false)
   , mTableUpdate(nullptr)
 {
 }
 
 ProtocolParserV2::~ProtocolParserV2()
 {
 }
 
@@ -183,17 +182,18 @@ ProtocolParserV2::ProcessControl(bool* a
       // Set the table name from the table header line.
       SetCurrentTable(Substring(line, 2));
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("n:"))) {
       if (PR_sscanf(line.get(), "n:%d", &mUpdateWaitSec) != 1) {
         PARSER_LOG(("Error parsing n: '%s' (%d)", line.get(), mUpdateWaitSec));
         return NS_ERROR_FAILURE;
       }
     } else if (line.EqualsLiteral("r:pleasereset")) {
-      mResetRequested = true;
+      PARSER_LOG(("All tables will be reset."));
+      mTablesToReset = mRequestedTables;
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("u:"))) {
       rv = ProcessForward(line);
       NS_ENSURE_SUCCESS(rv, rv);
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("a:")) ||
                StringBeginsWith(line, NS_LITERAL_CSTRING("s:"))) {
       rv = ProcessChunkControl(line);
       NS_ENSURE_SUCCESS(rv, rv);
       *aDone = false;
@@ -775,31 +775,39 @@ ProtocolParserProtobuf::End()
   }
 
   auto minWaitDuration = response.minimum_wait_duration();
   mUpdateWaitSec = minWaitDuration.seconds() +
                    minWaitDuration.nanos() / 1000000000;
 
   for (int i = 0; i < response.list_update_responses_size(); i++) {
     auto r = response.list_update_responses(i);
-    nsresult rv = ProcessOneResponse(r);
+    nsAutoCString listName;
+    nsresult rv = ProcessOneResponse(r, listName);
     if (NS_SUCCEEDED(rv)) {
       mUpdateStatus = rv;
     } else {
       nsAutoCString errorName;
       mozilla::GetErrorName(rv, errorName);
-      NS_WARNING(nsPrintfCString("Failed to process one response: %s",
-                                 errorName.get()).get());
+      NS_WARNING(nsPrintfCString("Failed to process one response for '%s': %s",
+                                 listName.get(), errorName.get()).get());
+      if (!listName.IsEmpty()) {
+        PARSER_LOG(("Table %s will be reset.", listName.get()));
+        mTablesToReset.AppendElement(listName);
+      }
     }
   }
 }
 
 nsresult
-ProtocolParserProtobuf::ProcessOneResponse(const ListUpdateResponse& aResponse)
+ProtocolParserProtobuf::ProcessOneResponse(const ListUpdateResponse& aResponse,
+                                           nsACString& aListName)
 {
+  MOZ_ASSERT(aListName.IsEmpty());
+
   // A response must have a threat type.
   if (!aResponse.has_threat_type()) {
     NS_WARNING("Threat type not initialized. This seems to be an invalid response.");
     return NS_ERROR_UC_PARSER_MISSING_PARAM;
   }
 
   // Convert threat type to list name.
   nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
@@ -811,27 +819,26 @@ ProtocolParserProtobuf::ProcessOneRespon
     PARSER_LOG(("Threat type to list name conversion error: %d",
                 aResponse.threat_type()));
     return NS_ERROR_UC_PARSER_UNKNOWN_THREAT;
   }
 
   // Match the table name we received with one of the ones we requested.
   // We ignore the case where a threat type matches more than one list
   // per provider and return the first one. See bug 1287059."
-  nsCString listName;
   nsTArray<nsCString> possibleListNameArray;
   Classifier::SplitTables(possibleListNames, possibleListNameArray);
   for (auto possibleName : possibleListNameArray) {
     if (mRequestedTables.Contains(possibleName)) {
-      listName = possibleName;
+      aListName = possibleName;
       break;
     }
   }
 
-  if (listName.IsEmpty()) {
+  if (aListName.IsEmpty()) {
     PARSER_LOG(("We received an update for a list we didn't ask for. Ignoring it."));
     return NS_ERROR_FAILURE;
   }
 
   // Test if this is a full update.
   bool isFullUpdate = false;
   if (aResponse.has_response_type()) {
     isFullUpdate =
@@ -842,30 +849,30 @@ ProtocolParserProtobuf::ProcessOneRespon
   }
 
   // Warn if there's no new state.
   if (!aResponse.has_new_client_state()) {
     NS_WARNING("New state not initialized.");
     return NS_ERROR_UC_PARSER_MISSING_PARAM;
   }
 
-  auto tu = GetTableUpdate(nsCString(listName.get()));
+  auto tu = GetTableUpdate(aListName);
   auto tuV4 = TableUpdate::Cast<TableUpdateV4>(tu);
   NS_ENSURE_TRUE(tuV4, NS_ERROR_FAILURE);
 
   nsCString state(aResponse.new_client_state().c_str(),
                   aResponse.new_client_state().size());
   tuV4->SetNewClientState(state);
 
   if (aResponse.has_checksum()) {
     tuV4->NewChecksum(aResponse.checksum().sha256());
   }
 
   PARSER_LOG(("==== Update for threat type '%d' ====", aResponse.threat_type()));
-  PARSER_LOG(("* listName: %s\n", listName.get()));
+  PARSER_LOG(("* aListName: %s\n", PromiseFlatCString(aListName).get()));
   PARSER_LOG(("* newState: %s\n", aResponse.new_client_state().c_str()));
   PARSER_LOG(("* isFullUpdate: %s\n", (isFullUpdate ? "yes" : "no")));
   PARSER_LOG(("* hasChecksum: %s\n", (aResponse.has_checksum() ? "yes" : "no")));
   PARSER_LOG(("* additions: %d\n", aResponse.additions().size()));
   PARSER_LOG(("* removals: %d\n", aResponse.removals().size()));
 
   tuV4->SetFullUpdate(isFullUpdate);
 
--- a/toolkit/components/url-classifier/ProtocolParser.h
+++ b/toolkit/components/url-classifier/ProtocolParser.h
@@ -53,32 +53,36 @@ public:
   void ForgetTableUpdates() { mTableUpdates.Clear(); }
   nsTArray<TableUpdate*> &GetTableUpdates() { return mTableUpdates; }
 
   // These are only meaningful to V2. Since they were originally public,
   // moving them to ProtocolParserV2 requires a dymamic cast in the call
   // sites. As a result, we will leave them until we remove support
   // for V2 entirely..
   virtual const nsTArray<ForwardedUpdate> &Forwards() const { return mForwards; }
-  virtual bool ResetRequested() { return false; }
+  bool ResetRequested() const { return !mTablesToReset.IsEmpty(); }
+  const nsTArray<nsCString>& TablesToReset() const { return mTablesToReset; }
 
 protected:
   virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const = 0;
 
   nsCString mPending;
   nsresult mUpdateStatus;
 
   // Keep track of updates to apply before passing them to the DBServiceWorkers.
   nsTArray<TableUpdate*> mTableUpdates;
 
   nsTArray<ForwardedUpdate> mForwards;
 
   // The table names that were requested from the client.
   nsTArray<nsCString> mRequestedTables;
 
+  // The table names that failed to update and need to be reset.
+  nsTArray<nsCString> mTablesToReset;
+
   // How long we should wait until the next update.
   uint32_t mUpdateWaitSec;
 
 private:
   void CleanupUpdates();
 };
 
 /**
@@ -90,17 +94,16 @@ public:
   virtual ~ProtocolParserV2();
 
   virtual void SetCurrentTable(const nsACString& aTable) override;
   virtual nsresult AppendStream(const nsACString& aData) override;
   virtual void End() override;
 
   // Update information.
   virtual const nsTArray<ForwardedUpdate> &Forwards() const override { return mForwards; }
-  virtual bool ResetRequested() override { return mResetRequested; }
 
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
   // Unfortunately we have to override to return mRawUpdate which
   // will not be modified during the parsing, unlike mPending.
   virtual nsCString GetRawTableUpdates() const override { return mRawUpdate; }
 #endif
 
 private:
@@ -151,18 +154,16 @@ private:
     ChunkType type;
     uint32_t num;
     uint32_t hashSize;
     uint32_t length;
     void Clear() { num = 0; hashSize = 0; length = 0; }
   };
   ChunkState mChunkState;
 
-  bool mResetRequested;
-
   // Updates to apply to the current table being parsed.
   TableUpdateV2 *mTableUpdate;
 
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
   nsCString mRawUpdate; // Keep a copy of mPending before it's processed.
 #endif
 };
 
@@ -180,17 +181,18 @@ public:
   virtual void End() override;
 
 private:
   virtual ~ProtocolParserProtobuf();
 
   virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const override;
 
   // For parsing update info.
-  nsresult ProcessOneResponse(const ListUpdateResponse& aResponse);
+  nsresult ProcessOneResponse(const ListUpdateResponse& aResponse,
+                              nsACString& aListName);
 
   nsresult ProcessAdditionOrRemoval(TableUpdateV4& aTableUpdate,
                                     const ThreatEntrySetList& aUpdate,
                                     bool aIsAddition);
 
   nsresult ProcessRawAddition(TableUpdateV4& aTableUpdate,
                               const ThreatEntrySet& aAddition);
 
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -566,17 +566,18 @@ nsUrlClassifierDBServiceWorker::FinishSt
     LOG(("nsUrlClassifierDBService::FinishStream Failed to parse the stream "
          "using mProtocolParser."));
     mUpdateStatus = mProtocolParser->Status();
   }
   mUpdateObserver->StreamFinished(mProtocolParser->Status(), 0);
 
   if (NS_SUCCEEDED(mUpdateStatus)) {
     if (mProtocolParser->ResetRequested()) {
-      mClassifier->ResetTables(Classifier::Clear_All, mUpdateTables);
+      mClassifier->ResetTables(Classifier::Clear_All,
+                               mProtocolParser->TablesToReset());
     }
   }
 
   mProtocolParser = nullptr;
 
   return mUpdateStatus;
 }