Adding condition attributes for response headers to Declarative WebRequest

chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.cc

Index: chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.cc
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.cc
index 4d825d9e154226af331e555bf84de0a77d9073df..6cd7bbe7a0b934dc3f7da293f0dccb99e6386f8a 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.cc
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 
 #include "base/logging.h"
+#include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/declarative_webrequest/request_stage.h"
@@ -18,6 +19,11 @@
 #include "net/http/http_request_headers.h"
 #include "net/url_request/url_request.h"
 
+using base::DictionaryValue;
+using base::ListValue;
+using base::StringValue;
+using base::Value;
+
 namespace {
 // Error messages.
 const char kUnknownConditionAttribute[] = "Unknown matching condition: '*'";
@@ -43,7 +49,9 @@ bool WebRequestConditionAttribute::IsKnownType(
     const std::string& instance_type) {
   return
       WebRequestConditionAttributeResourceType::IsMatchingType(instance_type) ||
-      WebRequestConditionAttributeContentType::IsMatchingType(instance_type);
+      WebRequestConditionAttributeContentType::IsMatchingType(instance_type) ||
+      WebRequestConditionAttributeResponseHeaders::IsMatchingType(
+          instance_type);
 }
 
 // static
@@ -52,10 +60,15 @@ WebRequestConditionAttribute::Create(
     const std::string& name,
     const base::Value* value,
     std::string* error) {
+  CHECK(value != NULL && error != NULL);
   if (WebRequestConditionAttributeResourceType::IsMatchingType(name)) {
     return WebRequestConditionAttributeResourceType::Create(name, value, error);
   } else if (WebRequestConditionAttributeContentType::IsMatchingType(name)) {
     return WebRequestConditionAttributeContentType::Create(name, value, error);
+  } else if (WebRequestConditionAttributeResponseHeaders::IsMatchingType(
+      name)) {
+    return WebRequestConditionAttributeResponseHeaders::Create(
+        name, value, error);
   }
 
   *error = ExtensionErrorUtils::FormatErrorMessage(kUnknownConditionAttribute,
@@ -122,7 +135,7 @@ int WebRequestConditionAttributeResourceType::GetStages() const {
 }
 
 bool WebRequestConditionAttributeResourceType::IsFulfilled(
-    const WebRequestRule::RequestData& request_data) {
+    const WebRequestRule::RequestData& request_data) const {
   if (!(request_data.stage & GetStages()))
     return false;
   const content::ResourceRequestInfo* info =
@@ -193,7 +206,7 @@ int WebRequestConditionAttributeContentType::GetStages() const {
 }
 
 bool WebRequestConditionAttributeContentType::IsFulfilled(
-    const WebRequestRule::RequestData& request_data) {
+    const WebRequestRule::RequestData& request_data) const {
   if (!(request_data.stage & GetStages()))
     return false;
   std::string content_type;
@@ -219,4 +232,244 @@ WebRequestConditionAttributeContentType::GetType() const {
   return CONDITION_CONTENT_TYPE;
 }
 
+//
+// WebRequestConditionAttributeResponseHeaders
+//
+
+WebRequestConditionAttributeResponseHeaders::MatchTest::MatchTest(
+    scoped_ptr<const std::string> data,
+    MatchType type)
+    : data_(data.Pass()),
+      type_(type) {
+  CHECK(data_.get() != NULL);
+}
+
+WebRequestConditionAttributeResponseHeaders::MatchTest::~MatchTest() {}
+
+WebRequestConditionAttributeResponseHeaders::TestGroup::TestGroup(
+    ScopedVector<const MatchTest>* name,
+    ScopedVector<const MatchTest>* value)
+    : name_(name->Pass()),
+      value_(value->Pass()) {}
+
+WebRequestConditionAttributeResponseHeaders::TestGroup::~TestGroup() {}
+
+WebRequestConditionAttributeResponseHeaders::
+WebRequestConditionAttributeResponseHeaders(bool positive_test)
+    : positive_test_(positive_test) {}
+
+WebRequestConditionAttributeResponseHeaders::
+~WebRequestConditionAttributeResponseHeaders() {}
+
+// static
+bool WebRequestConditionAttributeResponseHeaders::IsMatchingType(
+    const std::string& instance_type) {
+  return instance_type == keys::kResponseHeadersKey ||
+      instance_type == keys::kExcludeResponseHeadersKey;
+}
+
+// static
+scoped_ptr<WebRequestConditionAttribute>
+WebRequestConditionAttributeResponseHeaders::Create(
+    const std::string& name,
+    const base::Value* value,
+    std::string* error) {
+  DCHECK(IsMatchingType(name));
+
+  const ListValue* value_as_list = NULL;
+  if (!value->GetAsList(&value_as_list)) {
+    *error = ExtensionErrorUtils::FormatErrorMessage(kInvalidValue, name);
+    return scoped_ptr<WebRequestConditionAttribute>(NULL);
+  }
+
+  ScopedVector<const TestGroup> groups;
+  for (ListValue::const_iterator it = value_as_list->begin();
+       it != value_as_list->end(); ++it) {
+    const DictionaryValue* tests = NULL;
+    if (!(*it)->GetAsDictionary(&tests)) {
+      *error = ExtensionErrorUtils::FormatErrorMessage(kInvalidValue, name);
+      return scoped_ptr<WebRequestConditionAttribute>(NULL);
+    }
+
+    scoped_ptr<const TestGroup> group(GetTests(tests, error).Pass());

[battre, 2012/08/24 16:24:41]

Is this Pass necessary?

[vabr (Chromium), 2012/08/27 12:11:50]

I don't understand why, but it is not (at least with my GCC). :)
I removed it.

+    if (group.get() == NULL)
+      return scoped_ptr<WebRequestConditionAttribute>(NULL);
+    groups.push_back(group.release());
+  }
+
+  scoped_ptr<WebRequestConditionAttributeResponseHeaders> result;
+  result.reset(new WebRequestConditionAttributeResponseHeaders(
+      name == keys::kResponseHeadersKey));
+  result->tests_.swap(groups);

[battre, 2012/08/24 16:24:41]

nit: Why don't you pass a pointer to groups to the constructor and let result do
the swapping? by calling ->Pass()

[vabr (Chromium), 2012/08/27 12:11:50]

Done.

+
+  return result.PassAs<WebRequestConditionAttribute>();
+}
+
+int WebRequestConditionAttributeResponseHeaders::GetStages() const {
+  return ON_HEADERS_RECEIVED;
+}
+
+bool WebRequestConditionAttributeResponseHeaders::IsFulfilled(
+    const WebRequestRule::RequestData& request_data) const {
+  if (!(request_data.stage & GetStages()))
+    return false;
+
+  const net::HttpResponseHeaders* headers =
+      request_data.original_response_headers;
+  if (headers == NULL) {
+    // Each header of an empty set satisfies (the negation of) everything;
+    // OTOH, there is no header to satisfy even the most permissive test.
+    return !positive_test_;
+  }
+
+  // |success| == for some |i|, has there been a name-value header pair
+  // satisfying all tests from |tests_[i]|?
+  bool success = false;
+
+  for (size_t i = 0; !success && i < tests_.size(); ++i) {
+    std::string name;
+    std::string value;
+    // Has some header already passed the current test group?
+    bool header_found = false;

[battre, 2012/08/24 16:24:41]

move this to line 327 and remove success?

[vabr (Chromium), 2012/08/27 12:11:50]

Done.

+
+    for (void* iter = NULL;
+         !header_found && headers->EnumerateHeaderLines(&iter, &name, &value);
+         /* No increment here, this is done inside EnumerateHeaderLines. */) {
+      StringToLowerASCII(&name);  // Header names are case-insensitive.
+      header_found |= tests_[i]->Matches(name, value);

[vabr (Chromium), 2012/08/24 15:10:07]

If one header occurs on multiple lines, then we test the header name as many
times against the whole test group, as many lines that header occupies.

I am not sure how much complexity this adds and how would it compare to caching
the test results for header names. If you feel it would be better to cache the
test results for headers, just please let me know, and I'll add it.

+    }
+
+    success |= header_found;
+  }
+
+  return (positive_test_ ? success : !success);
+}
+
+WebRequestConditionAttribute::Type
+WebRequestConditionAttributeResponseHeaders::GetType() const {
+  return CONDITION_REQUEST_HEADERS;
+}
+
+bool WebRequestConditionAttributeResponseHeaders::MatchTest::Matches(
+    const std::string& str) const {
+  switch (type_) {
+    case kPrefix:
+      return StartsWithASCII(str, data(), true /*case_sensitive*/);

[battre, 2012/08/24 16:24:41]

you can replace data() with data_ and remove data() entirely.

[vabr (Chromium), 2012/08/27 12:11:50]

Done.

+    case kSuffix:
+      return EndsWith(str, data(), true /*case_sensitive*/);
+    case kEquals:
+      return data() == str;
+    case kContains:
+      return str.find(data()) != std::string::npos;
+  }
+  // We never get past the "switch", but the compiler worries about no return.
+  NOTREACHED();
+  return false;
+}
+
+bool WebRequestConditionAttributeResponseHeaders::TestGroup::Matches(
+    const std::string& name,
+    const std::string& value) const {
+  // |header_success| == have all tests for this header passed so far?
+  bool header_success = true;
+  for (size_t j = 0; header_success && j < name_.size(); ++j) {

[battre, 2012/08/24 16:24:41]

nit: use i as iterator? also in line 383

[vabr (Chromium), 2012/08/27 12:11:50]

Done.

+    header_success &= name_[j]->Matches(name);
+  }
+  if (!header_success)
+    return false;
+
+  // |value_success| == have all tests for this value passed so far?
+  bool value_success = true;
+  for (size_t j = 0; value_success && j < value_.size(); ++j) {
+    value_success &= value_[j]->Matches(value);
+  }
+
+  // Here we already know that header_success == true, just check values.
+  return value_success;

[battre, 2012/08/24 16:24:41]

you could make this function slightly shorter:
for (size_t i = 0; i < name_.size(); ++j) {
  if (!name_[i]->Matches(name))
    return false;
}
for (size_t i = 0; i < value_.size(); ++j) {
  if (!value_[i]->Matches(name))
    return false;
}
return false;

[vabr (Chromium), 2012/08/27 12:11:50]

Done (except for making the last return true :)).

+}
+
+
+// static
+scoped_ptr<const WebRequestConditionAttributeResponseHeaders::TestGroup>
+WebRequestConditionAttributeResponseHeaders::GetTests(
+    const DictionaryValue* tests,
+    std::string* error) {
+  ScopedVector<const MatchTest> name;
+  ScopedVector<const MatchTest> value;
+
+  for (DictionaryValue::key_iterator key = tests->begin_keys();
+       key != tests->end_keys();
+       ++key) {
+    bool is_name = false;  // Is this test for header name?
+    MatchType match_type;
+    if (*key == keys::kNamePrefixKey) {
+      is_name = true;
+      match_type = kPrefix;
+    } else if (*key == keys::kNameSuffixKey) {
+      is_name = true;
+      match_type = kSuffix;
+    } else if (*key == keys::kNameContainsKey) {
+      is_name = true;
+      match_type = kContains;
+    } else if (*key == keys::kNameEqualsKey) {
+      is_name = true;
+      match_type = kEquals;
+    } else if (*key == keys::kValuePrefixKey) {
+      match_type = kPrefix;
+    } else if (*key == keys::kValueSuffixKey) {
+      match_type = kSuffix;
+    } else if (*key == keys::kValueContainsKey) {
+      match_type = kContains;
+    } else if (*key == keys::kValueEqualsKey) {
+      match_type = kEquals;
+    } else {
+      NOTREACHED();  // JSON schema type checking should prevent this.
+      *error = ExtensionErrorUtils::FormatErrorMessage(kInvalidValue, *key);
+      return scoped_ptr<const TestGroup>(NULL);
+    }
+    const Value* content = NULL;
+    // This should not fire, we already checked that |key| is there.
+    CHECK(tests->Get(*key, &content));
+
+    switch (content->GetType()) {
+      case Value::TYPE_LIST: {
+        const ListValue* list = NULL;
+        CHECK(content->GetAsList(&list));
+        for (ListValue::const_iterator it = list->begin();
+             it != list->end(); ++it) {
+          ScopedVector<const MatchTest>* tests = is_name ? &name : &value;
+          tests->push_back(GetMatchTest(*it, is_name, match_type).release());
+        }
+        break;
+      }
+      case Value::TYPE_STRING: {
+        ScopedVector<const MatchTest>* tests = is_name ? &name : &value;
+        tests->push_back(GetMatchTest(content, is_name, match_type).release());
+        break;
+      }
+      default: {
+        NOTREACHED();  // JSON schema type checking should prevent this.
+        *error = ExtensionErrorUtils::FormatErrorMessage(kInvalidValue, *key);
+        return scoped_ptr<const TestGroup>(NULL);
+      }
+    }
+  }
+
+  return scoped_ptr<const TestGroup>(new TestGroup(&name, &value));

[battre, 2012/08/24 16:24:41]

FYI: we can now use make_scoped_ptr as well, i.e.
return make_scoped_ptr(new TestGroup(&name, &value);

[vabr (Chromium), 2012/08/27 12:11:50]

Thanks, I did not know that.
As you only wrote "FYI" and not "change this" I left it as it is, because in
this case it seems to improve readability -- it is clear what type we return
without seeking one page up and reading the ugly "const
WebRequestConditionAttributeResponseHeaders::TestGroup".

If you let me know that you want me to change that, I will do that, though.

+}
+
+// static
+scoped_ptr<const WebRequestConditionAttributeResponseHeaders::MatchTest>
+WebRequestConditionAttributeResponseHeaders::GetMatchTest(
+    const Value* content, bool is_name_test, MatchType match_type) {
+  scoped_ptr<std::string> str(new std::string);
+
+  CHECK(content->GetAsString(str.get()));
+  if (is_name_test)  // Header names are case-insensitive.
+    StringToLowerASCII(str.get());
+
+  return scoped_ptr<const MatchTest>(
+      new MatchTest(str.PassAs<const std::string>(), match_type));
+}
+
 }  // namespace extensions