Commit 295d736c authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Chromium LUCI CQ

Reset last_invalid_percent_index if output is invalidated

When a GURL's path is canonicalized, the code is trying to handle
nested escape sequences. It keeps the index of the last seemingly
invalid "%" in a variable called last_invalid_percent_index, so that
ssequences like "%%41B" can be transformed to "%AB".

However, it can happen, that an already output part of the path is
invalidated and cut off, such as in "abc/../", which is equivalent to
"".

The bug in the current code is that last_invalid_percent_index is not
reset if the part of the output which it points to gets invalidated.
This resulted in the mysterious inserting of the hex code of "%" in
the middle of the escape sequence of ">" when canonicalizing the path
"5%/../>%41".

This CL simply makes sure that last_invalid_percent_index is reset
whenever it points beyond the end of the current output.

Bug: 1080890
Change-Id: I04a5246ecbb66b645d796ec88babd74383d18661
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587060
Auto-Submit: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837031}
parent fc6ed361
......@@ -5,6 +5,7 @@
#include <limits.h>
#include "base/check.h"
#include "base/check_op.h"
#include "url/url_canon.h"
#include "url/url_canon_internal.h"
#include "url/url_parse_internal.h"
......@@ -261,6 +262,7 @@ bool DoPartialPath(const CHAR* spec,
bool success = true;
for (int i = path.begin; i < end; i++) {
DCHECK_LT(last_invalid_percent_index, output->length());
UCHAR uch = static_cast<UCHAR>(spec[i]);
if (sizeof(CHAR) > 1 && uch >= 0x80) {
// We only need to test wide input for having non-ASCII characters. For
......@@ -303,6 +305,9 @@ bool DoPartialPath(const CHAR* spec,
break;
case DIRECTORY_UP:
BackUpToPreviousSlash(path_begin_in_output, output);
if (last_invalid_percent_index >= output->length()) {
last_invalid_percent_index = INT_MIN;
}
i += dotlen + consumed_len - 1;
break;
}
......
......@@ -1423,6 +1423,9 @@ TEST(URLCanonTest, CanonicalizeStandardURL) {
{"ws:)W\x1eW\xef\xb9\xaa"
"81:80/",
"ws://%29w%1ew%81/", false},
// Regression test for the last_invalid_percent_index bug described in
// https://crbug.com/1080890#c10.
{R"(HTTP:S/5%\../>%41)", "http://s/%3EA", true},
};
for (size_t i = 0; i < base::size(cases); i++) {
......
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