Skip to content

GitLab

T
tangled
Commit ec5bd116 authored by David Van Cleve's avatar David Van Cleve Committed by Commit Bot
Browse files
Options

URLPattern: Correctly reject pathless candidates 

A recent change (5f37444e) to GURL::PathForRequest
led to some crashes caused by URLPattern calling PathForRequest
with invalid URLs, which was explicitly prohibited with a DCHECK
prior to the change but, in release builds, had been silently
accepted.

This change makes URLPattern only call GURL::PathForRequest with
valid input, by failing URLPattern::MatchesURL on pathless inputs
prior to the call to PathForRequest.

Test: Added a regression test.

Bug: 1022771
Change-Id: I52896977b6474603d30cbea99ebb776eb3ccf308
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906511
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Auto-Submit: David Van Cleve <davidvc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715320}
parent 702b377e
Show whitespace changes
Inline Side-by-side
Showing with 18 additions and 5 deletions
extensions/common/url_pattern.cc
View file @ ec5bd116
......@@ -414,8 +414,12 @@ bool URLPattern::SetPort(base::StringPiece port) {
}
bool URLPattern::MatchesURL(const GURL& test) const {
// Invalid URLs can never match.
if (!test.is_valid())
return false;
const GURL* test_url = &test;
bool has_inner_url = test.inner_url() != NULL;
bool has_inner_url = test.inner_url() != nullptr;
if (has_inner_url) {
if (!test.SchemeIsFileSystem())
......
extensions/common/url_pattern.h
View file @ ec5bd116
......@@ -27,8 +27,8 @@ class GURL;
//
// * Host is not used when the scheme is 'file'.
// * The path can have embedded '*' characters which act as glob wildcards.
// * '<all_urls>' is a special pattern that matches any URL that contains a
// valid scheme (as specified by valid_schemes_).
// * '<all_urls>' is a special pattern that matches any valid URL that contains
// a valid scheme (as specified by valid_schemes_).
// * The '*' scheme pattern excludes file URLs.
//
// Examples of valid patterns:
......@@ -136,7 +136,7 @@ class URLPattern {
const std::string& path() const { return path_; }
void SetPath(base::StringPiece path);
// Returns true if this pattern matches all urls.
// Returns true if this pattern matches all (valid) urls.
bool match_all_urls() const { return match_all_urls_; }
void SetMatchAllURLs(bool val);
......@@ -152,7 +152,8 @@ class URLPattern {
// false otherwise. Uses valid_schemes_ to determine validity.
bool IsValidScheme(base::StringPiece scheme) const;
// Returns true if this instance matches the specified URL.
// Returns true if this instance matches the specified URL. Always returns
// false for invalid URLs.
bool MatchesURL(const GURL& test) const;
// Returns true if this instance matches the specified security origin.
......
extensions/common/url_pattern_unittest.cc
View file @ ec5bd116
......@@ -370,6 +370,14 @@ TEST(ExtensionURLPatternTest, Match12) {
GURL("data:text/html;charset=utf-8,<html>asdf</html>")));
}
TEST(ExtensionURLPatternTest, DoesntMatchInvalid) {
URLPattern pattern(kAllSchemes);
// Even the all_urls pattern shouldn't match an invalid URL.
EXPECT_EQ(URLPattern::ParseResult::kSuccess,
pattern.Parse(URLPattern::kAllUrlsPattern));
EXPECT_FALSE(pattern.MatchesURL(GURL("http:")));
}
static const struct MatchPatterns {
const char* pattern;
const char* matches;
......
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