From 482f862c5164302b6732ec5ba2d886336f48864d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 14 May 2019 09:50:42 +0000 Subject: [PATCH] Bug 1551399 part 1. Stop using [array] in url-classifier's makeUpdateRequestV4. r=dimi,gcp Differential Revision: https://phabricator.services.mozilla.com/D31020 --HG-- extra : moz-landing-system : lando --- .../UrlClassifierListManager.jsm | 3 +-- .../url-classifier/nsIUrlClassifierUtils.idl | 8 +++---- .../url-classifier/nsUrlClassifierUtils.cpp | 24 ++++++++++--------- .../tests/unit/test_listmanager.js | 6 ++--- .../unit/test_platform_specific_threats.js | 8 +++---- .../tests/unit/test_safebrowsing_protobuf.js | 8 +++---- 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/toolkit/components/url-classifier/UrlClassifierListManager.jsm b/toolkit/components/url-classifier/UrlClassifierListManager.jsm index 2e23c9f1caa8..01fbcc373ca5 100644 --- a/toolkit/components/url-classifier/UrlClassifierListManager.jsm +++ b/toolkit/components/url-classifier/UrlClassifierListManager.jsm @@ -491,8 +491,7 @@ PROT_ListManager.prototype.makeUpdateRequest_ = function(updateUrl, tableData) { .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 diff --git a/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl b/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl index 97830bb3200a..72250c116ab0 100644 --- a/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl +++ b/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl @@ -110,13 +110,13 @@ interface nsIUrlClassifierUtils : nsISupports * * @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 aListNames, + in Array aStatesBase64); /** * Make "find full hash" request by for the given prefixes. diff --git a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp index 5f1717bff8d3..8836d2e2ce7b 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp +++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp @@ -122,7 +122,7 @@ 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(); @@ -141,9 +141,9 @@ static void InitListUpdateRequest(ThreatType aThreatType, 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()); } @@ -392,19 +392,21 @@ nsUrlClassifierUtils::GetProtocolVersion(const nsACString& aProvider, } NS_IMETHODIMP -nsUrlClassifierUtils::MakeUpdateRequestV4(const char** aListNames, - const char** aStatesBase64, - uint32_t aCount, - nsACString& aRequest) { +nsUrlClassifierUtils::MakeUpdateRequestV4( + const nsTArray& aListNames, + const nsTArray& 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. } @@ -412,7 +414,7 @@ nsUrlClassifierUtils::MakeUpdateRequestV4(const char** aListNames, 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. } diff --git a/toolkit/components/url-classifier/tests/unit/test_listmanager.js b/toolkit/components/url-classifier/tests/unit/test_listmanager.js index ad62ee00128d..e84ca5243ac6 100644 --- a/toolkit/components/url-classifier/tests/unit/test_listmanager.js +++ b/toolkit/components/url-classifier/tests/unit/test_listmanager.js @@ -152,8 +152,7 @@ const SERVER_INVOLVED_TEST_CASE_LIST = [ // 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(); @@ -172,8 +171,7 @@ add_test(function test_partialUpdateV4() { // 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(); diff --git a/toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js b/toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js index 358de0a18495..19f8a5d90e47 100644 --- a/toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js +++ b/toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js @@ -13,10 +13,10 @@ function testMobileOnlyThreats() { (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, @@ -59,10 +59,10 @@ function testDesktopOnlyThreats() { 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, diff --git a/toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js b/toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js index 38b9e3ed3d03..2905e7044ca6 100644 --- a/toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js +++ b/toolkit/components/url-classifier/tests/unit/test_safebrowsing_protobuf.js @@ -3,20 +3,20 @@ function run_test() { .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);