Declarative Web Request: firstPartyForCookiesUrl

chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry_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_rules_registry.h"
 
+#include <string>
 #include <vector>
 
+#include "base/basictypes.h"
 #include "base/json/json_reader.h"
 #include "base/memory/linked_ptr.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/test/values_test_util.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_constants.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api_helpers.h"
 #include "chrome/common/extensions/matcher/url_matcher_constants.h"
 #include "content/public/test/test_browser_thread.h"
@@ skipping 157 matching lines @@
 
     linked_ptr<RulesRegistry::Rule> rule(new RulesRegistry::Rule);
     rule->id.reset(new std::string(kRuleId4));
     rule->priority.reset(new int(200));
     rule->actions.push_back(linked_ptr<base::Value>(action_dict.DeepCopy()));
     rule->conditions.push_back(
         linked_ptr<base::Value>(condition_dict.DeepCopy()));
     return rule;
   }
 
-  // Create a condition with the attributes specified. An example value of a
-  // string from |attributes| is: "\"resourceType\": [\"stylesheet\"], \n".
-  linked_ptr<base::Value> CreateCondition(
-      const std::vector<const char *>& attributes) {
-    // Starting boilerplate.
+  // Create a condition with the attributes specified. An example value of
+  // |attributes| is: "\"resourceType\": [\"stylesheet\"], \n".
+  linked_ptr<base::Value> CreateCondition(const std::string& attributes) {
     std::string json_description =
         "{ \n"
         "  \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n";
-    for (size_t i = 0; i < attributes.size(); ++i) {
-      json_description += attributes[i];
-    }
-    // Ending boilerplate.
+    json_description += attributes;
     json_description += "}";
 
     return linked_ptr<base::Value>(
         base::test::ParseJson(json_description).release());
   }
 
-  // Create a rule with the ID |rule_id| and with a single condition and a
-  // cancelling action. The condition contains a non-matching non-URL attribute,
-  // and optionally, according to |with_url_attribute| also a URL attribute
-  // matching any (!) URL.
-  linked_ptr<RulesRegistry::Rule> CreateNonMatchingRule(
-      bool with_url_attribute,
-      const char* rule_id) {
-    std::vector<const char*> attributes;
-    if (with_url_attribute)
-      attributes.push_back("\"url\": { \"pathContains\": \"\" }, \n");
-
-    // The following attribute never matches in this unit test.
-    attributes.push_back("\"resourceType\": [\"stylesheet\"], \n");
-
+  // Create a rule with the ID |rule_id| and with conditions created from the
+  // |attributes| specified (one entry one condition). An example value of a
+  // string from |attributes| is: "\"resourceType\": [\"stylesheet\"], \n".
+  linked_ptr<RulesRegistry::Rule> CreateCancellingRule(
+      const char* rule_id,
+      const std::vector<const std::string*>& attributes) {
     DictionaryValue action_dict;
     action_dict.SetString(keys::kInstanceTypeKey, keys::kCancelRequestType);
 
-    linked_ptr<RulesRegistry::Rule> rule(new RulesRegistry::Rule);
-    rule->id.reset(new std::string(rule_id));
-    rule->priority.reset(new int(1));
-    rule->actions.push_back(linked_ptr<base::Value>(action_dict.DeepCopy()));
-    rule->conditions.push_back(CreateCondition(attributes));
-    return rule;
-  }
-
-  // Create a rule with the ID |rule_id| and with two conditions and a
-  // cancelling action. One condition contains a non-matching non-URL attribute,
-  // and the other one a URL attribute matching any URL.
-  linked_ptr<RulesRegistry::Rule> CreateRuleWithTwoConditions(
-      const char* rule_id) {
-    std::vector<const char*> url_attributes;
-    url_attributes.push_back("\"url\": { \"pathContains\": \"\" }, \n");
-
-    // The following attribute never matches in this unit test.
-    std::vector<const char*> non_matching_attributes;
-    non_matching_attributes.push_back("\"resourceType\": [\"stylesheet\"], \n");
-
-    DictionaryValue action_dict;
-    action_dict.SetString(keys::kInstanceTypeKey, keys::kCancelRequestType);
-
     linked_ptr<RulesRegistry::Rule> rule(new RulesRegistry::Rule);
     rule->id.reset(new std::string(rule_id));
     rule->priority.reset(new int(1));
     rule->actions.push_back(linked_ptr<base::Value>(action_dict.DeepCopy()));
-    rule->conditions.push_back(CreateCondition(url_attributes));
-    rule->conditions.push_back(CreateCondition(non_matching_attributes));
+    for (std::vector<const std::string*>::const_iterator it =
+             attributes.begin();
+         it != attributes.end(); ++it)
+      rule->conditions.push_back(CreateCondition(**it));
     return rule;
   }
 
  protected:
   MessageLoop message_loop;
   content::TestBrowserThread ui;
   content::TestBrowserThread io;
 };
 
 TEST_F(WebRequestRulesRegistryTest, AddRulesImpl) {
   scoped_refptr<TestWebRequestRulesRegistry> registry(
       new TestWebRequestRulesRegistry());
   std::string error;
 
   std::vector<linked_ptr<RulesRegistry::Rule> > rules;
   rules.push_back(CreateRule1());
   rules.push_back(CreateRule2());
 
   error = registry->AddRules(kExtensionId, rules);
   EXPECT_EQ("", error);
   EXPECT_EQ(1, registry->num_clear_cache_calls());
 
   std::set<const WebRequestRule*> matches;
 
   GURL http_url("http://www.example.com");
   net::TestURLRequestContext context;
   net::TestURLRequest http_request(http_url, NULL, &context);
-  matches = registry->GetMatches(
-      DeclarativeWebRequestData(&http_request, ON_BEFORE_REQUEST));
+  WebRequestData request_data(&http_request, ON_BEFORE_REQUEST);
+  matches = registry->GetMatches(request_data);
   EXPECT_EQ(2u, matches.size());
 
   std::set<WebRequestRule::GlobalRuleId> matches_ids;
   for (std::set<const WebRequestRule*>::const_iterator it = matches.begin();
        it != matches.end(); ++it)
     matches_ids.insert((*it)->id());
   EXPECT_TRUE(ContainsKey(matches_ids, std::make_pair(kExtensionId, kRuleId1)));
   EXPECT_TRUE(ContainsKey(matches_ids, std::make_pair(kExtensionId, kRuleId2)));
 
   GURL foobar_url("http://www.foobar.com");
   net::TestURLRequest foobar_request(foobar_url, NULL, &context);
-  matches = registry->GetMatches(
-      DeclarativeWebRequestData(&foobar_request, ON_BEFORE_REQUEST));
+  request_data.request = &foobar_request;
+  matches = registry->GetMatches(request_data);
   EXPECT_EQ(1u, matches.size());
   WebRequestRule::GlobalRuleId expected_pair =
       std::make_pair(kExtensionId, kRuleId2);
   EXPECT_EQ(expected_pair, (*matches.begin())->id());
 }
 
 TEST_F(WebRequestRulesRegistryTest, RemoveRulesImpl) {
   scoped_refptr<TestWebRequestRulesRegistry> registry(
       new TestWebRequestRulesRegistry());
   std::string error;
@@ skipping 104 matching lines @@
   EXPECT_EQ("", error);
 
   std::vector<linked_ptr<RulesRegistry::Rule> > rules_to_add_2(1);
   rules_to_add_2[0] = CreateRedirectRule("http://www.bar.com");
   error = registry->AddRules(kExtensionId2, rules_to_add_2);
   EXPECT_EQ("", error);
 
   GURL url("http://www.google.com");
   net::TestURLRequestContext context;
   net::TestURLRequest request(url, NULL, &context);
+  WebRequestData request_data(&request, ON_BEFORE_REQUEST);
   std::list<LinkedPtrEventResponseDelta> deltas =
-      registry->CreateDeltas(
-          NULL,
-          DeclarativeWebRequestData(&request, ON_BEFORE_REQUEST),
-          false);
+      registry->CreateDeltas(NULL, request_data, false);
 
   // The second extension is installed later and will win for this reason
   // in conflict resolution.
   ASSERT_EQ(2u, deltas.size());
   deltas.sort(&helpers::InDecreasingExtensionInstallationTimeOrder);
 
   std::list<LinkedPtrEventResponseDelta>::iterator i = deltas.begin();
   LinkedPtrEventResponseDelta winner = *i++;
   LinkedPtrEventResponseDelta loser = *i;
 
@@ skipping 25 matching lines @@
   EXPECT_EQ("", error);
 
   std::vector<linked_ptr<RulesRegistry::Rule> > rules_to_add_3(1);
   rules_to_add_3[0] = CreateIgnoreRule();
   error = registry->AddRules(kExtensionId, rules_to_add_3);
   EXPECT_EQ("", error);
 
   GURL url("http://www.google.com/index.html");
   net::TestURLRequestContext context;
   net::TestURLRequest request(url, NULL, &context);
+  WebRequestData request_data(&request, ON_BEFORE_REQUEST);
   std::list<LinkedPtrEventResponseDelta> deltas =
-      registry->CreateDeltas(
-          NULL,
-          DeclarativeWebRequestData(&request, ON_BEFORE_REQUEST),
-          false);
+      registry->CreateDeltas(NULL, request_data, false);
 
   // The redirect by the first extension is ignored due to the ignore rule.
   ASSERT_EQ(1u, deltas.size());
   LinkedPtrEventResponseDelta effective_rule = *(deltas.begin());
 
   EXPECT_EQ(kExtensionId2, effective_rule->extension_id);
   EXPECT_EQ(base::Time() + base::TimeDelta::FromDays(2),
             effective_rule->extension_install_time);
   EXPECT_EQ(GURL("http://www.bar.com"), effective_rule->new_url);
 }
 
 // Test that rules failing IsFulfilled on their conditions are never returned by
 // GetMatches.
 TEST_F(WebRequestRulesRegistryTest, GetMatchesCheckFulfilled) {
   scoped_refptr<TestWebRequestRulesRegistry> registry(
       new TestWebRequestRulesRegistry());
+  const std::string kMatchingUrlAttribute(
+      "\"url\": { \"pathContains\": \"\" }, \n");
+  const std::string kNonMatchingNonUrlAttribute(
+      "\"resourceType\": [\"stylesheet\"], \n");
+  const std::string kBothAttributes(kMatchingUrlAttribute +
+                                    kNonMatchingNonUrlAttribute);
   std::string error;
+  std::vector<const std::string*> attributes;
+  std::vector<linked_ptr<RulesRegistry::Rule> > rules;
 
-  std::vector<linked_ptr<RulesRegistry::Rule> > rules;
-  // Both rules have one condition, neither of them should fire.
-  // This rule's condition has only one, non-matching and non-URL attribute.
-  rules.push_back(CreateNonMatchingRule(false, kRuleId1));
-  // This rule's condition has two attributes: a matching URL, and a
-  // non-matching non-URL attribute.
-  rules.push_back(CreateNonMatchingRule(true, kRuleId2));
+  // Rules 1 and 2 have one condition, neither of them should fire.
+  attributes.push_back(&kNonMatchingNonUrlAttribute);
+  rules.push_back(CreateCancellingRule(kRuleId1, attributes));
 
-  // The 3rd rule has two conditions, one with a matching URL attribute, and one
+  attributes.clear();
+  attributes.push_back(&kBothAttributes);
+  rules.push_back(CreateCancellingRule(kRuleId2, attributes));
+
+  // Rule 3 has two conditions, one with a matching URL attribute, and one
   // with a non-matching non-URL attribute.
-  rules.push_back(CreateRuleWithTwoConditions(kRuleId3));
+  attributes.clear();
+  attributes.push_back(&kMatchingUrlAttribute);
+  attributes.push_back(&kNonMatchingNonUrlAttribute);
+  rules.push_back(CreateCancellingRule(kRuleId3, attributes));
 
   error = registry->AddRules(kExtensionId, rules);
   EXPECT_EQ("", error);
   EXPECT_EQ(1, registry->num_clear_cache_calls());
 
   std::set<const WebRequestRule*> matches;
 
   GURL http_url("http://www.example.com");
   net::TestURLRequestContext context;
   net::TestURLRequest http_request(http_url, NULL, &context);
-  matches = registry->GetMatches(
-      DeclarativeWebRequestData(&http_request, ON_BEFORE_REQUEST));
+  WebRequestData request_data(&http_request, ON_BEFORE_REQUEST);
+  matches = registry->GetMatches(request_data);
   EXPECT_EQ(1u, matches.size());
   WebRequestRule::GlobalRuleId expected_pair = std::make_pair(kExtensionId,
                                                               kRuleId3);
   EXPECT_EQ(expected_pair, (*matches.begin())->id());
 }
 
+// Test that the url and firstPartyForCookiesUrl attributes are evaluated
+// against corresponding URLs. Tested on requests where these URLs actually
+// differ.
+TEST_F(WebRequestRulesRegistryTest, GetMatchesDifferentUrls) {
+  scoped_refptr<TestWebRequestRulesRegistry> registry(
+      new TestWebRequestRulesRegistry());
+  const std::string kUrlAttribute(
+      "\"url\": { \"hostContains\": \"url\" }, \n");
+  const std::string kFirstPartyUrlAttribute(
+      "\"firstPartyForCookiesUrl\": { \"hostContains\": \"fpfc\" }, \n");
+  std::string error;
+  std::vector<const std::string*> attributes;
+  std::vector<linked_ptr<RulesRegistry::Rule> > rules;
+
+  // Rule 1 has one condition, with a url attribute
+  attributes.push_back(&kUrlAttribute);
+  rules.push_back(CreateCancellingRule(kRuleId1, attributes));
+
+  // Rule 2 has one condition, with a firstPartyForCookiesUrl attribute
+  attributes.clear();
+  attributes.push_back(&kFirstPartyUrlAttribute);
+  rules.push_back(CreateCancellingRule(kRuleId2, attributes));
+
+  error = registry->AddRules(kExtensionId, rules);
+  EXPECT_EQ("", error);
+  EXPECT_EQ(1, registry->num_clear_cache_calls());
+
+  std::set<const WebRequestRule*> matches;
+
+  const GURL urls[] = {
+    GURL("http://url.example.com"),  // matching
+    GURL("http://www.example.com")   // non-matching
+  };
+  const GURL firstPartyUrls[] = {
+    GURL("http://www.example.com"),  // non-matching
+    GURL("http://fpfc.example.com")  // matching
+  };
+  const char* matchingRuleIds[] = { kRuleId1, kRuleId2 };

[Jeffrey Yasskin, 2013/01/24 19:54:46]

It makes me nervous when a test changes both the input and the code that runs at
the same time. When you're looking for code (rules in this case) to be sensitive
to inputs, you need to run the same code over different inputs or the same input
through different code. Running different input through different code lets bugs
creep in.

[vabr (Chromium), 2013/01/24 21:12:19]

Clarified with Jeffrey, that both rules are used in each iteration of the for
loop. So it is only the inputs which change.

+  COMPILE_ASSERT(arraysize(urls) == arraysize(firstPartyUrls),
+                 urls_and_firstPartyUrls_need_to_have_the_same_size);
+  COMPILE_ASSERT(arraysize(urls) == arraysize(matchingRuleIds),
+                 urls_and_matchingRuleIds_need_to_have_the_same_size);
+  net::TestURLRequestContext context;
+
+  for (size_t i = 0; i < arraysize(matchingRuleIds); ++i) {
+    net::TestURLRequest http_request(urls[i], NULL, &context);
+    WebRequestData request_data(&http_request, ON_BEFORE_REQUEST);
+    http_request.set_first_party_for_cookies(firstPartyUrls[i]);
+    matches = registry->GetMatches(request_data);
+    EXPECT_EQ(1u, matches.size());

[Jeffrey Yasskin, 2013/01/24 19:54:46]

When you put an EXPECT in a loop, always be sure to include enough context to
figure out which iteration it failed in. SCOPED_TRACE
(https://code.google.com/p/googletest/wiki/V1_6_AdvancedGuide#Adding_Traces_to...)
can be enough for this.

[vabr (Chromium), 2013/01/24 21:12:19]

Thanks! Done.

+    WebRequestRule::GlobalRuleId expected_pair =
+      std::make_pair(kExtensionId, matchingRuleIds[i]);
+    EXPECT_EQ(expected_pair, (*matches.begin())->id());

[Jeffrey Yasskin, 2013/01/24 19:54:46]

I tend to prefer inlining the construction of the expected value. gtest prints
the source code it saw, and "expected_pair" is less informative than
"std::make_pair(kExtensionId, matchingRuleIds[i]);", which in turn would be less
informative than "std::make_pair(kExtensionId, kRuleId1);"

[vabr (Chromium), 2013/01/24 21:12:19]

Done. Which rule ID it is should be now clear from the SCOPED_TRACE.

+  }
+}
+
 TEST_F(WebRequestRulesRegistryTest, CheckConsistency) {
   // The contentType condition can only be evaluated during ON_HEADERS_RECEIVED
   // but the redirect action can only be executed during ON_BEFORE_REQUEST.
   // Therefore, this is an inconsistent rule that needs to be flagged.
   const char kRule[] =
       "{                                                                 \n"
       "  \"id\": \"rule1\",                                              \n"
       "  \"conditions\": [                                               \n"
       "    {                                                             \n"
       "      \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n"
@@ skipping 21 matching lines @@
       new TestWebRequestRulesRegistry());
 
   URLMatcher matcher;
   std::string error = registry->AddRulesImpl(kExtensionId, rules);
   EXPECT_THAT(error, HasSubstr("no time in the request life-cycle"));
   EXPECT_TRUE(registry->IsEmpty());
 }
 
 
 }  // namespace extensions