Commit fbecdfb2 authored by Matt Menke's avatar Matt Menke Committed by Chromium LUCI CQ

DomainRelability + NetworkIsolationKeys CL 1

Wire NetworkIsolationKey through DomainReliabilityUploader.

Bug: 1138994
Change-Id: Ic1a9a317d87c0e4b32315b61d0ab3f412c3ecf01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587455Reviewed-by: default avatarLily Chen <chlily@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836677}
parent 203d04a0
...@@ -150,6 +150,7 @@ void DomainReliabilityContext::StartUpload() { ...@@ -150,6 +150,7 @@ void DomainReliabilityContext::StartUpload() {
uploader_->UploadReport( uploader_->UploadReport(
report_json, max_upload_depth, collector_url, report_json, max_upload_depth, collector_url,
net::NetworkIsolationKey::Todo(),
base::BindOnce(&DomainReliabilityContext::OnUploadComplete, base::BindOnce(&DomainReliabilityContext::OnUploadComplete,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
} }
......
...@@ -179,6 +179,7 @@ class DomainReliabilityContextTest : public testing::Test { ...@@ -179,6 +179,7 @@ class DomainReliabilityContextTest : public testing::Test {
void OnUploadRequest(const std::string& report_json, void OnUploadRequest(const std::string& report_json,
int max_upload_depth, int max_upload_depth,
const GURL& upload_url, const GURL& upload_url,
const net::NetworkIsolationKey& network_isolation_key,
DomainReliabilityUploader::UploadCallback callback) { DomainReliabilityUploader::UploadCallback callback) {
ASSERT_FALSE(upload_pending_); ASSERT_FALSE(upload_pending_);
upload_report_ = report_json; upload_report_ = report_json;
......
...@@ -90,12 +90,14 @@ MockUploader::~MockUploader() = default; ...@@ -90,12 +90,14 @@ MockUploader::~MockUploader() = default;
bool MockUploader::discard_uploads() const { return discard_uploads_; } bool MockUploader::discard_uploads() const { return discard_uploads_; }
void MockUploader::UploadReport(const std::string& report_json, void MockUploader::UploadReport(
const std::string& report_json,
int max_upload_depth, int max_upload_depth,
const GURL& upload_url, const GURL& upload_url,
const net::NetworkIsolationKey& network_isolation_key,
UploadCallback callback) { UploadCallback callback) {
std::move(callback_).Run(report_json, max_upload_depth, upload_url, std::move(callback_).Run(report_json, max_upload_depth, upload_url,
std::move(callback)); network_isolation_key, std::move(callback));
} }
void MockUploader::Shutdown() {} void MockUploader::Shutdown() {}
......
...@@ -15,6 +15,10 @@ ...@@ -15,6 +15,10 @@
#include "net/base/host_port_pair.h" #include "net/base/host_port_pair.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace net {
class NetworkIsolationKey;
} // namespace net
namespace domain_reliability { namespace domain_reliability {
// A simple test callback that remembers whether it's been called. // A simple test callback that remembers whether it's been called.
...@@ -37,9 +41,11 @@ class TestCallback { ...@@ -37,9 +41,11 @@ class TestCallback {
class MockUploader : public DomainReliabilityUploader { class MockUploader : public DomainReliabilityUploader {
public: public:
typedef base::OnceCallback<void(const std::string& report_json, typedef base::OnceCallback<void(
const std::string& report_json,
int max_upload_depth, int max_upload_depth,
const GURL& upload_url, const GURL& upload_url,
const net::NetworkIsolationKey& network_isolation_key,
UploadCallback upload_callback)> UploadCallback upload_callback)>
UploadRequestCallback; UploadRequestCallback;
...@@ -53,6 +59,7 @@ class MockUploader : public DomainReliabilityUploader { ...@@ -53,6 +59,7 @@ class MockUploader : public DomainReliabilityUploader {
void UploadReport(const std::string& report_json, void UploadReport(const std::string& report_json,
int max_upload_depth, int max_upload_depth,
const GURL& upload_url, const GURL& upload_url,
const net::NetworkIsolationKey& network_isolation_key,
UploadCallback callback) override; UploadCallback callback) override;
void Shutdown() override; void Shutdown() override;
void SetDiscardUploads(bool discard_uploads) override; void SetDiscardUploads(bool discard_uploads) override;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/supports_user_data.h" #include "base/supports_user_data.h"
#include "components/domain_reliability/util.h" #include "components/domain_reliability/util.h"
#include "net/base/elements_upload_data_stream.h" #include "net/base/elements_upload_data_stream.h"
#include "net/base/isolation_info.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/base/upload_bytes_element_reader.h" #include "net/base/upload_bytes_element_reader.h"
#include "net/http/http_response_headers.h" #include "net/http/http_response_headers.h"
...@@ -70,6 +71,7 @@ class DomainReliabilityUploaderImpl : public DomainReliabilityUploader, ...@@ -70,6 +71,7 @@ class DomainReliabilityUploaderImpl : public DomainReliabilityUploader,
const std::string& report_json, const std::string& report_json,
int max_upload_depth, int max_upload_depth,
const GURL& upload_url, const GURL& upload_url,
const net::NetworkIsolationKey& network_isolation_key,
DomainReliabilityUploader::UploadCallback callback) override { DomainReliabilityUploader::UploadCallback callback) override {
DVLOG(1) << "Uploading report to " << upload_url; DVLOG(1) << "Uploading report to " << upload_url;
DVLOG(2) << "Report JSON: " << report_json; DVLOG(2) << "Report JSON: " << report_json;
...@@ -119,6 +121,8 @@ class DomainReliabilityUploaderImpl : public DomainReliabilityUploader, ...@@ -119,6 +121,8 @@ class DomainReliabilityUploaderImpl : public DomainReliabilityUploader,
request->set_allow_credentials(false); request->set_allow_credentials(false);
request->SetExtraRequestHeaderByName(net::HttpRequestHeaders::kContentType, request->SetExtraRequestHeaderByName(net::HttpRequestHeaders::kContentType,
kJsonMimeType, true /* overwrite */); kJsonMimeType, true /* overwrite */);
request->set_isolation_info(net::IsolationInfo::CreatePartial(
net::IsolationInfo::RequestType::kOther, network_isolation_key));
std::vector<char> report_data(report_json.begin(), report_json.end()); std::vector<char> report_data(report_json.begin(), report_json.end());
auto upload_reader = auto upload_reader =
std::make_unique<net::UploadOwnedBytesElementReader>(&report_data); std::make_unique<net::UploadOwnedBytesElementReader>(&report_data);
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "url/gurl.h" #include "url/gurl.h"
namespace net { namespace net {
class NetworkIsolationKey;
class URLRequest; class URLRequest;
class URLRequestContext; class URLRequestContext;
} // namespace net } // namespace net
...@@ -55,9 +56,11 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityUploader { ...@@ -55,9 +56,11 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityUploader {
// Uploads |report_json| to |upload_url| and calls |callback| when the upload // Uploads |report_json| to |upload_url| and calls |callback| when the upload
// has either completed or failed. // has either completed or failed.
virtual void UploadReport(const std::string& report_json, virtual void UploadReport(
const std::string& report_json,
int max_beacon_depth, int max_beacon_depth,
const GURL& upload_url, const GURL& upload_url,
const net::NetworkIsolationKey& network_isolation_key,
UploadCallback callback) = 0; UploadCallback callback) = 0;
// Shuts down the uploader prior to destruction. Currently, terminates pending // Shuts down the uploader prior to destruction. Currently, terminates pending
......
...@@ -16,7 +16,9 @@ ...@@ -16,7 +16,9 @@
#include "base/test/test_simple_task_runner.h" #include "base/test/test_simple_task_runner.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "components/domain_reliability/test_util.h" #include "components/domain_reliability/test_util.h"
#include "net/base/isolation_info.h"
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
#include "net/base/network_isolation_key.h"
#include "net/http/http_response_headers.h" #include "net/http/http_response_headers.h"
#include "net/http/http_response_info.h" #include "net/http/http_response_info.h"
#include "net/log/net_log_with_source.h" #include "net/log/net_log_with_source.h"
...@@ -101,12 +103,19 @@ class UploadMockURLRequestJob : public net::URLRequestJob { ...@@ -101,12 +103,19 @@ class UploadMockURLRequestJob : public net::URLRequestJob {
class UploadInterceptor : public net::URLRequestInterceptor { class UploadInterceptor : public net::URLRequestInterceptor {
public: public:
UploadInterceptor() : request_count_(0), last_upload_depth_(-1) {} explicit UploadInterceptor(
const net::IsolationInfo& expected_network_isolation_info)
: expected_network_isolation_info_(expected_network_isolation_info),
request_count_(0),
last_upload_depth_(-1) {}
~UploadInterceptor() override { EXPECT_TRUE(results_.empty()); } ~UploadInterceptor() override { EXPECT_TRUE(results_.empty()); }
std::unique_ptr<net::URLRequestJob> MaybeInterceptRequest( std::unique_ptr<net::URLRequestJob> MaybeInterceptRequest(
net::URLRequest* request) const override { net::URLRequest* request) const override {
EXPECT_TRUE(expected_network_isolation_info_.IsEqualForTesting(
request->isolation_info()));
EXPECT_FALSE(results_.empty()); EXPECT_FALSE(results_.empty());
MockUploadResult result = results_.front(); MockUploadResult result = results_.front();
results_.pop_front(); results_.pop_front();
...@@ -137,6 +146,8 @@ class UploadInterceptor : public net::URLRequestInterceptor { ...@@ -137,6 +146,8 @@ class UploadInterceptor : public net::URLRequestInterceptor {
int last_upload_depth() const { return last_upload_depth_; } int last_upload_depth() const { return last_upload_depth_; }
private: private:
const net::IsolationInfo expected_network_isolation_info_;
mutable std::list<MockUploadResult> results_; mutable std::list<MockUploadResult> results_;
mutable int request_count_; mutable int request_count_;
mutable int last_upload_depth_; mutable int last_upload_depth_;
...@@ -171,7 +182,8 @@ class DomainReliabilityUploaderTest : public testing::Test { ...@@ -171,7 +182,8 @@ class DomainReliabilityUploaderTest : public testing::Test {
DomainReliabilityUploaderTest() DomainReliabilityUploaderTest()
: uploader_( : uploader_(
DomainReliabilityUploader::Create(&time_, &url_request_context_)) { DomainReliabilityUploader::Create(&time_, &url_request_context_)) {
auto interceptor = std::make_unique<UploadInterceptor>(); auto interceptor =
std::make_unique<UploadInterceptor>(expected_isolation_info_);
interceptor_ = interceptor.get(); interceptor_ = interceptor.get();
net::URLRequestFilter::GetInstance()->AddUrlInterceptor( net::URLRequestFilter::GetInstance()->AddUrlInterceptor(
GURL(kUploadURL), std::move(interceptor)); GURL(kUploadURL), std::move(interceptor));
...@@ -188,9 +200,17 @@ class DomainReliabilityUploaderTest : public testing::Test { ...@@ -188,9 +200,17 @@ class DomainReliabilityUploaderTest : public testing::Test {
return &url_request_context_; return &url_request_context_;
} }
const net::NetworkIsolationKey& network_isolation_key() const {
return expected_isolation_info_.network_isolation_key();
}
private: private:
base::test::SingleThreadTaskEnvironment task_environment_{ base::test::SingleThreadTaskEnvironment task_environment_{
base::test::SingleThreadTaskEnvironment::MainThreadType::IO}; base::test::SingleThreadTaskEnvironment::MainThreadType::IO};
const net::IsolationInfo expected_isolation_info_ =
net::IsolationInfo::CreateTransient();
net::TestURLRequestContext url_request_context_; net::TestURLRequestContext url_request_context_;
UploadInterceptor* interceptor_; UploadInterceptor* interceptor_;
MockTime time_; MockTime time_;
...@@ -205,7 +225,8 @@ TEST_F(DomainReliabilityUploaderTest, SuccessfulUpload) { ...@@ -205,7 +225,8 @@ TEST_F(DomainReliabilityUploaderTest, SuccessfulUpload) {
interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 200\r\n\r\n"); interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 200\r\n\r\n");
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 0, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
EXPECT_TRUE(c.last_result().is_success()); EXPECT_TRUE(c.last_result().is_success());
...@@ -217,7 +238,8 @@ TEST_F(DomainReliabilityUploaderTest, NetworkErrorUpload) { ...@@ -217,7 +238,8 @@ TEST_F(DomainReliabilityUploaderTest, NetworkErrorUpload) {
interceptor()->ExpectRequestAndReturnError(net::ERR_CONNECTION_REFUSED); interceptor()->ExpectRequestAndReturnError(net::ERR_CONNECTION_REFUSED);
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 0, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
EXPECT_TRUE(c.last_result().is_failure()); EXPECT_TRUE(c.last_result().is_failure());
...@@ -229,7 +251,8 @@ TEST_F(DomainReliabilityUploaderTest, ServerErrorUpload) { ...@@ -229,7 +251,8 @@ TEST_F(DomainReliabilityUploaderTest, ServerErrorUpload) {
interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 500\r\n\r\n"); interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 500\r\n\r\n");
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 0, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
EXPECT_TRUE(c.last_result().is_failure()); EXPECT_TRUE(c.last_result().is_failure());
...@@ -242,7 +265,8 @@ TEST_F(DomainReliabilityUploaderTest, RetryAfterUpload) { ...@@ -242,7 +265,8 @@ TEST_F(DomainReliabilityUploaderTest, RetryAfterUpload) {
"HTTP/1.1 503 Ugh\nRetry-After: 3600\n\n"); "HTTP/1.1 503 Ugh\nRetry-After: 3600\n\n");
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 0, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
EXPECT_TRUE(c.last_result().is_retry_after()); EXPECT_TRUE(c.last_result().is_retry_after());
...@@ -254,7 +278,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadDepth1) { ...@@ -254,7 +278,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadDepth1) {
interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 200\r\n\r\n"); interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 200\r\n\r\n");
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 0, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
...@@ -267,7 +292,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadDepth2) { ...@@ -267,7 +292,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadDepth2) {
interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 200\r\n\r\n"); interceptor()->ExpectRequestAndReturnResponseHeaders("HTTP/1.1 200\r\n\r\n");
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 1, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 1, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
...@@ -280,7 +306,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadCanceledAtShutdown) { ...@@ -280,7 +306,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadCanceledAtShutdown) {
interceptor()->ExpectRequestAndReturnError(net::ERR_IO_PENDING); interceptor()->ExpectRequestAndReturnError(net::ERR_IO_PENDING);
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 1, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 1, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, interceptor()->request_count()); EXPECT_EQ(1, interceptor()->request_count());
EXPECT_EQ(0u, c.called_count()); EXPECT_EQ(0u, c.called_count());
...@@ -296,7 +323,8 @@ TEST_F(DomainReliabilityUploaderTest, NoUploadAfterShutdown) { ...@@ -296,7 +323,8 @@ TEST_F(DomainReliabilityUploaderTest, NoUploadAfterShutdown) {
uploader()->Shutdown(); uploader()->Shutdown();
TestUploadCallback c; TestUploadCallback c;
uploader()->UploadReport("{}", 1, GURL(kUploadURL), c.callback()); uploader()->UploadReport("{}", 1, GURL(kUploadURL), network_isolation_key(),
c.callback());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, c.called_count()); EXPECT_EQ(1u, c.called_count());
EXPECT_EQ(0, interceptor()->request_count()); EXPECT_EQ(0, interceptor()->request_count());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment