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

--HG--
extra : rebase_source : 5c100f179a23c805fe245a361f4e89c8d5f5ce0a

toolkit/components/url-classifier/ProtocolParser.cpp

@@ -113,7 +113,6 @@ ProtocolParser::GetTableUpdate(const nsACString& aTable)
 
 ProtocolParserV2::ProtocolParserV2()
   : mState(PROTOCOL_STATE_CONTROL)
-  , mResetRequested(false)
   , mTableUpdate(nullptr)
 {
 }
@@ -188,7 +187,8 @@ ProtocolParserV2::ProcessControl(bool* aDone)
         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);
@@ -780,21 +780,29 @@ ProtocolParserProtobuf::End()
 
   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.");
@@ -816,17 +824,16 @@ ProtocolParserProtobuf::ProcessOneResponse(const ListUpdateResponse& aResponse)
   // 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;
   }
@@ -847,7 +854,7 @@ ProtocolParserProtobuf::ProcessOneResponse(const ListUpdateResponse& aResponse)
     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);
 
@@ -860,7 +867,7 @@ ProtocolParserProtobuf::ProcessOneResponse(const ListUpdateResponse& aResponse)
   }
 
   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")));

toolkit/components/url-classifier/ProtocolParser.h

@@ -58,7 +58,8 @@ public:
   // 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;
@@ -74,6 +75,9 @@ protected:
   // 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;
 
@@ -95,7 +99,6 @@ public:
 
   // 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
@@ -156,8 +159,6 @@ private:
   };
   ChunkState mChunkState;
 
-  bool mResetRequested;
-
   // Updates to apply to the current table being parsed.
   TableUpdateV2 *mTableUpdate;
 
@@ -185,7 +186,8 @@ private:
   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,

toolkit/components/url-classifier/nsUrlClassifierDBService.cpp

@@ -571,7 +571,8 @@ nsUrlClassifierDBServiceWorker::FinishStream()
 
   if (NS_SUCCEEDED(mUpdateStatus)) {
     if (mProtocolParser->ResetRequested()) {
-      mClassifier->ResetTables(Classifier::Clear_All, mUpdateTables);
+      mClassifier->ResetTables(Classifier::Clear_All,
+                               mProtocolParser->TablesToReset());
     }
   }