Skip to content

Commit

Permalink
Make relative file url parsing fail where there is a host:port in the…
Browse files Browse the repository at this point in the history 
… relative URL.

BUG=285720
R=brettw@chromium.org, jar@chromium.org

Review URL: https://codereview.chromium.org/23464046

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223928 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tsepez@chromium.org committed Sep 18, 2013
1 parent 7105335 commit 4cee663
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 24 deletions.
2 changes: 2 additions & 0 deletions chrome/renderer/net/net_error_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ namespace {

bool IsLoadingErrorPage(WebKit::WebFrame* frame) {
GURL url = frame->provisionalDataSource()->request().url();
if (!url.is_valid())
return false;
return url.spec() == kUnreachableWebDataURL;
}

Expand Down
2 changes: 1 addition & 1 deletion url/url_canon_relative.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ bool DoResolveRelativeURL(const char* base_url,
// handles the special case where the URL is only slashes, since that
// doesn't have a host part either.
if (base_is_file &&
(num_slashes > 2 || num_slashes == relative_component.len)) {
(num_slashes >= 2 || num_slashes == relative_component.len)) {
return DoResolveAbsoluteFile(relative_url, relative_component,
query_converter, output, out_parsed);
}
Expand Down
5 changes: 5 additions & 0 deletions url/url_canon_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2001,6 +2001,11 @@ TEST(URLCanonTest, ResolveRelativeURL) {
// is not file.
{"http://host/a", true, false, "/c:\\foo", true, true, true, "http://host/c:/foo"},
{"http://host/a", true, false, "//c:\\foo", true, true, true, "http://c/foo"},
// Ensure that ports aren't allowed for hosts relative to a file url.
// Although the result string shows a host:port portion, the call to
// resolve the relative URL returns false, indicating parse failure,
// which is what is required.
{"file:///foo.txt", true, true, "//host:80/bar.txt", true, true, false, "file://host:80/bar.txt"},
// Filesystem URL tests; filesystem URLs are only valid and relative if
// they have no scheme, e.g. "./index.html". There's no valid equivalent
// to http:index.html.
Expand Down
18 changes: 11 additions & 7 deletions url/url_parse_file.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ void DoParseLocalFile(const CHAR* spec,
}

// Backend for the external functions that operates on either char type.
// We are handed the character after the "file:" at the beginning of the spec.
// Usually this is a slash, but needn't be; we allow paths like "file:c:\foo".
// Handles cases where there is a scheme, but also when handed the first
// character following the "file:" at the beginning of the spec. If so,
// this is usually a slash, but needn't be; we allow paths like "file:c:\foo".
template<typename CHAR>
void DoParseFileURL(const CHAR* spec, int spec_len, Parsed* parsed) {
DCHECK(spec_len >= 0);
Expand All @@ -130,8 +131,8 @@ void DoParseFileURL(const CHAR* spec, int spec_len, Parsed* parsed) {
int begin = 0;
TrimURL(spec, &begin, &spec_len);

// Find the scheme.
int num_slashes;
// Find the scheme, if any.
int num_slashes = CountConsecutiveSlashes(spec, begin, spec_len);
int after_scheme;
int after_slashes;
#ifdef WIN32
Expand All @@ -140,7 +141,6 @@ void DoParseFileURL(const CHAR* spec, int spec_len, Parsed* parsed) {
// links like "c:/foo/bar" or "//foo/bar". This is also called by the
// relative URL resolver when it determines there is an absolute URL, which
// may give us input like "/c:/foo".
num_slashes = CountConsecutiveSlashes(spec, begin, spec_len);
after_slashes = begin + num_slashes;
if (DoesBeginWindowsDriveSpec(spec, after_slashes, spec_len)) {
// Windows path, don't try to extract the scheme (for example, "c:\foo").
Expand All @@ -153,7 +153,12 @@ void DoParseFileURL(const CHAR* spec, int spec_len, Parsed* parsed) {
} else
#endif
{
if (ExtractScheme(&spec[begin], spec_len - begin, &parsed->scheme)) {
// ExtractScheme doesn't understand the possibility of filenames with
// colons in them, in which case it returns the entire spec up to the
// colon as the scheme. So handle /foo.c:5 as a file but foo.c:5 as
// the foo.c: scheme.
if (!num_slashes &&
ExtractScheme(&spec[begin], spec_len - begin, &parsed->scheme)) {
// Offset the results since we gave ExtractScheme a substring.
parsed->scheme.begin += begin;
after_scheme = parsed->scheme.end() + 1;
Expand All @@ -173,7 +178,6 @@ void DoParseFileURL(const CHAR* spec, int spec_len, Parsed* parsed) {
}

num_slashes = CountConsecutiveSlashes(spec, after_scheme, spec_len);

after_slashes = after_scheme + num_slashes;
#ifdef WIN32
// Check whether the input is a drive again. We checked above for windows
Expand Down
96 changes: 80 additions & 16 deletions url/url_parse_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,9 @@ TEST(URLParser, PathURL) {
}
}

#ifdef WIN32

// WindowsFile ----------------------------------------------------------------

// Various incarnations of file URLs. These are for Windows only.
// Various incarnations of file URLs.
static URLParseCase file_cases[] = {
#ifdef WIN32
{"file:server", "file", NULL, NULL, "server", -1, NULL, NULL, NULL},
{" file: server \t", "file", NULL, NULL, " server",-1, NULL, NULL, NULL},
{"FiLe:c|", "FiLe", NULL, NULL, NULL, -1, "c|", NULL, NULL},
Expand Down Expand Up @@ -404,29 +401,96 @@ static URLParseCase file_cases[] = {
// Queries and refs are valid for file URLs as well.
{"file:///C:/foo.html?#", "file", NULL, NULL, NULL, -1, "/C:/foo.html", "", ""},
{"file:///C:/foo.html?query=yes#ref", "file", NULL, NULL, NULL, -1, "/C:/foo.html", "query=yes", "ref"},
#else // WIN32
// No slashes.
{"file:", "file", NULL, NULL, NULL, -1, NULL, NULL, NULL},
{"file:path", "file", NULL, NULL, NULL, -1, "path", NULL, NULL},
{"file:path/", "file", NULL, NULL, NULL, -1, "path/", NULL, NULL},
{"file:path/f.txt", "file", NULL, NULL, NULL, -1, "path/f.txt", NULL, NULL},
// One slash.
{"file:/", "file", NULL, NULL, NULL, -1, "/", NULL, NULL},
{"file:/path", "file", NULL, NULL, NULL, -1, "/path", NULL, NULL},
{"file:/path/", "file", NULL, NULL, NULL, -1, "/path/", NULL, NULL},
{"file:/path/f.txt", "file", NULL, NULL, NULL, -1, "/path/f.txt", NULL, NULL},
// Two slashes.
{"file://", "file", NULL, NULL, NULL, -1, NULL, NULL, NULL},
{"file://server", "file", NULL, NULL, "server", -1, NULL, NULL, NULL},
{"file://server/", "file", NULL, NULL, "server", -1, "/", NULL, NULL},
{"file://server/f.txt", "file", NULL, NULL, "server", -1, "/f.txt", NULL, NULL},
// Three slashes.
{"file:///", "file", NULL, NULL, NULL, -1, "/", NULL, NULL},
{"file:///path", "file", NULL, NULL, NULL, -1, "/path", NULL, NULL},
{"file:///path/", "file", NULL, NULL, NULL, -1, "/path/", NULL, NULL},
{"file:///path/f.txt", "file", NULL, NULL, NULL, -1, "/path/f.txt", NULL, NULL},
// More than three slashes.
{"file:////", "file", NULL, NULL, NULL, -1, "/", NULL, NULL},
{"file:////path", "file", NULL, NULL, NULL, -1, "/path", NULL, NULL},
{"file:////path/", "file", NULL, NULL, NULL, -1, "/path/", NULL, NULL},
{"file:////path/f.txt", "file", NULL, NULL, NULL, -1, "/path/f.txt", NULL, NULL},
// Schemeless URLs
{"path/f.txt", NULL, NULL, NULL, NULL, -1, "path/f.txt", NULL, NULL},
{"path:80/f.txt", "path", NULL, NULL, NULL, -1, "80/f.txt", NULL, NULL},
{"path/f.txt:80", "path/f.txt",NULL, NULL, NULL, -1, "80", NULL, NULL}, // Wrong.
{"/path/f.txt", NULL, NULL, NULL, NULL, -1, "/path/f.txt", NULL, NULL},
{"/path:80/f.txt", NULL, NULL, NULL, NULL, -1, "/path:80/f.txt",NULL, NULL},
{"/path/f.txt:80", NULL, NULL, NULL, NULL, -1, "/path/f.txt:80",NULL, NULL},
{"//server/f.txt", NULL, NULL, NULL, "server", -1, "/f.txt", NULL, NULL},
{"//server:80/f.txt", NULL, NULL, NULL, "server:80",-1, "/f.txt", NULL, NULL},
{"//server/f.txt:80", NULL, NULL, NULL, "server", -1, "/f.txt:80", NULL, NULL},
{"///path/f.txt", NULL, NULL, NULL, NULL, -1, "/path/f.txt", NULL, NULL},
{"///path:80/f.txt", NULL, NULL, NULL, NULL, -1, "/path:80/f.txt",NULL, NULL},
{"///path/f.txt:80", NULL, NULL, NULL, NULL, -1, "/path/f.txt:80",NULL, NULL},
{"////path/f.txt", NULL, NULL, NULL, NULL, -1, "/path/f.txt", NULL, NULL},
{"////path:80/f.txt", NULL, NULL, NULL, NULL, -1, "/path:80/f.txt",NULL, NULL},
{"////path/f.txt:80", NULL, NULL, NULL, NULL, -1, "/path/f.txt:80",NULL, NULL},
// Queries and refs are valid for file URLs as well.
{"file:///foo.html?#", "file", NULL, NULL, NULL, -1, "/foo.html", "", ""},
{"file:///foo.html?q=y#ref", "file", NULL, NULL, NULL, -1, "/foo.html", "q=y", "ref"},
#endif // WIN32
};

TEST(URLParser, WindowsFile) {
TEST(URLParser, ParseFileURL) {
// Declared outside for loop to try to catch cases in init() where we forget
// to reset something that is reset by the construtor.
url_parse::Parsed parsed;
for (int i = 0; i < arraysize(file_cases); i++) {
for (size_t i = 0; i < arraysize(file_cases); i++) {
const char* url = file_cases[i].input;
url_parse::ParseFileURL(url, static_cast<int>(strlen(url)), &parsed);
int port = url_parse::ParsePort(url, parsed.port);

EXPECT_TRUE(ComponentMatches(url, file_cases[i].scheme, parsed.scheme));
EXPECT_TRUE(ComponentMatches(url, file_cases[i].username, parsed.username));
EXPECT_TRUE(ComponentMatches(url, file_cases[i].password, parsed.password));
EXPECT_TRUE(ComponentMatches(url, file_cases[i].host, parsed.host));
EXPECT_EQ(file_cases[i].port, port);
EXPECT_TRUE(ComponentMatches(url, file_cases[i].path, parsed.path));
EXPECT_TRUE(ComponentMatches(url, file_cases[i].query, parsed.query));
EXPECT_TRUE(ComponentMatches(url, file_cases[i].ref, parsed.ref));
EXPECT_TRUE(ComponentMatches(url, file_cases[i].scheme, parsed.scheme))
<< " for case #" << i << " [" << url << "] "
<< parsed.scheme.begin << ", " << parsed.scheme.len;

EXPECT_TRUE(ComponentMatches(url, file_cases[i].username, parsed.username))
<< " for case #" << i << " [" << url << "] "
<< parsed.username.begin << ", " << parsed.username.len;

EXPECT_TRUE(ComponentMatches(url, file_cases[i].password, parsed.password))
<< " for case #" << i << " [" << url << "] "
<< parsed.password.begin << ", " << parsed.password.len;

EXPECT_TRUE(ComponentMatches(url, file_cases[i].host, parsed.host))
<< " for case #" << i << " [" << url << "] "
<< parsed.host.begin << ", " << parsed.host.len;

EXPECT_EQ(file_cases[i].port, port)
<< " for case #" << i << " [ " << url << "] " << port;

EXPECT_TRUE(ComponentMatches(url, file_cases[i].path, parsed.path))
<< " for case #" << i << " [" << url << "] "
<< parsed.path.begin << ", " << parsed.path.len;

EXPECT_TRUE(ComponentMatches(url, file_cases[i].query, parsed.query))
<< " for case #" << i << " [" << url << "] "
<< parsed.query.begin << ", " << parsed.query.len;

EXPECT_TRUE(ComponentMatches(url, file_cases[i].ref, parsed.ref))
<< " for case #" << i << " [ "<< url << "] "
<< parsed.query.begin << ", " << parsed.scheme.len;
}
}

#endif // WIN32

TEST(URLParser, ExtractFileName) {
struct FileCase {
Expand Down

0 comments on commit 4cee663

Please sign in to comment.