Add net::CertificateReportSender for handling cert report sending

net/http/certificate_report_sender_impl_unittest.cc

+// Copyright 2015 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 "net/http/certificate_report_sender_impl.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "base/thread_task_runner_handle.h"
+#include "net/base/load_flags.h"
+#include "net/base/network_delegate_impl.h"
+#include "net/base/upload_bytes_element_reader.h"
+#include "net/base/upload_data_stream.h"
+#include "net/base/upload_element_reader.h"
+#include "net/test/url_request/url_request_failed_job.h"
+#include "net/test/url_request/url_request_mock_data_job.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+

[davidben, 2015/07/23 00:09:42]

This is a test in //net, so you can wrap the whole thing in namespace net and
remove the net:: prefixes.

[estark, 2015/07/23 02:41:22]

Done.

+namespace {
+
+const char kDummyReport[] = "foo.test";
+const char kSecondDummyReport[] = "foo2.test";
+
+void EnableUrlRequestMocks(bool enable) {

[davidben, 2015/07/23 00:09:42]

Perhaps SetURLRequestMocksEnabled(bool enable) since this can also disable.

[estark, 2015/07/23 02:41:21]

Done.

+  net::URLRequestFilter::GetInstance()->ClearHandlers();
+  if (!enable)
+    return;
+
+  net::URLRequestFailedJob::AddUrlHandler();
+  net::URLRequestMockDataJob::AddUrlHandler();
+}
+
+// Check that data uploaded in the request matches the test report
+// data. The sent reports will be erased from |expect_reports|.
+void CheckUploadData(net::URLRequest* request,
+                     std::set<std::string>* expect_reports) {
+  const net::UploadDataStream* upload = request->get_upload();
+  ASSERT_TRUE(upload);
+  ASSERT_TRUE(upload->GetElementReaders());
+  EXPECT_EQ(1u, upload->GetElementReaders()->size());

[davidben, 2015/07/23 00:09:42]

This probably should be ASSERT_EQ so you don't crash if zero.

[estark, 2015/07/23 02:41:22]

Done.

+
+  const net::UploadBytesElementReader* reader =
+      (*upload->GetElementReaders())[0]->AsBytesReader();
+  ASSERT_TRUE(reader);
+  std::string upload_data(reader->bytes(), reader->length());
+
+  EXPECT_EQ(1u, expect_reports->count(upload_data));
+  expect_reports->erase(upload_data);

[davidben, 2015/07/23 00:09:42]

This can be
  EXPECT_EQ(1, expect_reports->erase(upload_data));

[estark, 2015/07/23 02:41:21]

Done.

+}
+
+// A network delegate that lets tests check that a certificate report
+// was sent. It counts the number of requests and lets tests register a
+// callback to run when the request is destroyed. It also checks that
+// the uploaded data is as expected.
+class TestCertificateReportSenderNetworkDelegate
+    : public net::NetworkDelegateImpl {
+ public:
+  TestCertificateReportSenderNetworkDelegate()
+      : url_request_destroyed_callback_(base::Bind(&base::DoNothing)),
+        all_url_requests_destroyed_callback_(base::Bind(&base::DoNothing)),
+        num_requests_(0),
+        expect_cookies_(false) {}
+
+  ~TestCertificateReportSenderNetworkDelegate() override {}
+
+  void ExpectReport(const std::string& report) {
+    expect_reports_.insert(report);
+  }
+
+  void set_all_url_requests_destroyed_callback(
+      const base::Closure& all_url_requests_destroyed_callback) {
+    all_url_requests_destroyed_callback_ = all_url_requests_destroyed_callback;
+  }
+
+  void set_url_request_destroyed_callback(
+      const base::Closure& url_request_destroyed_callback) {
+    url_request_destroyed_callback_ = url_request_destroyed_callback;
+  }
+
+  void set_expect_url(const GURL& expect_url) { expect_url_ = expect_url; }
+
+  size_t num_requests() const { return num_requests_; }
+
+  // Sets whether cookies are expected to be sent on requests.
+  void set_expect_cookies(bool expect_cookies) {
+    expect_cookies_ = expect_cookies;
+  }
+
+  // NetworkDelegateImpl implementation
+  int OnBeforeURLRequest(net::URLRequest* request,
+                         const net::CompletionCallback& callback,
+                         GURL* new_url) override {
+    num_requests_++;
+    EXPECT_EQ(expect_url_, request->url());
+    EXPECT_STRCASEEQ("POST", request->method().data());
+
+    if (expect_cookies_) {
+      EXPECT_FALSE(request->load_flags() & net::LOAD_DO_NOT_SEND_COOKIES);
+      EXPECT_FALSE(request->load_flags() & net::LOAD_DO_NOT_SAVE_COOKIES);
+    } else {
+      EXPECT_TRUE(request->load_flags() & net::LOAD_DO_NOT_SEND_COOKIES);
+      EXPECT_TRUE(request->load_flags() & net::LOAD_DO_NOT_SAVE_COOKIES);
+    }
+
+    CheckUploadData(request, &expect_reports_);
+    return net::OK;
+  }
+
+  void OnURLRequestDestroyed(net::URLRequest* request) override {
+    url_request_destroyed_callback_.Run();
+    if (expect_reports_.empty())
+      all_url_requests_destroyed_callback_.Run();
+  }
+
+ private:
+  base::Closure url_request_destroyed_callback_;
+  base::Closure all_url_requests_destroyed_callback_;
+  size_t num_requests_;
+  GURL expect_url_;
+  std::set<std::string> expect_reports_;
+  bool expect_cookies_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestCertificateReportSenderNetworkDelegate);
+};
+
+class CertificateReportSenderImplTest : public ::testing::Test {
+ public:
+  CertificateReportSenderImplTest() : context_(true) {
+    EnableUrlRequestMocks(true);
+    context_.set_network_delegate(&network_delegate_);
+    context_.Init();
+  }
+
+  ~CertificateReportSenderImplTest() override { EnableUrlRequestMocks(false); }
+
+  TestCertificateReportSenderNetworkDelegate* network_delegate() {
+    return &network_delegate_;
+  }
+
+  net::TestURLRequestContext* context() { return &context_; }
+
+ private:
+  TestCertificateReportSenderNetworkDelegate network_delegate_;
+  net::TestURLRequestContext context_;
+};
+
+void SendReport(net::CertificateReportSender* reporter,
+                TestCertificateReportSenderNetworkDelegate* network_delegate,
+                const std::string& report,
+                const GURL& url,
+                size_t request_sequence_number) {

[davidben, 2015/07/23 00:09:42]

May as well make this a protected method of SendReport and save yourself the
network_delegate parameter.

[estark, 2015/07/23 02:41:21]

Done.

+  base::RunLoop run_loop;
+  network_delegate->set_url_request_destroyed_callback(run_loop.QuitClosure());
+
+  network_delegate->set_expect_url(url);
+  network_delegate->ExpectReport(report);
+
+  EXPECT_EQ(request_sequence_number, network_delegate->num_requests());
+
+  reporter->Send(url, report);
+
+  // The report is sent asynchronously, so wait for the report's
+  // URLRequest to be destroyed before checking that the report was
+  // sent.
+  run_loop.Run();
+
+  EXPECT_EQ(request_sequence_number + 1, network_delegate->num_requests());
+}
+
+// Test that CertificateReportSender::Send creates a URLRequest for the
+// endpoint and sends the expected data.
+TEST_F(CertificateReportSenderImplTest, SendsRequest) {
+  GURL url = net::URLRequestMockDataJob::GetMockHttpsUrl("dummy data", 1);
+  net::CertificateReportSenderImpl reporter(
+      context(), net::CertificateReportSenderImpl::DO_NOT_SEND_COOKIES);
+  SendReport(&reporter, network_delegate(), kDummyReport, url, 0);
+}
+
+TEST_F(CertificateReportSenderImplTest, SendMultipleReportsSequentially) {
+  GURL url = net::URLRequestMockDataJob::GetMockHttpsUrl("dummy data", 1);
+  net::CertificateReportSenderImpl reporter(
+      context(), net::CertificateReportSenderImpl::DO_NOT_SEND_COOKIES);
+  SendReport(&reporter, network_delegate(), kDummyReport, url, 0);
+  SendReport(&reporter, network_delegate(), kDummyReport, url, 1);
+}
+
+TEST_F(CertificateReportSenderImplTest, SendMultipleReportsSimultaneously) {
+  base::RunLoop run_loop;
+  network_delegate()->set_all_url_requests_destroyed_callback(
+      run_loop.QuitClosure());
+
+  GURL url = net::URLRequestMockDataJob::GetMockHttpsUrl("dummy data", 1);
+  network_delegate()->set_expect_url(url);
+  network_delegate()->ExpectReport(kDummyReport);
+  network_delegate()->ExpectReport(kSecondDummyReport);
+
+  net::CertificateReportSenderImpl reporter(
+      context(), net::CertificateReportSenderImpl::DO_NOT_SEND_COOKIES);
+
+  EXPECT_EQ(0u, network_delegate()->num_requests());
+
+  reporter.Send(url, kDummyReport);
+  reporter.Send(url, kSecondDummyReport);
+
+  run_loop.Run();
+
+  EXPECT_EQ(2u, network_delegate()->num_requests());
+}
+
+// Test that pending URLRequests get cleaned up when the report sender
+// is deleted.
+TEST_F(CertificateReportSenderImplTest, PendingRequestGetsDeleted) {
+  base::RunLoop run_loop;
+  network_delegate()->set_url_request_destroyed_callback(
+      run_loop.QuitClosure());
+
+  GURL url = net::URLRequestFailedJob::GetMockHttpUrlWithFailurePhase(
+      net::URLRequestFailedJob::START, net::ERR_IO_PENDING);
+  network_delegate()->set_expect_url(url);
+  network_delegate()->ExpectReport(kDummyReport);
+
+  EXPECT_EQ(0u, network_delegate()->num_requests());
+
+  scoped_ptr<net::CertificateReportSender> reporter(
+      new net::CertificateReportSenderImpl(
+          context(), net::CertificateReportSenderImpl::DO_NOT_SEND_COOKIES));
+  reporter->Send(url, kDummyReport);
+  reporter.reset();
+
+  run_loop.Run();

[davidben, 2015/07/23 00:09:42]

This happens synchronously, so you could also set the callback to something that
sets a local variable (bind a base::Unretained pointer to it perhaps) to true,
rather than spin an event loop and everything.

[estark, 2015/07/23 02:41:21]

Done.

+
+  EXPECT_EQ(1u, network_delegate()->num_requests());
+}
+
+// Test that a request that returns an error gets cleaned up.
+TEST_F(CertificateReportSenderImplTest, ErroredRequestGetsDeleted) {
+  GURL url = net::URLRequestFailedJob::GetMockHttpsUrl(net::ERR_FAILED);
+  net::CertificateReportSenderImpl reporter(
+      context(), net::CertificateReportSenderImpl::DO_NOT_SEND_COOKIES);

[davidben, 2015/07/23 00:09:42]

Probably want a comment so it's clear:
  // SendReport will block until the URLRequest is destroyed.

[estark, 2015/07/23 02:41:21]

Done.

+  SendReport(&reporter, network_delegate(), kDummyReport, url, 0);
+}
+
+// Test that cookies are sent or not sent according to the error
+// reporter's cookies preference.
+
+TEST_F(CertificateReportSenderImplTest, SendCookiesPreference) {
+  GURL url = net::URLRequestMockDataJob::GetMockHttpsUrl("dummy data", 1);
+  net::CertificateReportSenderImpl reporter(
+      context(), net::CertificateReportSenderImpl::SEND_COOKIES);
+
+  network_delegate()->set_expect_cookies(true);
+  SendReport(&reporter, network_delegate(), kDummyReport, url, 0);
+}
+
+TEST_F(CertificateReportSenderImplTest, DoNotSendCookiesPreference) {
+  GURL url = net::URLRequestMockDataJob::GetMockHttpsUrl("dummy data", 1);
+  net::CertificateReportSenderImpl reporter(
+      context(), net::CertificateReportSenderImpl::DO_NOT_SEND_COOKIES);
+
+  network_delegate()->set_expect_cookies(false);
+  SendReport(&reporter, network_delegate(), kDummyReport, url, 0);
+}
+
+}  // namespace

[davidben, 2015/07/23 00:09:42]

I believe the conclusion the last time this came up was that tests (and their
fixtures) shouldn't be in an anonymous namespace because the names have to be
globally unique.

(Of course, that we also wrap our tests in namespaces kinda nullifies that, but
whatever.)

[estark, 2015/07/23 02:41:21]

Done.