OLD | NEW |
---|---|
(Empty) | |
1 // Copyright 2015 The Chromium Authors. All rights reserved. | |
2 // Use of this source code is governed by a BSD-style license that can be | |
3 // found in the LICENSE file. | |
4 | |
5 #include "chrome/browser/net/certificate_error_reporter.h" | |
6 | |
7 #include "base/bind.h" | |
8 #include "base/bind_helpers.h" | |
9 #include "base/files/file_path.h" | |
10 #include "base/files/file_util.h" | |
11 #include "base/path_service.h" | |
12 #include "base/run_loop.h" | |
13 #include "base/thread_task_runner_handle.h" | |
14 #include "chrome/browser/net/cert_logger.pb.h" | |
15 #include "chrome/browser/net/url_request_mock_util.h" | |
16 #include "chrome/common/chrome_paths.h" | |
17 #include "content/public/browser/browser_thread.h" | |
18 #include "content/public/test/test_browser_thread_bundle.h" | |
19 #include "net/base/network_delegate_impl.h" | |
20 #include "net/base/test_data_directory.h" | |
21 #include "net/test/cert_test_util.h" | |
22 #include "net/test/url_request/url_request_failed_job.h" | |
23 #include "net/test/url_request/url_request_mock_http_job.h" | |
24 #include "net/url_request/url_request_filter.h" | |
25 #include "net/url_request/url_request_test_util.h" | |
26 #include "testing/gtest/include/gtest/gtest.h" | |
27 | |
28 using chrome_browser_net::CertificateErrorReporter; | |
29 using content::BrowserThread; | |
30 using net::CompletionCallback; | |
31 using net::SSLInfo; | |
32 using net::NetworkDelegateImpl; | |
33 using net::TestURLRequestContext; | |
34 using net::URLRequest; | |
35 | |
36 namespace { | |
37 | |
38 const char kHostname[] = "test.mail.google.com"; | |
39 const char kDummyFailureLog[] = "dummy failure log"; | |
40 const char kTestCertFilename[] = FILE_PATH_LITERAL("test_mail_google_com.pem"); | |
41 | |
42 SSLInfo GetSSLInfo() { | |
43 SSLInfo info; | |
44 info.cert = | |
45 net::ImportCertFromFile(net::GetTestCertsDirectory(), kTestCertFilename); | |
46 info.is_issued_by_known_root = true; | |
47 info.pinning_failure_log = kDummyFailureLog; | |
48 return info; | |
49 } | |
50 | |
51 std::string GetPEMEncodedChain() { | |
52 base::FilePath cert_path = | |
53 net::GetTestCertsDirectory().AppendASCII(kTestCertFilename); | |
54 std::string cert_data; | |
55 EXPECT_TRUE(base::ReadFileToString(cert_path, &cert_data)); | |
56 return cert_data; | |
57 } | |
58 | |
59 void EnableUrlRequestMocks(bool enable) { | |
60 net::URLRequestFilter::GetInstance()->ClearHandlers(); | |
61 if (enable) { | |
62 base::FilePath root_http; | |
63 net::URLRequestFailedJob::AddUrlHandler(); | |
64 PathService::Get(chrome::DIR_TEST_DATA, &root_http); | |
65 net::URLRequestMockHTTPJob::AddUrlHandlers( | |
66 root_http, BrowserThread::GetBlockingPool()); | |
67 } | |
68 } | |
69 | |
70 // A network delegate that lets tests check that requests were created | |
71 // or destroyed. | |
72 class TestNetworkDelegate : public NetworkDelegateImpl { | |
73 public: | |
74 TestNetworkDelegate() : NetworkDelegateImpl() { | |
mmenke
2015/03/10 15:08:46
0-argument parent class constructor does not need
estark
2015/03/11 05:58:33
Done.
| |
75 requests_ = 0; | |
76 destroyed_requests_ = 0; | |
mmenke
2015/03/10 15:08:46
These should be in the initializer list.
estark
2015/03/11 05:58:34
Done.
| |
77 } | |
78 ~TestNetworkDelegate() {} | |
mmenke
2015/03/10 15:08:45
override
estark
2015/03/11 05:58:33
Done.
| |
79 | |
80 int OnBeforeURLRequest(URLRequest* request, | |
81 const CompletionCallback& callback, | |
82 GURL* new_url) override { | |
83 requests_++; | |
84 return net::OK; | |
85 } | |
86 | |
87 void OnURLRequestDestroyed(URLRequest* request) { destroyed_requests_++; } | |
mmenke
2015/03/10 15:08:46
override
estark
2015/03/11 05:58:34
Done.
| |
88 | |
89 int GetNumRequests() { return requests_; } | |
90 int GetNumDestroyedRequests() { return destroyed_requests_; } | |
mmenke
2015/03/10 15:08:46
these should both be const, and have the same name
estark
2015/03/11 05:58:33
Done.
| |
91 | |
92 private: | |
93 int requests_; | |
94 int destroyed_requests_; | |
mmenke
2015/03/10 15:08:46
These should match the number of the getters. Add
estark
2015/03/11 05:58:34
Done.
| |
95 }; | |
96 | |
97 class CertificateErrorReporterTest : public ::testing::Test { | |
98 public: | |
99 ~CertificateErrorReporterTest() { | |
mmenke
2015/03/10 15:08:45
override
estark
2015/03/11 05:58:33
Done.
| |
100 EnableUrlRequestMocks(false); | |
101 chrome_browser_net::SetUrlRequestMocksEnabled(false); | |
102 } | |
103 | |
104 protected: | |
105 CertificateErrorReporterTest() : context_(true) { | |
mmenke
2015/03/10 15:08:46
Optional: Suggest just making this public, before
estark
2015/03/11 05:58:33
Done.
| |
106 EnableUrlRequestMocks(true); | |
107 context_.set_network_delegate(&network_delegate_); | |
108 context_.Init(); | |
109 } | |
110 | |
111 content::TestBrowserThreadBundle thread_bundle_; | |
mmenke
2015/03/10 15:08:46
The Google style guide requires all class variable
Ryan Sleevi
2015/03/10 17:39:30
Matt, test fixtures are explicitly allowed to be P
mmenke
2015/03/10 17:52:32
Hrm...Did the old version say that? I admittedly
estark
2015/03/11 05:58:34
Done.
| |
112 TestURLRequestContext context_; | |
113 TestNetworkDelegate network_delegate_; | |
114 }; | |
115 | |
116 class TestReporter : public CertificateErrorReporter { | |
mmenke
2015/03/10 15:08:45
Need a virtual destructor in child classes.
estark
2015/03/11 05:58:34
This class no longer exists. I moved the callback
| |
117 public: | |
118 TestReporter(net::URLRequestContext* context, const GURL& upload_url) | |
119 : CertificateErrorReporter(context, upload_url), latest_request_url_() { | |
mmenke
2015/03/10 15:08:46
Remove latest_request_url_() - the no argument con
estark
2015/03/11 05:58:33
no longer applicable here, but noted for future
| |
120 response_started_callback_ = base::Bind(&base::DoNothing); | |
mmenke
2015/03/10 15:08:46
This should be in an initializer list.
estark
2015/03/11 05:58:34
Done (in the network delegate subclass now).
| |
121 } | |
122 | |
123 TestReporter(net::URLRequestContext* context, | |
124 const GURL& upload_url, | |
125 base::Closure response_started_callback) | |
126 : CertificateErrorReporter(context, upload_url), | |
127 latest_request_url_(), | |
mmenke
2015/03/10 15:08:45
Remove latest_request_url_()
estark
2015/03/11 05:58:33
(no longer applicable)
| |
128 response_started_callback_(response_started_callback) {} | |
129 | |
130 void OnResponseStarted(net::URLRequest* request) override { | |
131 latest_request_url_ = request->url(); | |
132 CertificateErrorReporter::OnResponseStarted(request); | |
133 base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, | |
134 response_started_callback_); | |
mmenke
2015/03/10 15:08:46
Can invoke this synchronously with just response_s
Ryan Sleevi
2015/03/10 17:39:30
I'm not sure I'd agree on the is_empty. By not che
mmenke
2015/03/10 17:52:32
You're right - I hadn't realized that the other co
estark
2015/03/11 05:58:34
I think this discussion is no longer applicable, s
| |
135 } | |
136 | |
137 GURL& GetLatestRequestURL() { return latest_request_url_; } | |
mmenke
2015/03/10 15:08:46
This should be: const GURL& latest_request_url()
estark
2015/03/11 05:58:33
(no longer applicable)
| |
138 | |
139 private: | |
140 GURL latest_request_url_; | |
141 base::Closure response_started_callback_; | |
142 }; | |
143 | |
144 // Test that CertificateErrorReporter::SendReport creates a URLRequest | |
145 // for the endpoint. | |
146 TEST_F(CertificateErrorReporterTest, SendReportSendsRequest) { | |
147 base::RunLoop run_loop; | |
148 base::Closure callback = run_loop.QuitClosure(); | |
mmenke
2015/03/10 15:08:45
optional nit: No need to store this on the stack.
estark
2015/03/11 05:58:34
Done.
| |
149 GURL url = net::URLRequestMockHTTPJob::GetMockHttpsUrl( | |
150 base::FilePath(FILE_PATH_LITERAL("empty.html"))); | |
151 TestReporter reporter(&context_, url, callback); | |
152 | |
153 EXPECT_EQ(0, network_delegate_.GetNumRequests()); | |
154 | |
155 reporter.SendReport(CertificateErrorReporter::REPORT_TYPE_PINNING_VIOLATION, | |
156 kHostname, GetSSLInfo()); | |
157 run_loop.Run(); | |
158 | |
159 EXPECT_EQ(1, network_delegate_.GetNumRequests()); | |
160 EXPECT_EQ(url, reporter.GetLatestRequestURL()); | |
161 } | |
162 | |
163 // Test that pending URLRequests get cleaned up when the reporter is | |
164 // deleted. | |
165 TEST_F(CertificateErrorReporterTest, PendingRequestGetsDeleted) { | |
166 base::RunLoop run_loop; | |
167 GURL url = net::URLRequestMockHTTPJob::GetMockUrlWithFailure( | |
168 base::FilePath(FILE_PATH_LITERAL("empty.html")), | |
169 net::URLRequestMockHTTPJob::START, net::ERR_IO_PENDING); | |
170 | |
171 EXPECT_EQ(0, network_delegate_.GetNumRequests()); | |
172 EXPECT_EQ(0, network_delegate_.GetNumDestroyedRequests()); | |
173 | |
174 TestReporter* reporter = new TestReporter(&context_, url); | |
175 reporter->SendReport(CertificateErrorReporter::REPORT_TYPE_PINNING_VIOLATION, | |
176 kHostname, GetSSLInfo()); | |
177 delete reporter; | |
mmenke
2015/03/10 15:08:45
Rather than calling delete, I suggest putting it i
estark
2015/03/11 05:58:34
Done.
| |
178 | |
179 run_loop.RunUntilIdle(); | |
mmenke
2015/03/10 15:08:46
Is this needed?
estark
2015/03/10 15:54:15
rsleevi advised to add it to avoid coupling with t
mmenke
2015/03/10 16:08:52
Of course, it could *also* post a delayed task to
Ryan Sleevi
2015/03/10 17:39:30
On 2015/03/10 16:08:52, mmenke wrote:
mmenke
2015/03/10 17:52:33
I'm not suggesting adding anything to the main Net
estark
2015/03/11 05:58:34
Done -- the callback is now called from |OnURLRequ
| |
180 | |
181 EXPECT_EQ(1, network_delegate_.GetNumRequests()); | |
182 EXPECT_EQ(1, network_delegate_.GetNumDestroyedRequests()); | |
183 } | |
184 | |
185 // Test that a request that returns an error gets cleaned up. | |
186 TEST_F(CertificateErrorReporterTest, ErroredRequestGetsDeleted) { | |
187 GURL url = net::URLRequestFailedJob::GetMockHttpsUrl(net::ERR_FAILED); | |
188 base::RunLoop run_loop; | |
189 base::Closure callback = run_loop.QuitClosure(); | |
190 | |
191 EXPECT_EQ(0, network_delegate_.GetNumRequests()); | |
192 EXPECT_EQ(0, network_delegate_.GetNumDestroyedRequests()); | |
193 | |
194 TestReporter reporter(&context_, url, callback); | |
195 reporter.SendReport(CertificateErrorReporter::REPORT_TYPE_PINNING_VIOLATION, | |
196 kHostname, GetSSLInfo()); | |
197 run_loop.Run(); | |
198 | |
199 EXPECT_EQ(1, network_delegate_.GetNumRequests()); | |
200 EXPECT_EQ(1, network_delegate_.GetNumDestroyedRequests()); | |
201 } | |
202 | |
mmenke
2015/03/10 15:08:46
Since we're using a std::set for requests, we shou
estark
2015/03/11 05:58:34
If it's okay with you, I'd like to do this in a fo
mmenke
2015/03/11 15:34:28
If this were a CL adding a feature, I'd definitely
felt
2015/03/11 15:51:23
My $0.02:
- This CL is blocking a lot of other wo
| |
203 } // namespace | |
204 | |
205 namespace chrome_browser_net { | |
mmenke
2015/03/10 15:08:46
Having a test in a separate namespace is weird. S
estark
2015/03/10 15:54:15
Sorry, don't follow. This test is in the chrome_br
mmenke
2015/03/10 16:08:52
Sorry, didn't notice that. I'm not a fan of frien
Ryan Sleevi
2015/03/10 17:39:30
On 2015/03/10 16:08:52, mmenke wrote:
estark
2015/03/11 05:58:34
Done -- added a check that the uploaded report is
| |
206 | |
207 // Test that a built report has the correct data. | |
208 // | |
209 // This test is in the chrome_browser_net namespace so that it can be | |
210 // friended by CertificateErrorReporter. | |
211 TEST_F(CertificateErrorReporterTest, ReportIsBuiltCorrectly) { | |
212 SSLInfo info = GetSSLInfo(); | |
213 CertLoggerRequest request; | |
214 CertificateErrorReporter::BuildReport(kHostname, info, &request); | |
215 | |
216 EXPECT_EQ(request.hostname(), kHostname); | |
217 EXPECT_EQ(request.cert_chain(), GetPEMEncodedChain()); | |
218 EXPECT_EQ(request.pin().size(), 1); | |
219 EXPECT_EQ(request.pin().Get(0), kDummyFailureLog); | |
220 } | |
221 } // namespace chrome_browser_net | |
OLD | NEW |