Bug 1551399 part 1. Stop using [array] in url-classifier's makeUpdateRequestV4. r=dimi,gcp
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 14 May 2019 09:50:42 +0000
changeset 535654 d4ee430e90c2c4b21d13b2f1de063928f1b53d1d
parent 535653 b0c2270a44f33555ee54c4708f4b35a039375696
child 535655 3d3527abff681615f5a3dde9bd844b60b80f292d
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdimi, gcp
bugs1551399
milestone68.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 1551399 part 1. Stop using [array] in url-classifier's makeUpdateRequestV4. r=dimi,gcp Differential Revision: https://phabricator.services.mozilla.com/D31020
toolkit/components/url-classifier/UrlClassifierListManager.jsm
toolkit/components/url-classifier/nsIUrlClassifierUtils.idl
toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
toolkit/components/url-classifier/tests/unit/test_listmanager.js
toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js
toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js
--- a/toolkit/components/url-classifier/UrlClassifierListManager.jsm
+++ b/toolkit/components/url-classifier/UrlClassifierListManager.jsm
@@ -486,18 +486,17 @@ PROT_ListManager.prototype.makeUpdateReq
     });
 
     log("stateArray: " + stateArray);
 
     let urlUtils = Cc["@mozilla.org/url-classifier/utils;1"]
                      .getService(Ci.nsIUrlClassifierUtils);
 
     streamerMap.requestPayload = urlUtils.makeUpdateRequestV4(tableArray,
-                                                              stateArray,
-                                                              tableArray.length);
+                                                              stateArray);
     streamerMap.isPostRequest = false;
   } else {
     // Build the request. For each table already in the database, include the
     // chunk data from the database
     var lines = tableData.split("\n");
     for (var i = 0; i < lines.length; i++) {
       var fields = lines[i].split(";");
       var name = fields[0];
--- a/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl
@@ -105,23 +105,23 @@ interface nsIUrlClassifierUtils : nsISup
    */
   uint32_t convertListNameToThreatType(in ACString listName);
 
   /**
    * Make update request for given lists and their states.
    *
    * @param aListNames An array of list name represented in string.
    * @param aState An array of states (encoded in base64 format) for each list.
-   * @param aCount The array length of aList and aState.
+   *
+   * The two argument arrays must be the same length.
    *
    * @returns A base64url encoded string.
    */
-  ACString makeUpdateRequestV4([array, size_is(aCount)] in string aListNames,
-                               [array, size_is(aCount)] in string aStatesBase64,
-                               in uint32_t aCount);
+  ACString makeUpdateRequestV4(in Array<ACString> aListNames,
+                               in Array<ACString> aStatesBase64);
 
     /**
    * Make "find full hash" request by for the given prefixes.
    *
    * @param aListNames An array of list names represented in string.
    * @param aListStatesBase64 An array of list states represented in base64.
    * @param aPrefixes An array of prefixes for which we'd like to find full hashes..
    * @param aListCount The array length of aListNames
--- a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
@@ -117,17 +117,17 @@ static PlatformType GetPlatformType() {
   return LINUX_PLATFORM;
 #endif
 }
 
 typedef FetchThreatListUpdatesRequest_ListUpdateRequest ListUpdateRequest;
 typedef FetchThreatListUpdatesRequest_ListUpdateRequest_Constraints Constraints;
 
 static void InitListUpdateRequest(ThreatType aThreatType,
-                                  const char* aStateBase64,
+                                  const nsCString& aStateBase64,
                                   ListUpdateRequest* aListUpdateRequest) {
   aListUpdateRequest->set_threat_type(aThreatType);
   PlatformType platform = GetPlatformType();
 #if defined(ANDROID)
   // Temporary hack to fix bug 1441345.
   if ((aThreatType == SOCIAL_ENGINEERING_PUBLIC) ||
       (aThreatType == SOCIAL_ENGINEERING)) {
     platform = LINUX_PLATFORM;
@@ -136,19 +136,19 @@ static void InitListUpdateRequest(Threat
   aListUpdateRequest->set_platform_type(platform);
   aListUpdateRequest->set_threat_entry_type(URL);
 
   Constraints* contraints = new Constraints();
   contraints->add_supported_compressions(RICE);
   aListUpdateRequest->set_allocated_constraints(contraints);
 
   // Only set non-empty state.
-  if (aStateBase64[0] != '\0') {
+  if (!aStateBase64.IsEmpty()) {
     nsCString stateBinary;
-    nsresult rv = Base64Decode(nsDependentCString(aStateBase64), stateBinary);
+    nsresult rv = Base64Decode(aStateBase64, stateBinary);
     if (NS_SUCCEEDED(rv)) {
       aListUpdateRequest->set_state(stateBinary.get(), stateBinary.Length());
     }
   }
 }
 
 static ClientInfo* CreateClientInfo() {
   ClientInfo* c = new ClientInfo();
@@ -387,37 +387,39 @@ nsUrlClassifierUtils::GetProtocolVersion
   } else {
     aVersion = DEFAULT_PROTOCOL_VERSION;
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsUrlClassifierUtils::MakeUpdateRequestV4(const char** aListNames,
-                                          const char** aStatesBase64,
-                                          uint32_t aCount,
-                                          nsACString& aRequest) {
+nsUrlClassifierUtils::MakeUpdateRequestV4(
+    const nsTArray<nsCString>& aListNames,
+    const nsTArray<nsCString>& aStatesBase64, nsACString& aRequest) {
   using namespace mozilla::safebrowsing;
 
+  if (aListNames.Length() != aStatesBase64.Length()) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   FetchThreatListUpdatesRequest r;
   r.set_allocated_client(CreateClientInfo());
 
-  for (uint32_t i = 0; i < aCount; i++) {
-    nsCString listName(aListNames[i]);
+  for (uint32_t i = 0; i < aListNames.Length(); i++) {
     uint32_t threatType;
-    nsresult rv = ConvertListNameToThreatType(listName, &threatType);
+    nsresult rv = ConvertListNameToThreatType(aListNames[i], &threatType);
     if (NS_FAILED(rv)) {
       continue;  // Unknown list name.
     }
     if (!IsAllowedOnCurrentPlatform(threatType)) {
       NS_WARNING(
           nsPrintfCString(
               "Threat type %d (%s) is unsupported on current platform: %d",
-              threatType, aListNames[i], GetPlatformType())
+              threatType, aListNames[i].get(), GetPlatformType())
               .get());
       continue;  // Some threat types are not available on some platforms.
     }
     auto lur = r.mutable_list_update_requests()->Add();
     InitListUpdateRequest(static_cast<ThreatType>(threatType), aStatesBase64[i],
                           lur);
   }
 
--- a/toolkit/components/url-classifier/tests/unit/test_listmanager.js
+++ b/toolkit/components/url-classifier/tests/unit/test_listmanager.js
@@ -147,18 +147,17 @@ const SERVER_INVOLVED_TEST_CASE_LIST = [
                              TEST_TABLE_DATA_LIST[1].tableName + ";\n" +
                              TEST_TABLE_DATA_LIST[2].tableName + ";\n";
     gUpdateResponse = "n:1000\n";
 
     // We test the request against the query string since v4 request
     // would be appened to the query string. The request is generated
     // by protobuf API (binary) then encoded to base64 format.
     let requestV4 = gUrlUtils.makeUpdateRequestV4([TEST_TABLE_DATA_V4.tableName],
-                                                  [""],
-                                                  1);
+                                                  [""]);
     gExpectedQueryV4 = "&$req=" + requestV4;
 
     forceTableUpdate();
   },
 
 ];
 
 SERVER_INVOLVED_TEST_CASE_LIST.forEach(t => add_test(t));
@@ -167,18 +166,17 @@ add_test(function test_partialUpdateV4()
   disableAllUpdates();
 
   gListManager.enableUpdate(TEST_TABLE_DATA_V4.tableName);
 
   // Since the new client state has been responded and saved in
   // test_update_all_tables, this update request should send
   // a partial update to the server.
   let requestV4 = gUrlUtils.makeUpdateRequestV4([TEST_TABLE_DATA_V4.tableName],
-                                                [btoa(NEW_CLIENT_STATE)],
-                                                1);
+                                                [btoa(NEW_CLIENT_STATE)]);
   gExpectedQueryV4 = "&$req=" + requestV4;
 
   forceTableUpdate();
 });
 
 // Tests nsIUrlListManager.getGethashUrl.
 add_test(function test_getGethashUrl() {
   TEST_TABLE_DATA_LIST.forEach(function(t) {
--- a/toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js
+++ b/toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js
@@ -8,20 +8,20 @@ let urlUtils = Cc["@mozilla.org/url-clas
 
 function testMobileOnlyThreats() {
   // Mobile-only threat type(s):
   //   - goog-harmful-proto (POTENTIALLY_HARMFUL_APPLICATION)
 
   (function testUpdateRequest() {
     let requestWithPHA =
       urlUtils.makeUpdateRequestV4(["goog-phish-proto", "goog-harmful-proto"],
-                                   ["AAAAAA", "AAAAAA"], 2);
+                                   ["AAAAAA", "AAAAAA"]);
 
     let requestNoPHA =
-      urlUtils.makeUpdateRequestV4(["goog-phish-proto"], ["AAAAAA"], 1);
+      urlUtils.makeUpdateRequestV4(["goog-phish-proto"], ["AAAAAA"]);
 
     if (AppConstants.platform === "android") {
       notEqual(requestWithPHA, requestNoPHA,
                "PHA (i.e. goog-harmful-proto) shouldn't be filtered on mobile platform.");
     } else {
       equal(requestWithPHA, requestNoPHA,
             "PHA (i.e. goog-harmful-proto) should be filtered on non-mobile platform.");
     }
@@ -54,20 +54,20 @@ function testDesktopOnlyThreats() {
   // Desktop-only threats:
   //   - goog-downloadwhite-proto (CSD_WHITELIST)
   //   - goog-badbinurl-proto (MALICIOUS_BINARY)
 
   let requestWithDesktopOnlyThreats =
     urlUtils.makeUpdateRequestV4(["goog-phish-proto",
                                   "goog-downloadwhite-proto",
                                   "goog-badbinurl-proto"],
-                                 ["", "", ""], 3);
+                                 ["", "", ""]);
 
   let requestNoDesktopOnlyThreats =
-    urlUtils.makeUpdateRequestV4(["goog-phish-proto"], [""], 1);
+    urlUtils.makeUpdateRequestV4(["goog-phish-proto"], [""]);
 
   if (AppConstants.platform === "android") {
     equal(requestWithDesktopOnlyThreats, requestNoDesktopOnlyThreats,
           "Android shouldn't contain 'goog-downloadwhite-proto' and 'goog-badbinurl-proto'.");
   } else {
     notEqual(requestWithDesktopOnlyThreats, requestNoDesktopOnlyThreats,
              "Desktop should contain 'goog-downloadwhite-proto' and 'goog-badbinurl-proto'.");
   }
--- a/toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js
+++ b/toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js
@@ -1,23 +1,23 @@
 function run_test() {
   let urlUtils = Cc["@mozilla.org/url-classifier/utils;1"]
                    .getService(Ci.nsIUrlClassifierUtils);
 
   // No list at all.
-  let requestNoList = urlUtils.makeUpdateRequestV4([], [], 0);
+  let requestNoList = urlUtils.makeUpdateRequestV4([], []);
 
   // Only one valid list name.
   let requestOneValid =
-    urlUtils.makeUpdateRequestV4(["goog-phish-proto"], ["AAAAAA"], 1);
+    urlUtils.makeUpdateRequestV4(["goog-phish-proto"], ["AAAAAA"]);
 
   // Only one invalid list name.
   let requestOneInvalid =
-    urlUtils.makeUpdateRequestV4(["bad-list-name"], ["AAAAAA"], 1);
+    urlUtils.makeUpdateRequestV4(["bad-list-name"], ["AAAAAA"]);
 
   // One valid and one invalid list name.
   let requestOneInvalidOneValid =
     urlUtils.makeUpdateRequestV4(["goog-phish-proto", "bad-list-name"],
-                                 ["AAAAAA", "AAAAAA"], 2);
+                                 ["AAAAAA", "AAAAAA"]);
 
   equal(requestNoList, requestOneInvalid);
   equal(requestOneValid, requestOneInvalidOneValid);
 }