Adding condition attributes for request headers to Declarative WebRequest

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_attribute.h"
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/message_loop.h"
 #include "base/values.h"
@@ skipping 216 matching lines @@
   return dictionary.Pass();
 }
 
 // Returns whether the response headers from |url_request| satisfy the match
 // criteria given in |tests|. For at least one |i| all tests from |tests[i]|
 // must pass. If |positive_test| is true, the dictionary is interpreted as the
 // containsHeaders property of a RequestMatcher, otherwise as
 // doesNotContainHeaders.
 void MatchAndCheck(const std::vector< std::vector<const std::string*> >& tests,
                    const std::string& key,
+                   RequestStage stage,
                    net::URLRequest* url_request,
                    bool* result) {
   ListValue contains_headers;
   for (size_t i = 0; i < tests.size(); ++i) {
     scoped_ptr<DictionaryValue> temp(GetDictionaryFromArray(tests[i]));
     ASSERT_TRUE(temp.get() != NULL);
     contains_headers.Append(temp.release());
   }
 
   std::string error;
   scoped_ptr<WebRequestConditionAttribute> attribute =
       WebRequestConditionAttribute::Create(key, &contains_headers, &error);
   ASSERT_EQ("", error);
   ASSERT_TRUE(attribute.get() != NULL);
 
   *result = attribute->IsFulfilled(WebRequestRule::RequestData(
-      url_request, ON_HEADERS_RECEIVED, url_request->response_headers()));
+      url_request, stage, url_request->response_headers()));
 }
 
 }  // namespace
 
+// Here we test WebRequestConditionAttributeRequestHeaders for matching
+// correctly against request headers. This test is not as extensive as
+// "ResponseHeaders" (below), because the header-matching code is shared
+// by both types of condition attributes, so it is enough to test it once.
+TEST(WebRequestConditionAttributeTest, RequestHeaders) {
+  // Necessary for TestURLRequest.
+  MessageLoop message_loop(MessageLoop::TYPE_IO);
+
+  TestURLRequestContext context;
+  TestDelegate delegate;
+  TestURLRequest url_request(GURL("http://example.com"),  // Dummy URL.
+                             &delegate, &context);
+  url_request.SetExtraRequestHeaderByName(
+      "Custom-header", "custom/value", true /* overwrite */);
+  url_request.Start();
+  MessageLoop::current()->Run();
+
+  std::vector< std::vector<const std::string*> > tests;

[battre, 2012/10/05 12:50:03]

nit: the first space after std::vector< does not belong there.

[vabr (Chromium), 2012/10/05 15:56:53]

Actually, the Google style guide allows it (search for "symmetric spacing"), and
the Chrome style guide does not change that.
I still changed it according to your suggestion, for compatibility reasons,
because you, as an OWNER, seem to prefer it.

+  bool result;

[battre, 2012/10/05 12:50:03]

can you please initialize this to false by default?

[vabr (Chromium), 2012/10/05 15:56:53]

Done.

+
+#define MATCH_REQUEST_HEADERS(key) \
+    MatchAndCheck(tests, key, ON_BEFORE_SEND_HEADERS, &url_request, &result);

[battre, 2012/10/05 12:50:03]

How about defining a variable stage and then repeating MatchAndCheck each time?
This is easier on code search and grep without sacrificing the 80 character
limit.

[vabr (Chromium), 2012/10/05 15:56:53]

Done. Sometimes it overflows, but I think the code search improvement is worth
it.

Also did this below for MATCH_RESPONSE_HEADERS.

+
+  // First set of test data -- passing conjunction.
+  const std::string kPassingCondition[] = {
+    keys::kNameContainsKey, "CuStOm",  // Header names are case insensitive.
+    keys::kNameEqualsKey, "custom-header",
+    keys::kValueSuffixKey, "alue",
+    keys::kValuePrefixKey, "custom/value"
+  };
+  const size_t kPassingConditionSizes[] = { arraysize(kPassingCondition) };
+  GetArrayAsVector(kPassingCondition, kPassingConditionSizes, 1u, &tests);
+  // Positive filter, passing (conjunction of tests).
+  MATCH_REQUEST_HEADERS(keys::kRequestHeadersKey);
+  EXPECT_TRUE(result);
+  // Negative filter, failing (conjunction of tests).
+  MATCH_REQUEST_HEADERS(keys::kExcludeRequestHeadersKey);
+  EXPECT_FALSE(result);
+
+  // Second set of test data -- failing disjunction.
+  const std::string kFailCondition[] = {
+    keys::kNameSuffixKey, "Custom",      // Test 1.
+    keys::kNameEqualsKey, "ustom-valu",  // Test 2.
+    keys::kValuePrefixKey, "custom ",    // Test 3.
+    keys::kValueContainsKey, " value"    // Test 4.
+  };
+  const size_t kFailConditionSizes[] = { 2u, 2u, 2u, 2u };
+  GetArrayAsVector(kFailCondition, kFailConditionSizes, 4u, &tests);
+  // Positive filter, failing (disjunction of tests).
+  MATCH_REQUEST_HEADERS(keys::kRequestHeadersKey);
+  EXPECT_FALSE(result);
+  // Negative filter, passing (disjunction of tests).
+  MATCH_REQUEST_HEADERS(keys::kExcludeRequestHeadersKey);
+  EXPECT_TRUE(result);
+
+  // Third set of test data, corner case -- empty disjunction.
+  GetArrayAsVector(NULL, NULL, 0u, &tests);
+  // Positive filter, failing (no test to pass).
+  MATCH_REQUEST_HEADERS(keys::kRequestHeadersKey);
+  EXPECT_FALSE(result);
+  // Negative filter, passing (no test to fail).
+  MATCH_REQUEST_HEADERS(keys::kExcludeRequestHeadersKey);
+  EXPECT_TRUE(result);
+
+  // Fourth set of test data, corner case -- empty conjunction.
+  const size_t kEmptyConjunctionSizes[] = { 0u };
+  GetArrayAsVector(NULL, kEmptyConjunctionSizes, 1u, &tests);
+  // Positive filter, passing (trivial test).
+  MATCH_REQUEST_HEADERS(keys::kRequestHeadersKey);
+  EXPECT_TRUE(result);
+  // Negative filter, failing.

[battre, 2012/10/05 12:50:03]

Can you remind me again, why this is supposed to fail?

[vabr (Chromium), 2012/10/05 15:56:53]

It corresponds to constructing one RequestMatcher object with a single
excludeRequestHeaders criterion. That criterion is an empty HeaderFilter, result
of which will be negated. Because a HeaderFilter applies all its (sub-)criteria
in conjunction, this results in a match, which after negation results in a
failure to match.

I added the fact about conjunction into HeaderFilter documentation.

+  MATCH_REQUEST_HEADERS(keys::kExcludeRequestHeadersKey);
+  EXPECT_FALSE(result);
+
+#undef MATCH_REQUEST_HEADERS
+}
+
 // Here we test WebRequestConditionAttributeResponseHeaders for:
 // 1. Correct implementation of prefix/suffix/contains/equals matching.
 // 2. Performing logical disjunction (||) between multiple specifications.
 // 3. Negating the match in case of 'doesNotContainHeaders'.
-TEST(WebRequestConditionAttributeTest, Headers) {
+TEST(WebRequestConditionAttributeTest, ResponseHeaders) {
   // Necessary for TestURLRequest.
   MessageLoop message_loop(MessageLoop::TYPE_IO);
 
   net::TestServer test_server(
       net::TestServer::TYPE_HTTP,
       net::TestServer::kLocalhost,
       FilePath(FILE_PATH_LITERAL(
           "chrome/test/data/extensions/api_test/webrequest/declarative")));
   ASSERT_TRUE(test_server.Start());
 
   TestURLRequestContext context;
   TestDelegate delegate;
   TestURLRequest url_request(test_server.GetURL("files/headers.html"),
                              &delegate, &context);
   url_request.Start();
   MessageLoop::current()->Run();
 
   // In all the tests below we assume that the server includes the headers
   // Custom-Header: custom/value
   // Custom-Header-B: valueA
   // Custom-Header-B: valueB
   // Custom-Header-C: valueC, valueD
   // Custom-Header-D:
   // in the response, but does not include "Non-existing: void".
 
   std::vector< std::vector<const std::string*> > tests;
   bool result;
 
+#define MATCH_RESPONSE_HEADERS(key) \
+    MatchAndCheck(tests, key, ON_HEADERS_RECEIVED, &url_request, &result);
+
   // 1.a. -- All these tests should pass.
   const std::string kPassingCondition[] = {
     keys::kNamePrefixKey, "Custom",
     keys::kNameSuffixKey, "m-header",  // Header names are case insensitive.
     keys::kValueContainsKey, "alu",
     keys::kValueEqualsKey, "custom/value"
   };
   const size_t kPassingConditionSizes[] = { arraysize(kPassingCondition) };
   GetArrayAsVector(kPassingCondition, kPassingConditionSizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 1.b. -- None of the following tests in the discjunction should pass.
   const std::string kFailCondition[] = {
     keys::kNamePrefixKey, " Custom",  // Test 1.
     keys::kNameContainsKey, " -",     // Test 2.
     keys::kValueSuffixKey, "alu",     // Test 3.
     keys::kValueEqualsKey, "custom"   // Test 4.
   };
   const size_t kFailConditionSizes[] = { 2u, 2u, 2u, 2u };
   GetArrayAsVector(kFailCondition, kFailConditionSizes, 4u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_FALSE(result);
 
   // 1.c. -- This should fail (mixing name and value from different headers)
   const std::string kMixingCondition[] = {
     keys::kNameSuffixKey, "Header-B",
     keys::kValueEqualsKey, "custom/value"
   };
   const size_t kMixingConditionSizes[] = { arraysize(kMixingCondition) };
   GetArrayAsVector(kMixingCondition, kMixingConditionSizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_FALSE(result);
 
   // 1.d. -- Test handling multiple values for one header (both should pass).
   const std::string kMoreValues1[] = {
     keys::kNameEqualsKey, "Custom-header-b",
     keys::kValueEqualsKey, "valueA"
   };
   const size_t kMoreValues1Sizes[] = { arraysize(kMoreValues1) };
   GetArrayAsVector(kMoreValues1, kMoreValues1Sizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
   const std::string kMoreValues2[] = {
     keys::kNameEqualsKey, "Custom-header-b",
     keys::kValueEqualsKey, "valueB"
   };
   const size_t kMoreValues2Sizes[] = { arraysize(kMoreValues2) };
   GetArrayAsVector(kMoreValues2, kMoreValues2Sizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 1.e. -- This should fail as conjunction but pass as disjunction.
   const std::string kConflict[] = {
     keys::kNameSuffixKey, "Header",      // True for some header.
     keys::kNameContainsKey, "Header-B"   // True for a different header.
   };
   // First disjunction, no conflict.
   const size_t kNoConflictSizes[] = { 2u, 2u };
   GetArrayAsVector(kConflict, kNoConflictSizes, 2u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
   // Then conjunction, conflict.
   const size_t kConflictSizes[] = { arraysize(kConflict) };
   GetArrayAsVector(kConflict, kConflictSizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_FALSE(result);
 
   // 1.f. -- This should pass, checking for correct treatment of ',' in values.
   const std::string kComma[] = {
     keys::kNameSuffixKey, "Header-C",
     keys::kValueEqualsKey, "valueC, valueD"
   };
   const size_t kCommaSizes[] = { arraysize(kComma) };
   GetArrayAsVector(kComma, kCommaSizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 1.g. -- This should pass, empty values are values as well.
   const std::string kEmpty[] = {
     keys::kNameEqualsKey, "custom-header-d",
     keys::kValueEqualsKey, ""
   };
   const size_t kEmptySizes[] = { arraysize(kEmpty) };
   GetArrayAsVector(kEmpty, kEmptySizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 1.h. -- Values are case-sensitive, this should fail.
   const std::string kLowercase[] = {
     keys::kNameEqualsKey, "Custom-header-b",
     keys::kValuePrefixKey, "valueb",  // valueb != valueB
     keys::kNameEqualsKey, "Custom-header-b",
     keys::kValueSuffixKey, "valueb",
     keys::kNameEqualsKey, "Custom-header-b",
     keys::kValueContainsKey, "valueb",
     keys::kNameEqualsKey, "Custom-header-b",
     keys::kValueEqualsKey, "valueb"
   };
   const size_t kLowercaseSizes[] = { 4u, 4u, 4u, 4u };  // As disjunction.
   GetArrayAsVector(kLowercase, kLowercaseSizes, 4u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_FALSE(result);
 
   // 1.i. -- Names are case-insensitive, this should pass.
   const std::string kUppercase[] = {
     keys::kNamePrefixKey, "CUSTOM-HEADER-B",
     keys::kNameSuffixKey, "CUSTOM-HEADER-B",
     keys::kNameEqualsKey, "CUSTOM-HEADER-B",
     keys::kNameContainsKey, "CUSTOM-HEADER-B"
   };
   const size_t kUppercaseSizes[] = { arraysize(kUppercase) };  // Conjunction.
   GetArrayAsVector(kUppercase, kUppercaseSizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 2.a. -- This should pass as disjunction, because one of the tests passes.
   const std::string kDisjunction[] = {
     keys::kNamePrefixKey, "Non-existing",  // This one fails.
     keys::kNameSuffixKey, "Non-existing",  // This one fails.
     keys::kValueEqualsKey, "void",         // This one fails.
     keys::kValueContainsKey, "alu"         // This passes.
   };
   const size_t kDisjunctionSizes[] = { 2u, 2u, 2u, 2u };
   GetArrayAsVector(kDisjunction, kDisjunctionSizes, 4u, &tests);
-  MatchAndCheck(tests, keys::kResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 3.a. -- This should pass.
   const std::string kNonExistent[] = {
     keys::kNameEqualsKey, "Non-existing",
     keys::kValueEqualsKey, "void"
   };
   const size_t kNonExistentSizes[] = { arraysize(kNonExistent) };
   GetArrayAsVector(kNonExistent, kNonExistentSizes, 1u, &tests);
-  MatchAndCheck(tests, keys::kExcludeResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kExcludeResponseHeadersKey);
   EXPECT_TRUE(result);
 
   // 3.b. -- This should fail.
   const std::string kExisting[] = {
     keys::kNameEqualsKey, "custom-header-b",
     keys::kValueEqualsKey, "valueB"
   };
   const size_t kExistingSize[] = { arraysize(kExisting) };
   GetArrayAsVector(kExisting, kExistingSize, 1u, &tests);
-  MatchAndCheck(tests, keys::kExcludeResponseHeadersKey, &url_request, &result);
+  MATCH_RESPONSE_HEADERS(keys::kExcludeResponseHeadersKey);
   EXPECT_FALSE(result);
+
+#undef MATCH_RESPONSE_HEADERS
 }
 
 }  // namespace extensions