From b3b4096aa2247eb71d44bef814d3c6874649c1bc Mon Sep 17 00:00:00 2001 From: Henry Chang Date: Wed, 7 Sep 2016 17:45:15 +0800 Subject: [PATCH] Bug 1301008 - Pass safebrowsing v4 list state in base64 format to avoid truncation. r=francois MozReview-Commit-ID: 6oVdQvEoMm2 --HG-- extra : rebase_source : c239f5682213ec6e4e197d8aa7d267342dcd08f8 --- .../url-classifier/content/listmanager.js | 2 +- .../url-classifier/nsIUrlClassifierUtils.idl | 4 ++-- .../url-classifier/nsUrlClassifierUtils.cpp | 14 +++++++++----- .../url-classifier/tests/unit/test_listmanager.js | 14 +------------- 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/toolkit/components/url-classifier/content/listmanager.js b/toolkit/components/url-classifier/content/listmanager.js index 73afc010940d..60b37442cdfd 100644 --- a/toolkit/components/url-classifier/content/listmanager.js +++ b/toolkit/components/url-classifier/content/listmanager.js @@ -401,7 +401,7 @@ PROT_ListManager.prototype.makeUpdateRequest_ = function(updateUrl, tableData) { // "saving states to HashStore". let statePrefName = "browser.safebrowsing.provider.google4.state." + listName; let stateBase64 = this.prefs_.getPref(statePrefName, ""); - stateArray.push(stateBase64 ? atob(stateBase64) : ""); + stateArray.push(stateBase64); }); let urlUtils = Cc["@mozilla.org/url-classifier/utils;1"] diff --git a/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl b/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl index ce4ad7c0917e..cde833e8359a 100644 --- a/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl +++ b/toolkit/components/url-classifier/nsIUrlClassifierUtils.idl @@ -54,12 +54,12 @@ interface nsIUrlClassifierUtils : nsISupports * 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 (in string) for each list. + * @param aState An array of states (encoded in base64 format) for each list. * @param aCount The array length of aList and aState. * * @returns A string to store request. Not null-terminated. */ ACString makeUpdateRequestV4([array, size_is(aCount)] in string aListNames, - [array, size_is(aCount)] in string aStates, + [array, size_is(aCount)] in string aStatesBase64, in uint32_t aCount); }; diff --git a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp index ce0b97ff2833..4677b2749739 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp +++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp @@ -100,7 +100,7 @@ typedef FetchThreatListUpdatesRequest_ListUpdateRequest_Constraints Constraints; static void InitListUpdateRequest(ThreatType aThreatType, - const char* aState, + const char* aStateBase64, ListUpdateRequest* aListUpdateRequest) { aListUpdateRequest->set_threat_type(aThreatType); @@ -114,8 +114,12 @@ InitListUpdateRequest(ThreatType aThreatType, aListUpdateRequest->set_allocated_constraints(contraints); // Only set non-empty state. - if (aState[0] != '\0') { - aListUpdateRequest->set_state(aState); + if (aStateBase64[0] != '\0') { + nsCString stateBinary; + nsresult rv = Base64Decode(nsCString(aStateBase64), stateBinary); + if (NS_SUCCEEDED(rv)) { + aListUpdateRequest->set_state(stateBinary.get()); + } } } @@ -267,7 +271,7 @@ nsUrlClassifierUtils::GetProtocolVersion(const nsACString& aProvider, NS_IMETHODIMP nsUrlClassifierUtils::MakeUpdateRequestV4(const char** aListNames, - const char** aStates, + const char** aStatesBase64, uint32_t aCount, nsACString &aRequest) { @@ -284,7 +288,7 @@ nsUrlClassifierUtils::MakeUpdateRequestV4(const char** aListNames, continue; // Unknown list name. } auto lur = r.mutable_list_update_requests()->Add(); - InitListUpdateRequest(static_cast(threatType), aStates[i], lur); + InitListUpdateRequest(static_cast(threatType), aStatesBase64[i], lur); } // Then serialize. diff --git a/toolkit/components/url-classifier/tests/unit/test_listmanager.js b/toolkit/components/url-classifier/tests/unit/test_listmanager.js index ce69479b3641..e7d4353aa778 100644 --- a/toolkit/components/url-classifier/tests/unit/test_listmanager.js +++ b/toolkit/components/url-classifier/tests/unit/test_listmanager.js @@ -154,7 +154,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], - [NEW_CLIENT_STATE], + [btoa(NEW_CLIENT_STATE)], 1); gExpectedQueryV4 = "&$req=" + btoa(requestV4); @@ -311,24 +311,12 @@ function readFileToString(aFilename) { return buf; } -function buildUpdateRequestV4InBase64() { - - let request = urlUtils.makeUpdateRequestV4([TEST_TABLE_DATA_V4.tableName], - [""], - 1); - return btoa(request); -} - function waitUntilStateSavedToPref(expectedState, callback) { const STATE_PREF_NAME_PREFIX = 'browser.safebrowsing.provider.google4.state.'; let stateBase64 = ''; try { - // The reason we get pref from 'googpub-phish-proto' instead of - // 'test-phish-proto' is 'googpub-phish-proto' would be returned - // while we look up the list name from SOCIAL_ENGINEERING_PUBLIC. - // See nsUrlClassifierUtils::THREAT_TYPE_CONV_TABLE. stateBase64 = prefBranch.getCharPref(STATE_PREF_NAME_PREFIX + 'test-phish-proto'); } catch (e) {}