Bug 1551399 part 2. Stop using [array] in url-classifier's makeFindFullHashRequestV4. r=dimi,gcp

Differential Revision: https://phabricator.services.mozilla.com/D31022
This commit is contained in:
Boris Zbarsky
2019-05-14 09:57:16 +00:00
parent d288f51dc3
commit a9b6918cce
6 changed files with 41 additions and 70 deletions

View File

@@ -530,9 +530,7 @@ HashCompleterRequest.prototype = {
return gUrlUtil.makeFindFullHashRequestV4(tableNameArray,
stateArray,
prefixArray,
tableNameArray.length,
prefixArray.length);
prefixArray);
},
// Returns a string for the request body based on the contents of

View File

@@ -124,16 +124,14 @@ interface nsIUrlClassifierUtils : nsISupports
* @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
* @param aPrefixCount The array length of aPrefixes
*
* The aListNames and aListStatesBase64 arrays must be the same length.
*
* @returns A base64url encoded string.
*/
ACString makeFindFullHashRequestV4([array, size_is(aListCount)] in string aListNames,
[array, size_is(aListCount)] in string aListStatesBase64,
[array, size_is(aPrefixCount)] in string aPrefixes,
in uint32_t aListCount,
in uint32_t aPrefixCount);
ACString makeFindFullHashRequestV4(in Array<ACString> aListNames,
in Array<ACString> aListStatesBase64,
in Array<ACString> aPrefixes);
/**
* Make ThreatHit report request body.

View File

@@ -438,12 +438,14 @@ nsUrlClassifierUtils::MakeUpdateRequestV4(
}
NS_IMETHODIMP
nsUrlClassifierUtils::MakeFindFullHashRequestV4(const char** aListNames,
const char** aListStatesBase64,
const char** aPrefixesBase64,
uint32_t aListCount,
uint32_t aPrefixCount,
nsACString& aRequest) {
nsUrlClassifierUtils::MakeFindFullHashRequestV4(
const nsTArray<nsCString>& aListNames,
const nsTArray<nsCString>& aListStatesBase64,
const nsTArray<nsCString>& aPrefixesBase64, nsACString& aRequest) {
if (aListNames.Length() != aListStatesBase64.Length()) {
return NS_ERROR_INVALID_ARG;
}
FindFullHashesRequest r;
r.set_allocated_client(CreateClientInfo());
@@ -456,17 +458,16 @@ nsUrlClassifierUtils::MakeFindFullHashRequestV4(const char** aListNames,
PlatformType platform = GetPlatformType();
// 1) Set threat types.
for (uint32_t i = 0; i < aListCount; i++) {
for (uint32_t i = 0; i < aListNames.Length(); i++) {
// Add threat types.
uint32_t threatType;
rv = ConvertListNameToThreatType(nsDependentCString(aListNames[i]),
&threatType);
rv = ConvertListNameToThreatType(aListNames[i], &threatType);
NS_ENSURE_SUCCESS(rv, rv);
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;
}
@@ -483,7 +484,7 @@ nsUrlClassifierUtils::MakeFindFullHashRequestV4(const char** aListNames,
// Add client states for index 'i' only when the threat type is available
// on current platform.
nsCString stateBinary;
rv = Base64Decode(nsDependentCString(aListStatesBase64[i]), stateBinary);
rv = Base64Decode(aListStatesBase64[i], stateBinary);
NS_ENSURE_SUCCESS(rv, rv);
r.add_client_states(stateBinary.get(), stateBinary.Length());
}
@@ -495,9 +496,9 @@ nsUrlClassifierUtils::MakeFindFullHashRequestV4(const char** aListNames,
threatInfo->add_threat_entry_types(URL);
// 4) Set threat entries.
for (uint32_t i = 0; i < aPrefixCount; i++) {
for (const nsCString& prefix : aPrefixesBase64) {
nsCString prefixBinary;
rv = Base64Decode(nsDependentCString(aPrefixesBase64[i]), prefixBinary);
rv = Base64Decode(prefix, prefixBinary);
threatInfo->add_threat_entries()->set_hash(prefixBinary.get(),
prefixBinary.Length());
}

View File

@@ -7,56 +7,36 @@ using namespace mozilla;
using namespace mozilla::safebrowsing;
namespace {
// |Base64EncodedStringArray| and |MakeBase64EncodedStringArray|
// works together to make us able to do things "literally" and easily.
// Given a nsCString array, construct an object which can be implicitly
// casted to |const char**|, where all owning c-style strings have been
// base64 encoded. The memory life cycle of what the "cast operator"
// returns is just as the object itself.
class Base64EncodedStringArray {
public:
Base64EncodedStringArray(nsCString aArray[], size_t N);
operator const char**() const { return (const char**)&mArray[0]; }
private:
// Since we can't guarantee the layout of nsCString (can we?),
// an additional nsTArray<nsCString> is required to manage the
// allocated string.
nsTArray<const char*> mArray;
nsTArray<nsCString> mStringStorage;
};
// Simply used to infer the fixed-array size automatically.
template <size_t N>
Base64EncodedStringArray MakeBase64EncodedStringArray(nsCString (&aArray)[N]) {
return Base64EncodedStringArray(aArray, N);
}
void ToBase64EncodedStringArray(nsCString (&aInput)[N],
nsTArray<nsCString>& aEncodedArray);
} // end of unnamed namespace.
TEST(UrlClassifierFindFullHash, Request)
{
nsUrlClassifierUtils* urlUtil = nsUrlClassifierUtils::GetInstance();
const char* listNames[] = {"test-phish-proto", "test-unwanted-proto"};
nsTArray<nsCString> listNames;
listNames.AppendElement("test-phish-proto");
listNames.AppendElement("test-unwanted-proto");
nsCString listStates[] = {nsCString("sta\x00te1", 7),
nsCString("sta\x00te2", 7)};
nsTArray<nsCString> listStateArray;
ToBase64EncodedStringArray(listStates, listStateArray);
nsCString prefixes[] = {nsCString("\x00\x00\x00\x01", 4),
nsCString("\x00\x00\x00\x00\x01", 5),
nsCString("\x00\xFF\x00\x01", 4),
nsCString("\x00\xFF\x00\x01\x11\x23\xAA\xBC", 8),
nsCString("\x00\x00\x00\x01\x00\x01\x98", 7)};
nsTArray<nsCString> prefixArray;
ToBase64EncodedStringArray(prefixes, prefixArray);
nsCString requestBase64;
nsresult rv;
rv = urlUtil->MakeFindFullHashRequestV4(
listNames, MakeBase64EncodedStringArray(listStates),
MakeBase64EncodedStringArray(prefixes), ArrayLength(listNames),
ArrayLength(prefixes), requestBase64);
rv = urlUtil->MakeFindFullHashRequestV4(listNames, listStateArray,
prefixArray, requestBase64);
ASSERT_TRUE(NS_SUCCEEDED(rv));
// Base64 URL decode first.
@@ -82,8 +62,8 @@ TEST(UrlClassifierFindFullHash, Request)
ASSERT_EQ(threatInfo.threat_types_size(), (int)ArrayLength(listStates));
for (int i = 0; i < threatInfo.threat_types_size(); i++) {
uint32_t expectedThreatType;
rv = urlUtil->ConvertListNameToThreatType(nsCString(listNames[i]),
&expectedThreatType);
rv =
urlUtil->ConvertListNameToThreatType(listNames[i], &expectedThreatType);
ASSERT_TRUE(NS_SUCCEEDED(rv));
ASSERT_EQ(threatInfo.threat_types(i), expectedThreatType);
}
@@ -228,14 +208,14 @@ TEST(UrlClassifierFindFullHash, ParseRequest)
/////////////////////////////////////////////////////////////
namespace {
Base64EncodedStringArray::Base64EncodedStringArray(nsCString aArray[],
size_t N) {
template <size_t N>
void ToBase64EncodedStringArray(nsCString (&aArray)[N],
nsTArray<nsCString>& aEncodedArray) {
for (size_t i = 0; i < N; i++) {
nsCString encoded;
nsresult rv = Base64Encode(aArray[i], encoded);
NS_ENSURE_SUCCESS_VOID(rv);
mStringStorage.AppendElement(encoded);
mArray.AppendElement(encoded.get());
aEncodedArray.AppendElement(encoded);
}
}

View File

@@ -82,9 +82,7 @@ add_test(function test_update_v4() {
add_test(function test_getHashRequestV4() {
let request = gUrlUtil.makeFindFullHashRequestV4([TEST_TABLE_DATA_V4.tableName],
[btoa(NEW_CLIENT_STATE)],
[btoa("0123"), btoa("1234567"), btoa("1111")].sort(),
1,
3);
[btoa("0123"), btoa("1234567"), btoa("1111")].sort());
registerHandlerGethashV4("&$req=" + request);
let completeFinishedCnt = 0;
@@ -187,9 +185,7 @@ add_test(function test_minWaitDuration() {
let request = gUrlUtil.makeFindFullHashRequestV4([TEST_TABLE_DATA_V4.tableName],
[btoa(NEW_CLIENT_STATE)],
[btoa("1234567")],
1,
1);
[btoa("1234567")]);
registerHandlerGethashV4("&$req=" + request);
// The last gethash response contained a min wait duration 12 secs 10 nano

View File

@@ -31,14 +31,12 @@ function testMobileOnlyThreats() {
let requestWithPHA =
urlUtils.makeFindFullHashRequestV4(["goog-phish-proto", "goog-harmful-proto"],
["", ""], // state.
[btoa("0123")], // prefix.
2, 1);
[btoa("0123")]); // prefix.
let requestNoPHA =
urlUtils.makeFindFullHashRequestV4(["goog-phish-proto"],
[""], // state.
[btoa("0123")], // prefix.
1, 1);
[btoa("0123")]); // prefix.
if (AppConstants.platform === "android") {
notEqual(requestWithPHA, requestNoPHA,