Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(854)

Unified Diff: components/url_matcher/url_matcher.cc

Issue 219613002: Add support for matching query parameters in URLMatcher (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fixes the review comments from Dominic and Joao Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/url_matcher/url_matcher.cc
diff --git a/components/url_matcher/url_matcher.cc b/components/url_matcher/url_matcher.cc
index de8632d730e4aa408515118b2b026f7ef0c79b3c..bd6828a02f82330a6bed11289840a3540dd463ed 100644
--- a/components/url_matcher/url_matcher.cc
+++ b/components/url_matcher/url_matcher.cc
@@ -196,7 +196,6 @@ bool URLMatcherCondition::IsFullURLCondition() const {
switch (criterion_) {
case HOST_CONTAINS:
case PATH_CONTAINS:
- case QUERY_CONTAINS:
case URL_PREFIX:
case URL_SUFFIX:
case URL_CONTAINS:
@@ -222,9 +221,9 @@ bool URLMatcherCondition::IsMatch(
DCHECK(string_pattern_);
if (!ContainsKey(matching_patterns, string_pattern_->id()))
return false;
- // The criteria HOST_CONTAINS, PATH_CONTAINS, QUERY_CONTAINS are based on
- // a substring match on the raw URL. In case of a match, we need to verify
- // that the match was found in the correct component of the URL.
+ // The criteria HOST_CONTAINS, PATH_CONTAINS are based on a substring match on
+ // the raw URL. In case of a match, we need to verify that the match was found
+ // in the correct component of the URL.
switch (criterion_) {
case HOST_CONTAINS:
return url.host().find(string_pattern_->pattern()) !=
@@ -232,9 +231,6 @@ bool URLMatcherCondition::IsMatch(
case PATH_CONTAINS:
return url.path().find(string_pattern_->pattern()) !=
std::string::npos;
- case QUERY_CONTAINS:
- return url.query().find(string_pattern_->pattern()) !=
- std::string::npos;
default:
break;
}
@@ -250,7 +246,26 @@ namespace {
const char kBeginningOfURL[] = {static_cast<char>(-1), 0};
const char kEndOfDomain[] = {static_cast<char>(-2), 0};
const char kEndOfPath[] = {static_cast<char>(-3), 0};
-const char kEndOfURL[] = {static_cast<char>(-4), 0};
+const char kBeginningOfQueryComponent[] = {static_cast<char>(-4), 0};
+const char kEndOfURL[] = {static_cast<char>(-5), 0};
+const char kQuerySeperator = '&';
+
+// This function prepares the query string by replacing query separator with a
+// magic value (|kBeginningOfQueryComponent|). When the boolean
+// |prepend_beginning_of_query_component| is true the function prepends the
+// query with the same magic. This is done to locate the start of a key value
+// pair in the query string. The parameter |query| is passed by value
+// intentionally, since it is locally modified.
+std::string PrepareQuery(std::string query,
+ bool prepend_beginning_of_query_component) {
+ for (std::string::iterator it = query.begin(); it != query.end(); ++it) {
+ if (*it == kQuerySeperator)
+ *it = kBeginningOfQueryComponent[0];
+ }
+ return prepend_beginning_of_query_component
+ ? kBeginningOfQueryComponent + query
+ : query;
+}
} // namespace
URLMatcherConditionFactory::URLMatcherConditionFactory() : id_counter_(0) {}
@@ -265,7 +280,8 @@ std::string URLMatcherConditionFactory::CanonicalizeURLForComponentSearches(
const GURL& url) const {
return kBeginningOfURL + CanonicalizeHostname(url.host()) + kEndOfDomain +
url.path() + kEndOfPath +
- (url.has_query() ? "?" + url.query() : std::string()) + kEndOfURL;
+ (url.has_query() ? PrepareQuery(url.query(), true) : std::string()) +
+ kEndOfURL;
}
URLMatcherCondition URLMatcherConditionFactory::CreateHostPrefixCondition(
@@ -317,9 +333,9 @@ URLMatcherCondition URLMatcherConditionFactory::CreateQueryPrefixCondition(
const std::string& prefix) {
std::string pattern;
if (!prefix.empty() && prefix[0] == '?')
- pattern = kEndOfPath + prefix;
+ pattern = kEndOfPath + PrepareQuery(prefix.substr(1), true);
else
- pattern = kEndOfPath + ('?' + prefix);
+ pattern = kEndOfPath + PrepareQuery(prefix, true);
return CreateCondition(URLMatcherCondition::QUERY_PREFIX, pattern);
}
@@ -330,25 +346,28 @@ URLMatcherCondition URLMatcherConditionFactory::CreateQuerySuffixCondition(
return CreateQueryEqualsCondition(suffix);
} else {
return CreateCondition(URLMatcherCondition::QUERY_SUFFIX,
- suffix + kEndOfURL);
+ PrepareQuery(suffix, false) + kEndOfURL);
}
}
URLMatcherCondition URLMatcherConditionFactory::CreateQueryContainsCondition(
const std::string& str) {
+ std::string pattern;
if (!str.empty() && str[0] == '?')
- return CreateQueryPrefixCondition(str);
+ pattern = str.substr(1);
else
- return CreateCondition(URLMatcherCondition::QUERY_CONTAINS, str);
+ pattern = str;
+ return CreateCondition(URLMatcherCondition::QUERY_CONTAINS,
+ PrepareQuery(pattern, true));
}
URLMatcherCondition URLMatcherConditionFactory::CreateQueryEqualsCondition(
const std::string& str) {
std::string pattern;
if (!str.empty() && str[0] == '?')
- pattern = kEndOfPath + str + kEndOfURL;
+ pattern = kEndOfPath + PrepareQuery(str.substr(1), true) + kEndOfURL;
else
- pattern = kEndOfPath + ('?' + str) + kEndOfURL;
+ pattern = kEndOfPath + PrepareQuery(str, true) + kEndOfURL;
return CreateCondition(URLMatcherCondition::QUERY_EQUALS, pattern);
}

Powered by Google App Engine
This is Rietveld 408576698