Domain Reliability: Don't send proxy address, other fixes

components/domain_reliability/monitor_unittest.cc

Index: components/domain_reliability/monitor_unittest.cc
diff --git a/components/domain_reliability/monitor_unittest.cc b/components/domain_reliability/monitor_unittest.cc
index 1366f8df6da77a8094ae8b7acd7866f4bc38dbdf..8f4309af60e887d685b4dc0325b8c77f419dfac0 100644
--- a/components/domain_reliability/monitor_unittest.cc
+++ b/components/domain_reliability/monitor_unittest.cc
@@ -17,6 +17,8 @@
 #include "components/domain_reliability/test_util.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/load_flags.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_util.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
@@ -24,8 +26,56 @@
 
 namespace domain_reliability {
 
+namespace {
+
 typedef std::vector<DomainReliabilityBeacon> BeaconVector;
 
+scoped_refptr<net::HttpResponseHeaders> MakeHttpResponseHeaders(
+    const std::string& headers) {
+  return scoped_refptr<net::HttpResponseHeaders>(
+      new net::HttpResponseHeaders(net::HttpUtil::AssembleRawHeaders(
+          headers.c_str(), headers.length())));
+}
+
+static scoped_ptr<const DomainReliabilityConfig> MakeConfig() {
+  DomainReliabilityConfig* config = new DomainReliabilityConfig();
+
+  DomainReliabilityConfig::Resource* resource;
+
+  resource = new DomainReliabilityConfig::Resource();
+  resource->name = "always_report";
+  resource->url_patterns.push_back(
+      new std::string("http://example/always_report"));
+  resource->success_sample_rate = 1.0;
+  resource->failure_sample_rate = 1.0;
+  EXPECT_TRUE(resource->IsValid());
+  config->resources.push_back(resource);
+
+  resource = new DomainReliabilityConfig::Resource();
+  resource->name = "never_report";
+  resource->url_patterns.push_back(
+      new std::string("http://example/never_report"));
+  resource->success_sample_rate = 0.0;
+  resource->failure_sample_rate = 0.0;
+  EXPECT_TRUE(resource->IsValid());
+  config->resources.push_back(resource);
+
+  DomainReliabilityConfig::Collector* collector;
+  collector = new DomainReliabilityConfig::Collector();
+  collector->upload_url = GURL("https://example/upload");
+  EXPECT_TRUE(collector->IsValid());
+  config->collectors.push_back(collector);
+
+  config->version = "1";
+  config->valid_until = 1234567890.0;
+  config->domain = "example";
+  EXPECT_TRUE(config->IsValid());
+
+  return scoped_ptr<const DomainReliabilityConfig>(config);
+}
+
+}  // namespace
+
 class DomainReliabilityMonitorTest : public testing::Test {
  protected:
   typedef DomainReliabilityMonitor::RequestInfo RequestInfo;
@@ -38,50 +88,17 @@ class DomainReliabilityMonitorTest : public testing::Test {
         monitor_(url_request_context_getter_->GetURLRequestContext(),
                  "test-reporter",
                  scoped_ptr<MockableTime>(time_)),
-        context_(monitor_.AddContextForTesting(CreateConfig())) {}
-
-  static scoped_ptr<const DomainReliabilityConfig> CreateConfig() {
-    DomainReliabilityConfig* config = new DomainReliabilityConfig();
-
-    DomainReliabilityConfig::Resource* resource;
-
-    resource = new DomainReliabilityConfig::Resource();
-    resource->name = "always_report";
-    resource->url_patterns.push_back(
-        new std::string("http://example/always_report"));
-    resource->success_sample_rate = 1.0;
-    resource->failure_sample_rate = 1.0;
-    EXPECT_TRUE(resource->IsValid());
-    config->resources.push_back(resource);
-
-    resource = new DomainReliabilityConfig::Resource();
-    resource->name = "never_report";
-    resource->url_patterns.push_back(
-        new std::string("http://example/never_report"));
-    resource->success_sample_rate = 0.0;
-    resource->failure_sample_rate = 0.0;
-    EXPECT_TRUE(resource->IsValid());
-    config->resources.push_back(resource);
-
-    DomainReliabilityConfig::Collector* collector;
-    collector = new DomainReliabilityConfig::Collector();
-    collector->upload_url = GURL("https://example/upload");
-    EXPECT_TRUE(collector->IsValid());
-    config->collectors.push_back(collector);
-
-    config->version = "1";
-    config->valid_until = 1234567890.0;
-    config->domain = "example";
-    EXPECT_TRUE(config->IsValid());
-
-    return scoped_ptr<const DomainReliabilityConfig>(config);
-  }
+        context_(monitor_.AddContextForTesting(MakeConfig())) {}
 
-  RequestInfo MakeRequestInfo() {
+  static RequestInfo MakeRequestInfo() {
     RequestInfo request;
     request.status = net::URLRequestStatus();
-    request.response_code = 200;
-    request.was_cached = false;
+    request.status.set_status(net::URLRequestStatus::SUCCESS);
+    request.status.set_error(net::OK);
+    request.response_info.headers = MakeHttpResponseHeaders(
+        "HTTP/1.1 200 OK\n\n");
+    request.response_info.network_accessed = true;
+    request.response_info.was_fetched_via_proxy = false;
     request.load_flags = 0;
     request.is_upload = false;
     return request;
@@ -120,6 +137,8 @@ TEST_F(DomainReliabilityMonitorTest, NoContextRequest) {
   EXPECT_TRUE(CheckNoBeacons(1));
 }
 
+namespace {
+
 TEST_F(DomainReliabilityMonitorTest, ContextRequest) {
   RequestInfo request = MakeRequestInfo();
   request.url = GURL("http://example/always_report");
@@ -141,6 +160,20 @@ TEST_F(DomainReliabilityMonitorTest, ContextRequestWithDoNotSendCookies) {
   EXPECT_TRUE(CheckNoBeacons(1));
 }
 
+// Make sure the monitor does not log the proxy's IP if one was used.
+TEST_F(DomainReliabilityMonitorTest, ContextRequestFetchedViaProxy) {
+  RequestInfo request = MakeRequestInfo();
+  request.url = GURL("http://example/always_report");
+  request.response_info.was_fetched_via_proxy = true;
+  OnRequestLegComplete(request);
+
+  BeaconVector beacons;
+  context_->GetQueuedDataForTesting(0, &beacons, NULL, NULL);
+  EXPECT_EQ(1u, beacons.size());
+  EXPECT_EQ("", beacons[0].server_ip);
+  EXPECT_TRUE(CheckNoBeacons(1));
+}
+

[Ryan Sleevi, 2014/04/30 23:20:14]

More things to test: 
- Cache behaviour (no network access)
- Redirect chains
  - ... that end up in redirect loops
  - ... that go from an always_report to a never_report [perhaps with an
ambivalent intermediary URL between them]
  - ... that go from a never_report to an always_report [... ditto]
  - ... that cross origins
- non-whitelisted error codes
- whitelisted error codes

Just a few thoughts on things that might have surprising (security) behaviours

 TEST_F(DomainReliabilityMonitorTest, ContextRequestThatIsUpload) {
   RequestInfo request = MakeRequestInfo();
   request.url = GURL("http://example/always_report");
@@ -167,4 +200,6 @@ TEST_F(DomainReliabilityMonitorTest, AddBakedInConfigs) {
   EXPECT_EQ(num_baked_in_configs + 1, monitor_.contexts_size_for_testing());
 }
 
+}  // namespace
+
 }  // namespace domain_reliability