Bug 1307541 - ProtocolParserProtobuf to init and return update time properly. r=francois.

MozReview-Commit-ID: CmVWVKUeunJ

--HG--
extra : rebase_source : e8cd28a805b892fd26ca9bede3523a04f1bb5686

toolkit/components/url-classifier/ProtocolParser.cpp

@@ -69,6 +69,7 @@ ParseChunkRange(nsACString::const_iterator& aBegin,
 
 ProtocolParser::ProtocolParser()
   : mUpdateStatus(NS_OK)
+  , mUpdateWaitSec(0)
 {
 }
 
@@ -116,7 +117,6 @@ ProtocolParser::GetTableUpdate(const nsACString& aTable)
 
 ProtocolParserV2::ProtocolParserV2()
   : mState(PROTOCOL_STATE_CONTROL)
-  , mUpdateWait(0)
   , mResetRequested(false)
   , mTableUpdate(nullptr)
 {
@@ -180,8 +180,8 @@ ProtocolParserV2::ProcessControl(bool* aDone)
       // 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")) {
@@ -771,6 +771,10 @@ ProtocolParserProtobuf::End()
     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);

toolkit/components/url-classifier/ProtocolParser.h

@@ -40,6 +40,8 @@ public:
   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;
@@ -52,10 +54,9 @@ public:
 
   // 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:
@@ -73,6 +74,9 @@ protected:
   // 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();
 };
@@ -91,7 +95,6 @@ public:
 
   // 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:
@@ -147,7 +150,6 @@ private:
   };
   ChunkState mChunkState;
 
-  uint32_t mUpdateWait;
   bool mResetRequested;
 
   // Updates to apply to the current table being parsed.

toolkit/components/url-classifier/content/listmanager.js

@@ -204,7 +204,7 @@ PROT_ListManager.prototype.kickoffUpdate_ = function (onDiskTableData)
   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]) {
@@ -492,12 +492,15 @@ PROT_ListManager.prototype.makeUpdateRequestForEntry_ = function(updateUrl,
  *        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
@@ -556,7 +559,7 @@ PROT_ListManager.prototype.updateSuccess_ = function(tableList, updateUrl,
 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);

toolkit/components/url-classifier/nsUrlClassifierDBService.cpp

@@ -392,7 +392,7 @@ void
 nsUrlClassifierDBServiceWorker::ResetUpdate()
 {
   LOG(("ResetUpdate"));
-  mUpdateWait = 0;
+  mUpdateWaitSec = 0;
   mUpdateStatus = NS_OK;
   mUpdateObserver = nullptr;
 }
@@ -543,8 +543,8 @@ nsUrlClassifierDBServiceWorker::FinishStream()
   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 =
@@ -592,8 +592,8 @@ nsUrlClassifierDBServiceWorker::FinishUpdate()
   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."));

toolkit/components/url-classifier/nsUrlClassifierDBService.h

@@ -217,7 +217,7 @@ private:
   // 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

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());
+}

toolkit/components/url-classifier/tests/gtest/moz.build

@@ -11,6 +11,7 @@ LOCAL_INCLUDES += [
 UNIFIED_SOURCES += [
     'TestChunkSet.cpp',
     'TestPerProviderDirectory.cpp',
+    'TestProtocolParser.cpp',
     'TestRiceDeltaDecoder.cpp',
     'TestSafebrowsingHash.cpp',
     'TestSafeBrowsingProtobuf.cpp',