Bug 1307541 - ProtocolParserProtobuf to init and return update time properly. r=francois. draft
authorHenry <hchang@mozilla.com>
Tue, 18 Oct 2016 14:45:21 +0800
changeset 427465 e8cd28a805b892fd26ca9bede3523a04f1bb5686
parent 427338 99a239e1866a57f987b08dad796528e4ea30e622
child 534467 6645a50af6e934ef6002a2cfd68031d0bb4cceba
push id33017
push userhchang@mozilla.com
push dateThu, 20 Oct 2016 10:39:43 +0000
reviewersfrancois
bugs1307541
milestone52.0a1
Bug 1307541 - ProtocolParserProtobuf to init and return update time properly. r=francois. MozReview-Commit-ID: CmVWVKUeunJ
toolkit/components/url-classifier/ProtocolParser.cpp
toolkit/components/url-classifier/ProtocolParser.h
toolkit/components/url-classifier/content/listmanager.js
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/tests/gtest/TestProtocolParser.cpp
toolkit/components/url-classifier/tests/gtest/moz.build
--- a/toolkit/components/url-classifier/ProtocolParser.cpp
+++ b/toolkit/components/url-classifier/ProtocolParser.cpp
@@ -64,16 +64,17 @@ ParseChunkRange(nsACString::const_iterat
   return false;
 }
 
 ///////////////////////////////////////////////////////////////
 // ProtocolParser implementation
 
 ProtocolParser::ProtocolParser()
   : mUpdateStatus(NS_OK)
+  , mUpdateWaitSec(0)
 {
 }
 
 ProtocolParser::~ProtocolParser()
 {
   CleanupUpdates();
 }
 
@@ -111,17 +112,16 @@ ProtocolParser::GetTableUpdate(const nsA
   return update;
 }
 
 ///////////////////////////////////////////////////////////////////////
 // ProtocolParserV2
 
 ProtocolParserV2::ProtocolParserV2()
   : mState(PROTOCOL_STATE_CONTROL)
-  , mUpdateWait(0)
   , mResetRequested(false)
   , mTableUpdate(nullptr)
 {
 }
 
 ProtocolParserV2::~ProtocolParserV2()
 {
 }
@@ -175,18 +175,18 @@ ProtocolParserV2::ProcessControl(bool* a
   *aDone = true;
   while (NextLine(line)) {
     PARSER_LOG(("Processing %s\n", line.get()));
 
     if (StringBeginsWith(line, NS_LITERAL_CSTRING("i:"))) {
       // 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", &mUpdateWait) != 1) {
-        PARSER_LOG(("Error parsing n: '%s' (%d)", line.get(), mUpdateWait));
+      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;
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("u:"))) {
       rv = ProcessForward(line);
       NS_ENSURE_SUCCESS(rv, rv);
     } else if (StringBeginsWith(line, NS_LITERAL_CSTRING("a:")) ||
@@ -766,16 +766,20 @@ ProtocolParserProtobuf::End()
   mUpdateStatus = NS_ERROR_FAILURE;
 
   FetchThreatListUpdatesResponse response;
   if (!response.ParseFromArray(mPending.get(), mPending.Length())) {
     NS_WARNING("ProtocolParserProtobuf failed parsing data.");
     return;
   }
 
+  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);
     if (NS_SUCCEEDED(rv)) {
       mUpdateStatus = rv;
     } else {
       NS_WARNING("Failed to process one response.");
     }
--- a/toolkit/components/url-classifier/ProtocolParser.h
+++ b/toolkit/components/url-classifier/ProtocolParser.h
@@ -35,32 +35,33 @@ public:
   void SetRequestedTables(const nsTArray<nsCString>& aRequestTables)
   {
     mRequestedTables = aRequestTables;
   }
 
   nsresult Begin();
   virtual nsresult AppendStream(const nsACString& aData) = 0;
 
+  uint32_t UpdateWaitSec() { return mUpdateWaitSec; }
+
   // Notify that the inbound data is ready for parsing if progressive
   // parsing is not supported, for example in V4.
   virtual void End() = 0;
 
   // Forget the table updates that were created by this pass.  It
   // becomes the caller's responsibility to free them.  This is shitty.
   TableUpdate *GetTableUpdate(const nsACString& aTable);
   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 
+  // sites. As a result, we will leave them until we remove support
   // for V2 entirely..
   virtual const nsTArray<ForwardedUpdate> &Forwards() const { return mForwards; }
-  virtual int32_t UpdateWait() { return 0; }
   virtual bool ResetRequested() { return false; }
 
 protected:
   virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const = 0;
 
   nsCString mPending;
   nsresult mUpdateStatus;
 
@@ -68,16 +69,19 @@ protected:
   nsTArray<TableUpdate*> mTableUpdates;
 
   nsTArray<ForwardedUpdate> mForwards;
   nsCOMPtr<nsICryptoHash> mCryptoHash;
 
   // The table names that were requested from the client.
   nsTArray<nsCString> mRequestedTables;
 
+  // How long we should wait until the next update.
+  uint32_t mUpdateWaitSec;
+
 private:
   void CleanupUpdates();
 };
 
 /**
  * Helpers to parse the "shavar", "digest256" and "simple" list formats.
  */
 class ProtocolParserV2 final : public ProtocolParser {
@@ -86,17 +90,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 int32_t UpdateWait() override { return mUpdateWait; }
   virtual bool ResetRequested() override { return mResetRequested; }
 
 private:
   virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const override;
 
   nsresult ProcessControl(bool* aDone);
   nsresult ProcessExpirations(const nsCString& aLine);
   nsresult ProcessChunkControl(const nsCString& aLine);
@@ -142,17 +145,16 @@ private:
     ChunkType type;
     uint32_t num;
     uint32_t hashSize;
     uint32_t length;
     void Clear() { num = 0; hashSize = 0; length = 0; }
   };
   ChunkState mChunkState;
 
-  uint32_t mUpdateWait;
   bool mResetRequested;
 
   // Updates to apply to the current table being parsed.
   TableUpdateV2 *mTableUpdate;
 };
 
 // Helpers to parse the "proto" list format.
 class ProtocolParserProtobuf final : public ProtocolParser {
--- a/toolkit/components/url-classifier/content/listmanager.js
+++ b/toolkit/components/url-classifier/content/listmanager.js
@@ -199,17 +199,17 @@ PROT_ListManager.prototype.kickoffUpdate
   // Add a fuzz of 0-1 minutes for both v2 and v4 according to Bug 1305478.
   initialUpdateDelay += Math.floor(Math.random() * (1 * 60 * 1000));
 
   // If the user has never downloaded tables, do the check now.
   log("needsUpdate: " + JSON.stringify(this.needsUpdate_, undefined, 2));
   for (var updateUrl in this.needsUpdate_) {
     // If we haven't already kicked off updates for this updateUrl, set a
     // non-repeating timer for it. The timer delay will be reset either on
-    // updateSuccess to this.updateinterval, or backed off on downloadError.
+    // updateSuccess to this.updateInterval, or backed off on downloadError.
     // Don't set the updateChecker unless at least one table has updates
     // enabled.
     if (this.updatesNeeded_(updateUrl) && !this.updateCheckers_[updateUrl]) {
       let provider = null;
       Object.keys(this.tablesData).forEach(function(table) {
         if (this.tablesData[table].updateUrl === updateUrl) {
           let newProvider = this.tablesData[table].provider;
           if (provider) {
@@ -487,22 +487,25 @@ PROT_ListManager.prototype.makeUpdateReq
 }
 
 /**
  * Callback function if the update request succeeded.
  * @param waitForUpdate String The number of seconds that the client should
  *        wait before requesting again.
  */
 PROT_ListManager.prototype.updateSuccess_ = function(tableList, updateUrl,
-                                                     waitForUpdate) {
+                                                     waitForUpdateSec) {
   log("update success for " + tableList + " from " + updateUrl + ": " +
-      waitForUpdate + "\n");
+      waitForUpdateSec + "\n");
+
+  // The time unit below are all milliseconds if not specified.
+
   var delay = 0;
-  if (waitForUpdate) {
-    delay = parseInt(waitForUpdate, 10) * 1000;
+  if (waitForUpdateSec) {
+    delay = parseInt(waitForUpdateSec, 10) * 1000;
   }
   // As long as the delay is something sane (5 min to 1 day), update
   // our delay time for requesting updates. We always use a non-repeating
   // timer since the delay is set differently at every callback.
   if (delay > maxDelayMs) {
     log("Ignoring delay from server (too long), waiting " +
         maxDelayMs + "ms");
     delay = maxDelayMs;
@@ -551,17 +554,17 @@ PROT_ListManager.prototype.updateSuccess
 
 /**
  * Callback function if the update request succeeded.
  * @param result String The error code of the failure
  */
 PROT_ListManager.prototype.updateError_ = function(table, updateUrl, result) {
   log("update error for " + table + " from " + updateUrl + ": " + result + "\n");
   // There was some trouble applying the updates. Don't try again for at least
-  // updateInterval seconds.
+  // updateInterval milliseconds.
   this.updateCheckers_[updateUrl] =
     new G_Alarm(BindToObject(this.checkForUpdates, this, updateUrl),
                 this.updateInterval, false);
 }
 
 /**
  * Callback function when the download failed
  * @param status String http status or an empty string if connection refused.
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -387,17 +387,17 @@ nsUrlClassifierDBServiceWorker::ResetStr
   mInStream = false;
   mProtocolParser = nullptr;
 }
 
 void
 nsUrlClassifierDBServiceWorker::ResetUpdate()
 {
   LOG(("ResetUpdate"));
-  mUpdateWait = 0;
+  mUpdateWaitSec = 0;
   mUpdateStatus = NS_OK;
   mUpdateObserver = nullptr;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::SetHashCompleter(const nsACString &tableName,
                                                  nsIUrlClassifierHashCompleter *completer)
 {
@@ -538,18 +538,18 @@ nsUrlClassifierDBServiceWorker::FinishSt
   NS_ENSURE_STATE(mInStream);
   NS_ENSURE_STATE(mUpdateObserver);
 
   mInStream = false;
 
   mProtocolParser->End();
 
   if (NS_SUCCEEDED(mProtocolParser->Status())) {
-    if (mProtocolParser->UpdateWait()) {
-      mUpdateWait = mProtocolParser->UpdateWait();
+    if (mProtocolParser->UpdateWaitSec()) {
+      mUpdateWaitSec = mProtocolParser->UpdateWaitSec();
     }
     // XXX: Only allow forwards from the initial update?
     const nsTArray<ProtocolParser::ForwardedUpdate> &forwards =
       mProtocolParser->Forwards();
     for (uint32_t i = 0; i < forwards.Length(); i++) {
       const ProtocolParser::ForwardedUpdate &forward = forwards[i];
       mUpdateObserver->UpdateUrlRequested(forward.url, forward.table);
     }
@@ -587,18 +587,18 @@ nsUrlClassifierDBServiceWorker::FinishUp
   } else {
     LOG(("nsUrlClassifierDBServiceWorker::FinishUpdate() Not running "
          "ApplyUpdate() since the update has already failed."));
   }
 
   mMissCache.Clear();
 
   if (NS_SUCCEEDED(mUpdateStatus)) {
-    LOG(("Notifying success: %d", mUpdateWait));
-    mUpdateObserver->UpdateSuccess(mUpdateWait);
+    LOG(("Notifying success: %d", mUpdateWaitSec));
+    mUpdateObserver->UpdateSuccess(mUpdateWaitSec);
   } else if (NS_ERROR_NOT_IMPLEMENTED == mUpdateStatus) {
     LOG(("Treating NS_ERROR_NOT_IMPLEMENTED a successful update "
          "but still mark it spoiled."));
     mUpdateObserver->UpdateSuccess(0);
     mClassifier->MarkSpoiled(mUpdateTables);
   } else {
     if (LOG_ENABLED()) {
       nsAutoCString errorName;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -212,17 +212,17 @@ private:
 
   // Directory where to store the SB databases.
   nsCOMPtr<nsIFile> mCacheDir;
 
   // XXX: maybe an array of autoptrs.  Or maybe a class specifically
   // storing a series of updates.
   nsTArray<mozilla::safebrowsing::TableUpdate*> mTableUpdates;
 
-  int32_t mUpdateWait;
+  uint32_t mUpdateWaitSec;
 
   // Entries that cannot be completed. We expect them to die at
   // the next update
   PrefixArray mMissCache;
 
   // Stores the last results that triggered a table update.
   CacheResultArray mLastResults;
 
new file mode 100644
--- /dev/null
+++ b/toolkit/components/url-classifier/tests/gtest/TestProtocolParser.cpp
@@ -0,0 +1,87 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+#include "gtest/gtest.h"
+#include "ProtocolParser.h"
+#include "mozilla/EndianUtils.h"
+
+using namespace mozilla;
+using namespace mozilla::safebrowsing;
+
+typedef FetchThreatListUpdatesResponse_ListUpdateResponse ListUpdateResponse;
+
+static void
+InitUpdateResponse(ListUpdateResponse* aUpdateResponse,
+                   ThreatType aThreatType,
+                   const nsACString& aState,
+                   const nsACString& aChecksum,
+                   bool isFullUpdate,
+                   const nsTArray<uint32_t>& aFixedLengthPrefixes);
+
+static void
+DumpBinary(const nsACString& aBinary);
+
+TEST(ProtocolParser, UpdateWait)
+{
+  // Top level response which contains a list of update response
+  // for different lists.
+  FetchThreatListUpdatesResponse response;
+
+  auto r = response.mutable_list_update_responses()->Add();
+  InitUpdateResponse(r, SOCIAL_ENGINEERING_PUBLIC,
+                        nsCString("sta\x00te", 6),
+                        nsCString("check\x0sum", 9),
+                        true,
+                        {0, 1, 2, 3});
+
+  // Set min wait duration.
+  auto minWaitDuration = response.mutable_minimum_wait_duration();
+  minWaitDuration->set_seconds(8);
+  minWaitDuration->set_nanos(1 * 1000000000);
+
+  std::string s;
+  response.SerializeToString(&s);
+
+  DumpBinary(nsCString(s.c_str(), s.length()));
+
+  ProtocolParser* p = new ProtocolParserProtobuf();
+  p->AppendStream(nsCString(s.c_str(), s.length()));
+  p->End();
+  ASSERT_EQ(p->UpdateWaitSec(), 9u);
+  delete p;
+}
+
+static void
+InitUpdateResponse(ListUpdateResponse* aUpdateResponse,
+                   ThreatType aThreatType,
+                   const nsACString& aState,
+                   const nsACString& aChecksum,
+                   bool isFullUpdate,
+                   const nsTArray<uint32_t>& aFixedLengthPrefixes)
+{
+  aUpdateResponse->set_threat_type(aThreatType);
+  aUpdateResponse->set_new_client_state(aState.BeginReading(), aState.Length());
+  aUpdateResponse->mutable_checksum()->set_sha256(aChecksum.BeginReading(), aChecksum.Length());
+  aUpdateResponse->set_response_type(isFullUpdate ? ListUpdateResponse::FULL_UPDATE
+                                                  : ListUpdateResponse::PARTIAL_UPDATE);
+
+  auto additions = aUpdateResponse->mutable_additions()->Add();
+  additions->set_compression_type(RAW);
+  auto rawHashes = additions->mutable_raw_hashes();
+  rawHashes->set_prefix_size(4);
+  auto prefixes = rawHashes->mutable_raw_hashes();
+  for (auto p : aFixedLengthPrefixes) {
+    char buffer[4];
+    NativeEndian::copyAndSwapToBigEndian(buffer, &p, 1);
+    prefixes->append(buffer, 4);
+  }
+}
+
+static void DumpBinary(const nsACString& aBinary)
+{
+  nsCString s;
+  for (size_t i = 0; i < aBinary.Length(); i++) {
+    s.AppendPrintf("\\x%.2X", (uint8_t)aBinary[i]);
+  }
+  printf("%s\n", s.get());
+}
\ No newline at end of file
--- a/toolkit/components/url-classifier/tests/gtest/moz.build
+++ b/toolkit/components/url-classifier/tests/gtest/moz.build
@@ -6,16 +6,17 @@
 
 LOCAL_INCLUDES += [
     '../..',
 ]
 
 UNIFIED_SOURCES += [
     'TestChunkSet.cpp',
     'TestPerProviderDirectory.cpp',
+    'TestProtocolParser.cpp',
     'TestRiceDeltaDecoder.cpp',
     'TestSafebrowsingHash.cpp',
     'TestSafeBrowsingProtobuf.cpp',
     'TestTable.cpp',
     'TestUrlClassifierTableUpdateV4.cpp',
     'TestUrlClassifierUtils.cpp',
     'TestVariableLengthPrefixSet.cpp',
 ]