Commit b42f618b authored by Dan McArdle's avatar Dan McArdle Committed by Commit Bot

Extend copy check to UTF16 inputs in gurl_fuzzer.

In a prior CL [1], I added the CheckIdempotency() function, which tests
that GURL canonicalization produces a fixed point the first time it's
called.

There was some confusion whether CheckIdempotency was duplicating logic
that already existed in the fuzzer; the fuzzer already had a few lines
to copy the GURL with a no-op call to ReplaceComponents and check that
the spec string is unchanged.

This "copy check" is subtly different than the idempotency check. In
particular, it cannot catch the "file:///.//" -> "file:////" ->
"file:///" bug because it never reaches DoParseFileURL.

For context, the non-idempotency bug for "file:///.//" arises because
  * DoPartialPath drops "./", taking "file:///.//" to "file:////"
  * DoParseFileURL collapses "//" to "/", taking "file:////" to
    "file:///".

[1]: http://crrev.com/c/2414615

Bug: 1128999
Change-Id: Ib35982f56cac3ced23d13def74f073a0245bdee3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424764Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarVaclav Brozek <vabr@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810286}
parent e23c56b8
......@@ -3,7 +3,9 @@
// found in the LICENSE file.
#include "base/at_exit.h"
#include "base/check_op.h"
#include "base/i18n/icu_util.h"
#include "base/no_destructor.h"
#include "url/gurl.h"
struct TestCase {
......@@ -15,9 +17,6 @@ struct TestCase {
TestCase* test_case = new TestCase();
// Empty replacements cause no change when applied.
GURL::Replacements* no_op = new GURL::Replacements();
// Checks that GURL's canonicalization is idempotent. This can help discover
// issues like https://crbug.com/1128999.
void CheckIdempotency(const GURL& url) {
......@@ -29,52 +28,62 @@ void CheckIdempotency(const GURL& url) {
CHECK_EQ(spec, recanonicalized.spec());
}
// Checks that |url.spec()| is preserved across a call to ReplaceComponents with
// zero replacements, which is effectively a copy. This can help discover issues
// like https://crbug.com/1075515.
void CheckReplaceComponentsPreservesSpec(const GURL& url) {
static const base::NoDestructor<GURL::Replacements> no_op;
GURL copy = url.ReplaceComponents(*no_op);
CHECK_EQ(url.is_valid(), copy.is_valid());
if (url.is_valid()) {
CHECK_EQ(url.spec(), copy.spec());
}
}
// Entry point for LibFuzzer.
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
if (size < 1)
return 0;
base::StringPiece string_piece_input(reinterpret_cast<const char*>(data),
size);
GURL url_from_string_piece(string_piece_input);
CheckIdempotency(url_from_string_piece);
// Copying by applying empty replacements exercises interesting code paths.
// This can help discover issues like https://crbug.com/1075515.
GURL copy = url_from_string_piece.ReplaceComponents(*no_op);
CHECK_EQ(url_from_string_piece.is_valid(), copy.is_valid());
if (url_from_string_piece.is_valid()) {
CHECK_EQ(url_from_string_piece.spec(), copy.spec());
{
base::StringPiece string_piece_input(reinterpret_cast<const char*>(data),
size);
const GURL url_from_string_piece(string_piece_input);
CheckIdempotency(url_from_string_piece);
CheckReplaceComponentsPreservesSpec(url_from_string_piece);
}
// Test for StringPiece16 if size is even.
if (size % 2 == 0) {
base::StringPiece16 string_piece_input16(
reinterpret_cast<const base::char16*>(data), size / 2);
GURL url_from_string_piece16(string_piece_input16);
const GURL url_from_string_piece16(string_piece_input16);
CheckIdempotency(url_from_string_piece16);
CheckReplaceComponentsPreservesSpec(url_from_string_piece16);
}
// Resolve relative url tests.
size_t size_t_bytes = sizeof(size_t);
if (size < size_t_bytes + 1) {
return 0;
}
size_t relative_size =
*reinterpret_cast<const size_t*>(data) % (size - size_t_bytes);
std::string relative_string(
reinterpret_cast<const char*>(data + size_t_bytes), relative_size);
base::StringPiece string_piece_part_input(
reinterpret_cast<const char*>(data + size_t_bytes + relative_size),
size - relative_size - size_t_bytes);
GURL url_from_string_piece_part(string_piece_part_input);
url_from_string_piece_part.Resolve(relative_string);
{
size_t size_t_bytes = sizeof(size_t);
if (size < size_t_bytes + 1) {
return 0;
}
size_t relative_size =
*reinterpret_cast<const size_t*>(data) % (size - size_t_bytes);
std::string relative_string(
reinterpret_cast<const char*>(data + size_t_bytes), relative_size);
base::StringPiece string_piece_part_input(
reinterpret_cast<const char*>(data + size_t_bytes + relative_size),
size - relative_size - size_t_bytes);
const GURL url_from_string_piece_part(string_piece_part_input);
CheckIdempotency(url_from_string_piece_part);
CheckReplaceComponentsPreservesSpec(url_from_string_piece_part);
url_from_string_piece_part.Resolve(relative_string);
if (relative_size % 2 == 0) {
base::string16 relative_string16(
reinterpret_cast<const base::char16*>(data + size_t_bytes),
relative_size / 2);
url_from_string_piece_part.Resolve(relative_string16);
if (relative_size % 2 == 0) {
base::string16 relative_string16(
reinterpret_cast<const base::char16*>(data + size_t_bytes),
relative_size / 2);
url_from_string_piece_part.Resolve(relative_string16);
}
}
return 0;
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment