diff options
Diffstat (limited to 'chromium/content/browser/loader')
30 files changed, 830 insertions, 470 deletions
diff --git a/chromium/content/browser/loader/cross_site_document_blocking_browsertest.cc b/chromium/content/browser/loader/cross_site_document_blocking_browsertest.cc index 321ab2c8e87..fa11009de25 100644 --- a/chromium/content/browser/loader/cross_site_document_blocking_browsertest.cc +++ b/chromium/content/browser/loader/cross_site_document_blocking_browsertest.cc @@ -89,7 +89,7 @@ void InspectHistograms(const base::HistogramTester& histograms, const std::string& resource_name, ResourceType resource_type) { // //services/network doesn't have access to content::ResourceType and - // therefore cannot log some XSDB UMAs. + // therefore cannot log some CORB UMAs. bool is_restricted_uma_expected = false; if (base::FeatureList::IsEnabled(network::features::kNetworkService)) { is_restricted_uma_expected = true; @@ -613,38 +613,21 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingTest, BlockHeaders) { ASSERT_EQ(net::OK, interceptor.completion_status().error_code); ASSERT_TRUE(interceptor.completion_status().should_report_corb_blocking); - // Verify that safelisted headers have not been removed by XSDB. - // See https://fetch.spec.whatwg.org/#cors-safelisted-response-header-name. + // Verify that most response headers have been removed by CORB. const std::string& headers = interceptor.response_head().headers->raw_headers(); - EXPECT_THAT(headers, - HasSubstr("Cache-Control: no-cache, no-store, must-revalidate")); - EXPECT_THAT(headers, HasSubstr("Content-Language: TestLanguage")); - EXPECT_THAT(headers, - HasSubstr("Content-Type: application/json; charset=utf-8")); - EXPECT_THAT(headers, HasSubstr("Expires: Wed, 21 Oct 2199 07:28:00 GMT")); - EXPECT_THAT(headers, - HasSubstr("Last-Modified: Wed, 07 Feb 2018 13:55:00 PST")); - EXPECT_THAT(headers, HasSubstr("Pragma: TestPragma")); - - // Make sure the test covers all the safelisted headers known to the product - // code. - for (const std::string& safelisted_header : - network::CrossOriginReadBlocking::GetCorsSafelistedHeadersForTesting()) { - EXPECT_TRUE( - interceptor.response_head().headers->HasHeader(safelisted_header)); - - std::string value; - interceptor.response_head().headers->EnumerateHeader( - nullptr, safelisted_header, &value); - EXPECT_FALSE(value.empty()); - } - - // Verify that other response headers have been removed by XSDB. + EXPECT_THAT(headers, HasSubstr("Access-Control-Allow-Origin: https://other")); + EXPECT_THAT(headers, Not(HasSubstr("Cache-Control"))); + EXPECT_THAT(headers, Not(HasSubstr("Content-Language"))); EXPECT_THAT(headers, Not(HasSubstr("Content-Length"))); - EXPECT_THAT(headers, Not(HasSubstr("X-My-Secret-Header"))); + EXPECT_THAT(headers, Not(HasSubstr("Content-Type"))); + EXPECT_THAT(headers, Not(HasSubstr("Expires"))); + EXPECT_THAT(headers, Not(HasSubstr("Last-Modified"))); EXPECT_THAT(headers, Not(HasSubstr("MySecretCookieKey"))); EXPECT_THAT(headers, Not(HasSubstr("MySecretCookieValue"))); + EXPECT_THAT(headers, Not(HasSubstr("Pragma"))); + EXPECT_THAT(headers, Not(HasSubstr("X-Content-Type-Options"))); + EXPECT_THAT(headers, Not(HasSubstr("X-My-Secret-Header"))); // Verify that the body is empty. EXPECT_EQ("", interceptor.response_body()); @@ -899,7 +882,7 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingServiceWorkerTest, NoNetwork) { }); )"; EXPECT_TRUE(ExecuteScriptAndExtractString(shell(), script, &response)); - // Verify that XSDB didn't block the response (since it was "faked" within the + // Verify that CORB didn't block the response (since it was "faked" within the // service worker and didn't cross any security boundaries). EXPECT_EQ("Response created by service worker", response); InspectHistograms(histograms, kShouldBeAllowedWithoutSniffing, "blah.html", @@ -938,13 +921,13 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingServiceWorkerTest, std::string response; EXPECT_TRUE(ExecuteScriptAndExtractString(shell(), script, &response)); - // Verify that XSDB blocked the response from the network (from + // Verify that CORB blocked the response from the network (from // |cross_origin_https_server_|) to the service worker. InspectHistograms(histograms, kShouldBeBlockedWithoutSniffing, "network.txt", RESOURCE_TYPE_XHR); // Verify that the service worker replied with an expected error. - // Replying with an error means that XSDB is only active once (for the + // Replying with an error means that CORB is only active once (for the // initial, real network request) and therefore the test doesn't get // confused (second successful response would have added noise to the // histograms captured by the test). diff --git a/chromium/content/browser/loader/cross_site_document_resource_handler.cc b/chromium/content/browser/loader/cross_site_document_resource_handler.cc index 90792270128..585868689f6 100644 --- a/chromium/content/browser/loader/cross_site_document_resource_handler.cc +++ b/chromium/content/browser/loader/cross_site_document_resource_handler.cc @@ -221,6 +221,16 @@ void CrossSiteDocumentResourceHandler::OnResponseStarted( std::unique_ptr<ResourceController> controller) { has_response_started_ = true; + if (request()->initiator().has_value()) { + const char* initiator_scheme_exception = + GetContentClient() + ->browser() + ->GetInitiatorSchemeBypassingDocumentBlocking(); + is_initiator_scheme_excluded_ = + initiator_scheme_exception && + request()->initiator().value().scheme() == initiator_scheme_exception; + } + network::CrossOriginReadBlocking::LogAction( network::CrossOriginReadBlocking::Action::kResponseStarted); @@ -358,6 +368,12 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted( return; } + // If |next_handler_->OnReadCompleted(...)| was not called above, then the + // response bytes are being accumulated in the local buffer we've allocated in + // ResumeOnWillRead. + const size_t new_data_offset = local_buffer_bytes_read_; + local_buffer_bytes_read_ += bytes_read; + // If we intended to block the response and haven't sniffed yet, try to // confirm that we should block it. If sniffing is needed, look at the local // buffer and either report that zero bytes were read (to indicate the @@ -377,8 +393,6 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted( // JSONP, or another allowable data type and we should let it through. // Record how many bytes were read to see how often it's too small. (This // will typically be under 100,000.) - const size_t new_data_offset = local_buffer_bytes_read_; - local_buffer_bytes_read_ += bytes_read; DCHECK_LE(local_buffer_bytes_read_, next_handler_buffer_size_); const bool more_data_possible = bytes_read != 0 && local_buffer_bytes_read_ < net::kMaxBytesToSniff && @@ -406,6 +420,16 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted( } } + // At this point the block-vs-allow decision was made, but might be still + // suppressed because of |is_initiator_scheme_excluded_|. We perform the + // suppression at such a late point, because we want to ensure we only call + // LogInitiatorSchemeBypassingDocumentBlocking for cases that actuall matter + // in practice. + if (confirmed_blockable && is_initiator_scheme_excluded_) { + initiator_scheme_prevented_blocking_ = true; + confirmed_blockable = false; + } + // At this point we have already made a block-vs-allow decision and we know // that we can wake the |next_handler_| and let it catch-up with our // processing of the response. The first step will always be calling @@ -532,10 +556,22 @@ void CrossSiteDocumentResourceHandler::OnResponseCompleted( next_handler_->OnResponseCompleted(net::URLRequestStatus(), std::move(controller)); } else { - // Only report XSDB status for successful (i.e. non-aborted, + // Only report CORB status for successful (i.e. non-aborted, // non-errored-out) requests. - if (status.is_success()) + if (status.is_success()) { analyzer_->LogAllowedResponse(); + if (initiator_scheme_prevented_blocking_ && + analyzer_->ShouldReportBlockedResponse() && GetRequestInfo()) { + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::BindOnce(&ContentBrowserClient:: + LogInitiatorSchemeBypassingDocumentBlocking, + base::Unretained(GetContentClient()->browser()), + request()->initiator().value(), + GetRequestInfo()->GetChildID(), + GetRequestInfo()->GetResourceType())); + } + } next_handler_->OnResponseCompleted(status, std::move(controller)); } @@ -543,16 +579,12 @@ void CrossSiteDocumentResourceHandler::OnResponseCompleted( bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders( const network::ResourceResponse& response) { - // Give embedder a chance to skip document blocking for this response. - const char* initiator_scheme_exception = - GetContentClient() - ->browser() - ->GetInitatorSchemeBypassingDocumentBlocking(); + DCHECK_CURRENTLY_ON(BrowserThread::IO); // Delegate most decisions to CrossOriginReadBlocking::ResponseAnalyzer. analyzer_ = std::make_unique<network::CrossOriginReadBlocking::ResponseAnalyzer>( - *request(), response, initiator_scheme_exception); + *request(), response); if (analyzer_->ShouldAllow()) return false; @@ -582,14 +614,22 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders( if (!info || info->GetChildID() == -1) return false; - // Don't block plugin requests. - // TODO(lukasza): Only disable CORB for plugins with universal access (see - // PepperURLLoaderHost::has_universal_access_), because only such plugins may - // have their own CORS-like mechanisms - e.g. crossdomain.xml in Flash). We - // should still enforce CORB for other kinds of plugins (i.e. ones without - // universal access). + // Don't block some plugin requests. + // + // Note that in practice this exception only only matters to Flash and test + // plugins (both can issue requests without CORS and both will be covered by + // CORB::ShouldAllowForPlugin below). + // + // This exception is not needed for: + // - PNaCl (which always issues CORS requests) + // - PDF (which is already covered by the exception for chrome-extension://... + // initiators and therefore doesn't need another exception here; + // additionally PDF doesn't _really_ make *cross*-origin requests - it just + // seems that way because of the usage of the Chrome extension). if (info->GetResourceType() == RESOURCE_TYPE_PLUGIN_RESOURCE && - is_nocors_plugin_request_) { + is_nocors_plugin_request_ && + network::CrossOriginReadBlocking::ShouldAllowForPlugin( + info->GetChildID())) { return false; } diff --git a/chromium/content/browser/loader/cross_site_document_resource_handler.h b/chromium/content/browser/loader/cross_site_document_resource_handler.h index 1598fb024bd..5f11dc707ce 100644 --- a/chromium/content/browser/loader/cross_site_document_resource_handler.h +++ b/chromium/content/browser/loader/cross_site_document_resource_handler.h @@ -164,6 +164,14 @@ class CONTENT_EXPORT CrossSiteDocumentResourceHandler // completed, and thus it is safe to cancel or detach on the next read. bool blocked_read_completed_ = false; + // Whether the request should be allowed because of + // ContentBrowserClient::GetInitatorSchemeBypassingDocumentBlocking + bool is_initiator_scheme_excluded_ = false; + + // Whether |is_initiator_scheme_excluded_| actually prevented blocking from + // happening. + bool initiator_scheme_prevented_blocking_ = false; + base::WeakPtrFactory<CrossSiteDocumentResourceHandler> weak_this_; DISALLOW_COPY_AND_ASSIGN(CrossSiteDocumentResourceHandler); diff --git a/chromium/content/browser/loader/cross_site_document_resource_handler_unittest.cc b/chromium/content/browser/loader/cross_site_document_resource_handler_unittest.cc index 99405d1b48e..e09e9a1b602 100644 --- a/chromium/content/browser/loader/cross_site_document_resource_handler_unittest.cc +++ b/chromium/content/browser/loader/cross_site_document_resource_handler_unittest.cc @@ -345,7 +345,13 @@ const TestScenario kScenarios[] = { kVerdictPacketForHeadersBasedVerdict, // verdict_packet }, { - "Allowed: Cross-site fetch HTML from Flash without CORS", + // Blocked, because the unit test doesn't make a call to + // CrossOriginReadBlocking::AddExceptionForFlash (simulating a behavior + // of a compromised renderer that only pretends to be hosting Flash). + // + // The regular scenario is covered by integration tests: + // OutOfProcessPPAPITest.URLLoaderTrusted. + "Blocked: Cross-site fetch HTML from Flash without CORS", __LINE__, "http://www.b.com/plugin.html", // target_url RESOURCE_TYPE_PLUGIN_RESOURCE, // resource_type @@ -357,7 +363,7 @@ const TestScenario kScenarios[] = { false, // simulate_range_response AccessControlAllowOriginHeader::kOmit, // cors_response {"<html><head>this should sniff as HTML"}, // packets - Verdict::kAllow, // verdict + Verdict::kBlock, // verdict kVerdictPacketForHeadersBasedVerdict, // verdict_packet }, { diff --git a/chromium/content/browser/loader/global_routing_id.h b/chromium/content/browser/loader/global_routing_id.h deleted file mode 100644 index 3f5aadf94c0..00000000000 --- a/chromium/content/browser/loader/global_routing_id.h +++ /dev/null @@ -1,72 +0,0 @@ -// 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. - -#ifndef CONTENT_BROWSER_LOADER_GLOBAL_ROUTING_ID_H_ -#define CONTENT_BROWSER_LOADER_GLOBAL_ROUTING_ID_H_ - -#include <tuple> - -#include "ipc/ipc_message.h" - -namespace content { - -// Uniquely identifies the route from which a net::URLRequest comes. -struct GlobalRoutingID { - GlobalRoutingID() : child_id(-1), route_id(-1) { - } - - GlobalRoutingID(int child_id, int route_id) - : child_id(child_id), - route_id(route_id) { - } - - // The unique ID of the child process (different from OS's PID). - int child_id; - - // The route ID (unique for each URLRequest source). - int route_id; - - bool operator<(const GlobalRoutingID& other) const { - return std::tie(child_id, route_id) < - std::tie(other.child_id, other.route_id); - } - bool operator==(const GlobalRoutingID& other) const { - return child_id == other.child_id && - route_id == other.route_id; - } - bool operator!=(const GlobalRoutingID& other) const { - return !(*this == other); - } -}; - -// Same as GlobalRoutingID except the route_id must be a RenderFrameHost routing -// id. -struct GlobalFrameRoutingId { - GlobalFrameRoutingId() : child_id(0), frame_routing_id(MSG_ROUTING_NONE) {} - - GlobalFrameRoutingId(int child_id, int frame_routing_id) - : child_id(child_id), frame_routing_id(frame_routing_id) {} - - // The unique ID of the child process (different from OS's PID). - int child_id; - - // The route ID (unique for each URLRequest source). - int frame_routing_id; - - bool operator<(const GlobalFrameRoutingId& other) const { - return std::tie(child_id, frame_routing_id) < - std::tie(other.child_id, other.frame_routing_id); - } - bool operator==(const GlobalFrameRoutingId& other) const { - return child_id == other.child_id && - frame_routing_id == other.frame_routing_id; - } - bool operator!=(const GlobalFrameRoutingId& other) const { - return !(*this == other); - } -}; - -} // namespace content - -#endif // CONTENT_BROWSER_LOADER_GLOBAL_ROUTING_ID_H_ diff --git a/chromium/content/browser/loader/loader_browsertest.cc b/chromium/content/browser/loader/loader_browsertest.cc index 024fadee921..927f2af62d3 100644 --- a/chromium/content/browser/loader/loader_browsertest.cc +++ b/chromium/content/browser/loader/loader_browsertest.cc @@ -25,6 +25,7 @@ #include "content/public/browser/render_frame_host.h" #include "content/public/browser/resource_dispatcher_host_delegate.h" #include "content/public/browser/resource_request_info.h" +#include "content/public/browser/site_isolation_policy.h" #include "content/public/browser/web_contents.h" #include "content/public/common/browser_side_navigation_policy.h" #include "content/public/common/previews_state.h" @@ -627,6 +628,100 @@ IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, PageTransitionClientRedirect) { EXPECT_TRUE(delegate.page_transition() & ui::PAGE_TRANSITION_CLIENT_REDIRECT); } +IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, SubresourceRedirectToDataURLBlocked) { + ASSERT_TRUE(embedded_test_server()->Start()); + + EXPECT_TRUE(NavigateToURL(shell(), embedded_test_server()->GetURL("/echo"))); + + GURL subresource_url = embedded_test_server()->GetURL( + "/server-redirect?data:text/plain,redirected1"); + std::string script = R"((url => { + var xhr = new XMLHttpRequest(); + xhr.open('GET', url, true); + xhr.onload = () => domAutomationController.send("ALLOWED"); + xhr.onerror = () => domAutomationController.send("BLOCKED"); + xhr.send(); + }))"; + std::string result; + ASSERT_TRUE(ExecuteScriptAndExtractString( + shell(), script + "('" + subresource_url.spec() + "')", &result)); + + EXPECT_EQ("BLOCKED", result); +} + +IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, RedirectToDataURLBlocked) { + ASSERT_TRUE(embedded_test_server()->Start()); + EXPECT_FALSE(NavigateToURL( + shell(), embedded_test_server()->GetURL( + "/server-redirect?data:text/plain,redirected1"))); +} + +IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, RedirectToAboutURLBlocked) { + ASSERT_TRUE(embedded_test_server()->Start()); + EXPECT_FALSE(NavigateToURL( + shell(), embedded_test_server()->GetURL( + "/server-redirect?" + std::string(url::kAboutBlankURL)))); +} + +namespace { + +// Creates a valid filesystem URL. +GURL CreateFileSystemURL(Shell* window) { + std::string filesystem_url_string; + EXPECT_TRUE( + ExecuteScriptAndExtractString(window, R"( + var blob = new Blob(['<html><body>hello</body></html>'], + {type: 'text/html'}); + window.webkitRequestFileSystem(TEMPORARY, blob.size, fs => { + fs.root.getFile('foo.html', {create: true}, file => { + file.createWriter(writer => { + writer.write(blob); + writer.onwriteend = () => { + domAutomationController.send(file.toURL()); + } + }); + }); + });)", &filesystem_url_string)); + GURL filesystem_url(filesystem_url_string); + EXPECT_TRUE(filesystem_url.is_valid()); + EXPECT_TRUE(filesystem_url.SchemeIsFileSystem()); + return filesystem_url; +} + +} // namespace + +IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, + SubresourceRedirectToFileSystemURLBlocked) { + ASSERT_TRUE(embedded_test_server()->Start()); + + EXPECT_TRUE(NavigateToURL(shell(), embedded_test_server()->GetURL("/echo"))); + + GURL subresource_url = embedded_test_server()->GetURL( + "/server-redirect?" + CreateFileSystemURL(shell()).spec()); + std::string script = R"((url => { + var xhr = new XMLHttpRequest(); + xhr.open('GET', url, true); + xhr.onload = () => domAutomationController.send("ALLOWED"); + xhr.onerror = () => domAutomationController.send("BLOCKED"); + xhr.send(); + }))"; + std::string result; + ASSERT_TRUE(ExecuteScriptAndExtractString( + shell(), script + "('" + subresource_url.spec() + "')", &result)); + + EXPECT_EQ("BLOCKED", result); +} + +IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, RedirectToFileSystemURLBlocked) { + ASSERT_TRUE(embedded_test_server()->Start()); + // Need to navigate to a URL first so the filesystem can be created. + EXPECT_TRUE(NavigateToURL(shell(), embedded_test_server()->GetURL("/echo"))); + + EXPECT_FALSE(NavigateToURL( + shell(), embedded_test_server()->GetURL( + "/server-redirect?" + CreateFileSystemURL(shell()).spec()))); +} + namespace { // Checks whether the given urls are requested, and that GetPreviewsState() @@ -1172,6 +1267,11 @@ IN_PROC_BROWSER_TEST_F(RequestDataBrowserTest, CrossOriginNested) { // bypass cookies SameSite=Strict protections by navigating a new window twice. IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, CookieSameSiteStrictOpenNewNamedWindowTwice) { + // TODO(lukasza): https://crbug.com/417518: Get tests working with + // --site-per-process. + if (SiteIsolationPolicy::UseDedicatedProcessesForAllSites()) + return; + ASSERT_TRUE(embedded_test_server()->Start()); // 1) Add cookies for 'a.com', one of them with the "SameSite=Strict" option. diff --git a/chromium/content/browser/loader/loader_io_thread_notifier.cc b/chromium/content/browser/loader/loader_io_thread_notifier.cc index 47efe2831fe..215e2f95fac 100644 --- a/chromium/content/browser/loader/loader_io_thread_notifier.cc +++ b/chromium/content/browser/loader/loader_io_thread_notifier.cc @@ -5,9 +5,9 @@ #include "content/browser/loader/loader_io_thread_notifier.h" #include "content/browser/frame_host/render_frame_host_impl.h" -#include "content/browser/loader/global_routing_id.h" #include "content/browser/loader/resource_dispatcher_host_impl.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/global_routing_id.h" namespace content { diff --git a/chromium/content/browser/loader/merkle_integrity_source_stream.cc b/chromium/content/browser/loader/merkle_integrity_source_stream.cc index 94c532f18e2..77497667434 100644 --- a/chromium/content/browser/loader/merkle_integrity_source_stream.cc +++ b/chromium/content/browser/loader/merkle_integrity_source_stream.cc @@ -6,7 +6,7 @@ #include <string.h> -#include "base/base64url.h" +#include "base/base64.h" #include "base/big_endian.h" #include "base/numerics/safe_conversions.h" #include "net/base/io_buffer.h" @@ -19,7 +19,7 @@ namespace { // maximum record size in TLS and the default maximum frame size in HTTP/2. constexpr uint64_t kMaxRecordSize = 16 * 1024; -constexpr char kMiSha256Header[] = "mi-sha256-draft2="; +constexpr char kMiSha256Header[] = "mi-sha256-03="; constexpr size_t kMiSha256HeaderLength = sizeof(kMiSha256Header) - 1; // Copies as many bytes from |input| as will fit in |output| and advances both. @@ -34,16 +34,14 @@ size_t CopyClamped(base::span<const char>* input, base::span<char>* output) { } // namespace MerkleIntegritySourceStream::MerkleIntegritySourceStream( - base::StringPiece mi_header_value, + base::StringPiece digest_header_value, std::unique_ptr<SourceStream> upstream) // TODO(ksakamoto): Use appropriate SourceType. : net::FilterSourceStream(SourceStream::TYPE_NONE, std::move(upstream)) { - // TODO(ksakamoto): Support quoted parameter value. std::string next_proof; - if (!mi_header_value.starts_with(kMiSha256Header) || - !base::Base64UrlDecode(mi_header_value.substr(kMiSha256HeaderLength), - base::Base64UrlDecodePolicy::DISALLOW_PADDING, - &next_proof) || + if (!digest_header_value.starts_with(kMiSha256Header) || + !base::Base64Decode(digest_header_value.substr(kMiSha256HeaderLength), + &next_proof) || next_proof.size() != SHA256_DIGEST_LENGTH) { failed_ = true; } else { @@ -91,7 +89,17 @@ bool MerkleIntegritySourceStream::FilterDataImpl(base::span<char>* output, if (record_size_ == 0) { base::span<const char> bytes; if (!ConsumeBytes(input, 8, &bytes, &storage)) { - return !upstream_eof_reached; + if (!upstream_eof_reached) { + return true; // Wait for more data later. + } + if (partial_input_.empty()) { + // As a special case, the encoding of an empty payload is itself an + // empty message (i.e. it omits the initial record size), and its + // integrity proof is SHA-256("\0"). + final_record_done_ = true; + return ProcessRecord({}, final_record_done_, output); + } + return false; } uint64_t record_size; base::ReadBigEndian(bytes.data(), &record_size); @@ -124,14 +132,8 @@ bool MerkleIntegritySourceStream::FilterDataImpl(base::span<char>* output, } // The final record is shorter and does not contain a hash. Process all - // remaining input the final record. - // - // TODO(davidben): This matches the previous implementation in that it - // allows empty final records, but this does not match the specification - // and means some inputs have two valid encodings. However, the - // specification's version cannot represent the empty string. Update this - // when https://github.com/martinthomson/http-mice/issues/3 is resolved. - if (partial_input_.size() > record_size_) { + // remaining input as the final record. + if (partial_input_.empty() || partial_input_.size() > record_size_) { return false; } record = partial_input_; diff --git a/chromium/content/browser/loader/merkle_integrity_source_stream.h b/chromium/content/browser/loader/merkle_integrity_source_stream.h index d2554372a48..7fe7e4f18b6 100644 --- a/chromium/content/browser/loader/merkle_integrity_source_stream.h +++ b/chromium/content/browser/loader/merkle_integrity_source_stream.h @@ -19,12 +19,12 @@ namespace content { // MerkleIntegritySourceStream decodes and validates content encoded with the // "mi-sha256" content encoding -// (https://tools.ietf.org/html/draft-thomson-http-mice-02). +// (https://tools.ietf.org/html/draft-thomson-http-mice-03). // TODO(ksakamoto): This class should eventually live in src/net/filter/. class CONTENT_EXPORT MerkleIntegritySourceStream : public net::FilterSourceStream { public: - MerkleIntegritySourceStream(base::StringPiece mi_header_value, + MerkleIntegritySourceStream(base::StringPiece digest_header_value, std::unique_ptr<SourceStream> upstream); ~MerkleIntegritySourceStream() override; diff --git a/chromium/content/browser/loader/merkle_integrity_source_stream_unittest.cc b/chromium/content/browser/loader/merkle_integrity_source_stream_unittest.cc index 61c61692ec5..8765b844ae8 100644 --- a/chromium/content/browser/loader/merkle_integrity_source_stream_unittest.cc +++ b/chromium/content/browser/loader/merkle_integrity_source_stream_unittest.cc @@ -3,7 +3,7 @@ // found in the LICENSE file. #include "content/browser/loader/merkle_integrity_source_stream.h" -#include "base/base64url.h" +#include "base/base64.h" #include "net/base/io_buffer.h" #include "net/base/test_completion_callback.h" #include "net/filter/mock_source_stream.h" @@ -17,13 +17,13 @@ const int kBigBufferSize = 4096; const int kSmallBufferSize = 1; const char kMIEmptyBody[] = - "mi-sha256-draft2=bjQLnP-zepicpUTmu3gKLHiQHT-zNzh2hRGjBhevoB0"; + "mi-sha256-03=bjQLnP+zepicpUTmu3gKLHiQHT+zNzh2hRGjBhevoB0="; const char kMISingleRecord[] = - "mi-sha256-draft2=dcRDgR2GM35DluAV13PzgnG6-pvQwPywfFvAu1UeFrs"; + "mi-sha256-03=dcRDgR2GM35DluAV13PzgnG6+pvQwPywfFvAu1UeFrs="; const char kMIMultipleRecords[] = - "mi-sha256-draft2=IVa9shfs0nyKEhHqtB3WVNANJ2Njm5KjQLjRtnbkYJ4"; + "mi-sha256-03=IVa9shfs0nyKEhHqtB3WVNANJ2Njm5KjQLjRtnbkYJ4="; const char kMIWholeNumberOfRecords[] = - "mi-sha256-draft2=L2vdwBplKvIr0ZPkcuskWZfEVDgVdHa6aD363UpKuZs"; + "mi-sha256-03=L2vdwBplKvIr0ZPkcuskWZfEVDgVdHa6aD363UpKuZs="; enum class ReadResultType { // Each call to AddReadResult is a separate read from the lower layer @@ -112,8 +112,7 @@ class MerkleIntegritySourceStreamTest std::string Base64Decode(const char* hash) { std::string out; - EXPECT_TRUE(base::Base64UrlDecode( - hash, base::Base64UrlDecodePolicy::DISALLOW_PADDING, &out)); + EXPECT_TRUE(base::Base64Decode(hash, &out)); EXPECT_EQ(32u, out.size()); return out; } @@ -160,6 +159,14 @@ TEST_P(MerkleIntegritySourceStreamTest, EmptyStream) { source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); std::string actual_output; int result = ReadStream(&actual_output); + EXPECT_EQ(net::OK, result); +} + +TEST_P(MerkleIntegritySourceStreamTest, EmptyStreamWrongHash) { + Init(kMISingleRecord); + source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); + std::string actual_output; + int result = ReadStream(&actual_output); EXPECT_EQ(net::ERR_CONTENT_DECODING_FAILED, result); } @@ -180,7 +187,7 @@ TEST_P(MerkleIntegritySourceStreamTest, MalformedMIHeader) { } TEST_P(MerkleIntegritySourceStreamTest, WrongMIAttributeName) { - Init("mi-sha256-draft1=bjQLnP-zepicpUTmu3gKLHiQHT-zNzh2hRGjBhevoB0"); + Init("mi-sha256-01=bjQLnP+zepicpUTmu3gKLHiQHT+zNzh2hRGjBhevoB0="); source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); std::string actual_output; int result = ReadStream(&actual_output); @@ -188,7 +195,7 @@ TEST_P(MerkleIntegritySourceStreamTest, WrongMIAttributeName) { } TEST_P(MerkleIntegritySourceStreamTest, HashTooShort) { - Init("mi-sha256-draft2=bjQLnP-zepicpUTmu3gKLHiQHT-zNzh2hRGjBhevoA"); + Init("mi-sha256-03=bjQLnP+zepicpUTmu3gKLHiQHT+zNzh2hRGjBhevoA=="); source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); std::string actual_output; int result = ReadStream(&actual_output); @@ -196,7 +203,7 @@ TEST_P(MerkleIntegritySourceStreamTest, HashTooShort) { } TEST_P(MerkleIntegritySourceStreamTest, HashTooLong) { - Init("mi-sha256-draft2=bjQLnP-zepicpUTmu3gKLHiQHT-zNzh2hRGjBhevoB0A"); + Init("mi-sha256-03=bjQLnP+zepicpUTmu3gKLHiQHT+zNzh2hRGjBhevoB0A"); source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); std::string actual_output; int result = ReadStream(&actual_output); @@ -211,7 +218,7 @@ TEST_P(MerkleIntegritySourceStreamTest, RecordSizeOnly) { source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); std::string actual_output; int result = ReadStream(&actual_output); - EXPECT_EQ(net::OK, result); + EXPECT_EQ(net::ERR_CONTENT_DECODING_FAILED, result); } TEST_P(MerkleIntegritySourceStreamTest, TruncatedRecordSize) { @@ -329,11 +336,11 @@ TEST_P(MerkleIntegritySourceStreamTest, MultipleRecords) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("OElbplJlPK-Rv6JNK6p5_515IaoPoZo-2elWL7OQ60A"); + Base64Decode("OElbplJlPK+Rv6JNK6p5/515IaoPoZo+2elWL7OQ60A="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 16, 16, net::OK, GetParam().mode); std::string hash2 = - Base64Decode("iPMpmgExHPrbEX3_RvwP4d16fWlK4l--p75PUu_KyN0"); + Base64Decode("iPMpmgExHPrbEX3/RvwP4d16fWlK4l++p75PUu/KyN0="); source()->AddReadResult(hash2.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 32, kMessage.size() - 32, net::OK, GetParam().mode); @@ -353,9 +360,9 @@ TEST_P(MerkleIntegritySourceStreamTest, MultipleRecordsAllAtOnce) { std::string body(reinterpret_cast<const char*>(kRecordSize), sizeof(kRecordSize)); body += kMessage.substr(0, 16); - body += Base64Decode("OElbplJlPK-Rv6JNK6p5_515IaoPoZo-2elWL7OQ60A"); + body += Base64Decode("OElbplJlPK+Rv6JNK6p5/515IaoPoZo+2elWL7OQ60A="); body += kMessage.substr(16, 16); - body += Base64Decode("iPMpmgExHPrbEX3_RvwP4d16fWlK4l--p75PUu_KyN0"); + body += Base64Decode("iPMpmgExHPrbEX3/RvwP4d16fWlK4l++p75PUu/KyN0="); body += kMessage.substr(32); source()->AddReadResult(body.data(), body.size(), net::OK, GetParam().mode); @@ -375,11 +382,11 @@ TEST_P(MerkleIntegritySourceStreamTest, MultipleRecordsWrongLastRecordHash) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("OElbplJlPK-Rv6JNK6p5_515IaoPoZo-2elWL7OQ60A"); + Base64Decode("OElbplJlPK+Rv6JNK6p5/515IaoPoZo+2elWL7OQ60A="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 16, 16, net::OK, GetParam().mode); std::string hash2 = - Base64Decode("iPMpmgExHPrbEX3_RvwP4d16fWlK4l--p75PUu_KyN0"); + Base64Decode("iPMpmgExHPrbEX3/RvwP4d16fWlK4l++p75PUu/KyN0="); source()->AddReadResult(hash2.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 32, kMessage.size() - 32, net::OK, GetParam().mode); @@ -400,7 +407,7 @@ TEST_P(MerkleIntegritySourceStreamTest, MultipleRecordsWrongFirstRecordHash) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("OElbplJlPK-Rv6JNK6p5_515IaoPoZo-2elWL7OQ60A"); + Base64Decode("OElbplJlPK+Rv6JNK6p5/515IaoPoZo+2elWL7OQ60A="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); std::string actual_output; @@ -418,11 +425,11 @@ TEST_P(MerkleIntegritySourceStreamTest, TrailingNetError) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("OElbplJlPK-Rv6JNK6p5_515IaoPoZo-2elWL7OQ60A"); + Base64Decode("OElbplJlPK+Rv6JNK6p5/515IaoPoZo+2elWL7OQ60A="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 16, 16, net::OK, GetParam().mode); std::string hash2 = - Base64Decode("iPMpmgExHPrbEX3_RvwP4d16fWlK4l--p75PUu_KyN0"); + Base64Decode("iPMpmgExHPrbEX3/RvwP4d16fWlK4l++p75PUu/KyN0="); source()->AddReadResult(hash2.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 32, kMessage.size() - 32, net::OK, GetParam().mode); @@ -447,11 +454,11 @@ TEST_P(MerkleIntegritySourceStreamTest, Truncated) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("OElbplJlPK-Rv6JNK6p5_515IaoPoZo-2elWL7OQ60A"); + Base64Decode("OElbplJlPK+Rv6JNK6p5/515IaoPoZo+2elWL7OQ60A="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 16, 16, net::OK, GetParam().mode); std::string hash2 = - Base64Decode("iPMpmgExHPrbEX3_RvwP4d16fWlK4l--p75PUu_KyN0"); + Base64Decode("iPMpmgExHPrbEX3/RvwP4d16fWlK4l++p75PUu/KyN0="); source()->AddReadResult(hash2.data(), 32, net::OK, GetParam().mode); // |hash2| is the hash of "atermelon", but this stream ends early. Decoding // thus should fail. @@ -463,14 +470,9 @@ TEST_P(MerkleIntegritySourceStreamTest, Truncated) { EXPECT_EQ(kMessage, actual_output); } -// Test that the final record is allowed to be empty. -// -// TODO(davidben): This does not match the specification and means some inputs -// have two valid encodings. However, the specification's version cannot -// represent the empty string. Update the code and possibly this test depending -// on how https://github.com/martinthomson/http-mice/issues/3 is resolved. +// Test that the final record is not allowed to be empty. TEST_P(MerkleIntegritySourceStreamTest, EmptyFinalRecord) { - Init("mi-sha256-draft2=JJnIuaOEc2247K9V88VQAQy1GJuQ6ylaVM7mG69QkE4"); + Init("mi-sha256-03=JJnIuaOEc2247K9V88VQAQy1GJuQ6ylaVM7mG69QkE4="); const uint8_t kRecordSize[] = {0, 0, 0, 0, 0, 0, 0, 16}; const std::string kMessage( "When I grow up, I want to be a watermelon!! \xf0\x9f\x8d\x89"); @@ -479,21 +481,21 @@ TEST_P(MerkleIntegritySourceStreamTest, EmptyFinalRecord) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("hhJEKpkbuZoWUjzBPAZxMUN2DXdJ6epkS0McZh77IXo"); + Base64Decode("hhJEKpkbuZoWUjzBPAZxMUN2DXdJ6epkS0McZh77IXo="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 16, 16, net::OK, GetParam().mode); std::string hash2 = - Base64Decode("RKTTVSMiH3bkxUQKreVATPL1KUd5eqRdmDgRQcZq_80"); + Base64Decode("RKTTVSMiH3bkxUQKreVATPL1KUd5eqRdmDgRQcZq/80="); source()->AddReadResult(hash2.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 32, 16, net::OK, GetParam().mode); std::string hash3 = - Base64Decode("bjQLnP-zepicpUTmu3gKLHiQHT-zNzh2hRGjBhevoB0"); + Base64Decode("bjQLnP+zepicpUTmu3gKLHiQHT+zNzh2hRGjBhevoB0="); source()->AddReadResult(hash3.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode); std::string actual_output; int rv = ReadStream(&actual_output); - EXPECT_EQ(static_cast<int>(kMessage.size()), rv); + EXPECT_EQ(net::ERR_CONTENT_DECODING_FAILED, rv); EXPECT_EQ(kMessage, actual_output); } @@ -507,11 +509,11 @@ TEST_P(MerkleIntegritySourceStreamTest, WholeNumberOfRecords) { sizeof(kRecordSize), net::OK, GetParam().mode); source()->AddReadResult(kMessage.data(), 16, net::OK, GetParam().mode); std::string hash1 = - Base64Decode("2s-MNG6NrTt556s__HYnQTjG3WOktEcXZ61O8mzG9f4"); + Base64Decode("2s+MNG6NrTt556s//HYnQTjG3WOktEcXZ61O8mzG9f4="); source()->AddReadResult(hash1.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 16, 16, net::OK, GetParam().mode); std::string hash2 = - Base64Decode("qa_cQSMjFyZsm0cnYG4H6LqwOM_hzMSclK6I8iVoZYQ"); + Base64Decode("qa/cQSMjFyZsm0cnYG4H6LqwOM/hzMSclK6I8iVoZYQ="); source()->AddReadResult(hash2.data(), 32, net::OK, GetParam().mode); source()->AddReadResult(kMessage.data() + 32, 16, net::OK, GetParam().mode); @@ -530,9 +532,9 @@ TEST_P(MerkleIntegritySourceStreamTest, WholeNumberOfRecordsAllAtOnce) { std::string body(reinterpret_cast<const char*>(kRecordSize), sizeof(kRecordSize)); body += kMessage.substr(0, 16); - body += Base64Decode("2s-MNG6NrTt556s__HYnQTjG3WOktEcXZ61O8mzG9f4"); + body += Base64Decode("2s+MNG6NrTt556s//HYnQTjG3WOktEcXZ61O8mzG9f4="); body += kMessage.substr(16, 16); - body += Base64Decode("qa_cQSMjFyZsm0cnYG4H6LqwOM_hzMSclK6I8iVoZYQ"); + body += Base64Decode("qa/cQSMjFyZsm0cnYG4H6LqwOM/hzMSclK6I8iVoZYQ="); body += kMessage.substr(32, 16); source()->AddReadResult(body.data(), body.size(), net::OK, GetParam().mode); diff --git a/chromium/content/browser/loader/mime_sniffing_resource_handler.cc b/chromium/content/browser/loader/mime_sniffing_resource_handler.cc index 89e2c5351c4..74707ef3128 100644 --- a/chromium/content/browser/loader/mime_sniffing_resource_handler.cc +++ b/chromium/content/browser/loader/mime_sniffing_resource_handler.cc @@ -250,6 +250,7 @@ void MimeSniffingResourceHandler::OnReadCompleted( // the mime type. However, even if it returns false, it returns a new type // that is probably better than the current one. response_->head.mime_type.assign(new_type); + response_->head.did_mime_sniff = true; if (!made_final_decision && (bytes_read > 0)) { controller->Resume(); diff --git a/chromium/content/browser/loader/mime_sniffing_resource_handler_unittest.cc b/chromium/content/browser/loader/mime_sniffing_resource_handler_unittest.cc index 6ac35383ea9..6d23266622c 100644 --- a/chromium/content/browser/loader/mime_sniffing_resource_handler_unittest.cc +++ b/chromium/content/browser/loader/mime_sniffing_resource_handler_unittest.cc @@ -473,6 +473,8 @@ void MimeSniffingResourceHandlerTest::TestHandlerSniffing( EXPECT_EQ(1, test_handler->on_will_read_called()); EXPECT_EQ(1, test_handler->on_read_completed_called()); + EXPECT_TRUE(test_handler->resource_response()->head.did_mime_sniff); + // Process all messages to ensure proper test teardown. content::RunAllPendingInMessageLoop(); } @@ -601,6 +603,8 @@ void MimeSniffingResourceHandlerTest::TestHandlerNoSniffing( EXPECT_EQ(1, test_handler->on_will_read_called()); EXPECT_EQ(1, test_handler->on_read_completed_called()); + EXPECT_FALSE(test_handler->resource_response()->head.did_mime_sniff); + if (!read_completed) { EXPECT_EQ(MockResourceLoader::Status::CANCELED, mock_loader.status()); EXPECT_EQ(net::ERR_ABORTED, mock_loader.error_code()); diff --git a/chromium/content/browser/loader/mojo_async_resource_handler.cc b/chromium/content/browser/loader/mojo_async_resource_handler.cc index 4910201a2d1..de7d58c0a73 100644 --- a/chromium/content/browser/loader/mojo_async_resource_handler.cc +++ b/chromium/content/browser/loader/mojo_async_resource_handler.cc @@ -13,6 +13,7 @@ #include "base/logging.h" #include "base/macros.h" #include "base/time/time.h" +#include "base/timer/timer.h" #include "content/browser/loader/resource_controller.h" #include "content/browser/loader/resource_dispatcher_host_impl.h" #include "content/browser/loader/resource_request_info_impl.h" @@ -115,6 +116,7 @@ MojoAsyncResourceHandler::MojoAsyncResourceHandler( mojo::SimpleWatcher::ArmingPolicy::MANUAL, base::SequencedTaskRunnerHandle::Get()), url_loader_client_(std::move(url_loader_client)), + report_transfer_size_async_timer_(std::make_unique<base::OneShotTimer>()), weak_factory_(this) { DCHECK(IsResourceTypeFrame(resource_type) || resource_type == RESOURCE_TYPE_SERVICE_WORKER || @@ -304,6 +306,37 @@ void MojoAsyncResourceHandler::OnWillRead( controller->Resume(); } +void MojoAsyncResourceHandler::set_report_transfer_size_async_timer_for_testing( + std::unique_ptr<base::OneShotTimer> timer) { + report_transfer_size_async_timer_ = std::move(timer); +} + +void MojoAsyncResourceHandler::SendTransferSizeUpdate() { + int64_t transfer_size_diff = CalculateRecentlyReceivedBytes(); + if (transfer_size_diff > 0) { + url_loader_client_->OnTransferSizeUpdated(transfer_size_diff); + } +} + +void MojoAsyncResourceHandler::EnsureTransferSizeUpdate() { + auto current_time = base::TimeTicks::Now(); + if (earliest_time_next_transfer_size_report_.is_null() || + earliest_time_next_transfer_size_report_ <= current_time) { + report_transfer_size_async_timer_->Stop(); + SendTransferSizeUpdate(); + earliest_time_next_transfer_size_report_ = + current_time + kTransferSizeReportInterval; + } else { + // Ensure that a single transfer update will eventually occur even if reads + // stop. Unretained is safe here because the callback will only live as long + // as |report_transfer_size_async_timer_|. + report_transfer_size_async_timer_->Start( + FROM_HERE, kTransferSizeReportInterval, + base::BindOnce(&MojoAsyncResourceHandler::SendTransferSizeUpdate, + base::Unretained(this))); + } +} + void MojoAsyncResourceHandler::OnReadCompleted( int bytes_read, std::unique_ptr<ResourceController> controller) { @@ -318,15 +351,8 @@ void MojoAsyncResourceHandler::OnReadCompleted( return; } - if (ShouldReportTransferSize(GetRequestInfo()) && - (time_transfer_size_next_report_.is_null() || - time_transfer_size_next_report_ <= base::TimeTicks::Now())) { - auto transfer_size_diff = CalculateRecentlyReceivedBytes(); - if (transfer_size_diff > 0) { - url_loader_client_->OnTransferSizeUpdated(transfer_size_diff); - time_transfer_size_next_report_ = - base::TimeTicks::Now() + kTransferSizeReportInterval; - } + if (ShouldReportTransferSize(GetRequestInfo())) { + EnsureTransferSizeUpdate(); } if (response_body_consumer_handle_.is_valid()) { @@ -507,9 +533,9 @@ void MojoAsyncResourceHandler::OnResponseCompleted( } if (ShouldReportTransferSize(GetRequestInfo())) { - auto transfer_size_diff = CalculateRecentlyReceivedBytes(); - if (transfer_size_diff > 0) - url_loader_client_->OnTransferSizeUpdated(transfer_size_diff); + // All received bytes will be reported. + report_transfer_size_async_timer_->Stop(); + SendTransferSizeUpdate(); } url_loader_client_->OnComplete(loader_status); diff --git a/chromium/content/browser/loader/mojo_async_resource_handler.h b/chromium/content/browser/loader/mojo_async_resource_handler.h index ad87065771e..73ae262665c 100644 --- a/chromium/content/browser/loader/mojo_async_resource_handler.h +++ b/chromium/content/browser/loader/mojo_async_resource_handler.h @@ -28,6 +28,7 @@ class GURL; namespace base { class Location; +class OneShotTimer; } namespace net { @@ -93,6 +94,8 @@ class CONTENT_EXPORT MojoAsyncResourceHandler void PauseReadingBodyFromNet() override; void ResumeReadingBodyFromNet() override; + void set_report_transfer_size_async_timer_for_testing( + std::unique_ptr<base::OneShotTimer> timer); void OnWritableForTesting(); static void SetAllocationSizeForTesting(size_t size); static constexpr size_t kDefaultAllocationSize = 512 * 1024; @@ -123,6 +126,8 @@ class CONTENT_EXPORT MojoAsyncResourceHandler // |reported_total_received_bytes_|, returns it, and updates // |reported_total_received_bytes_|. int64_t CalculateRecentlyReceivedBytes(); + void SendTransferSizeUpdate(); + void EnsureTransferSizeUpdate(); // These functions can be overriden only for tests. virtual void ReportBadMessage(const std::string& error); @@ -150,9 +155,6 @@ class CONTENT_EXPORT MojoAsyncResourceHandler bool did_defer_on_redirect_ = false; bool did_defer_on_response_started_ = false; - // The time transfer size should be reported next. - base::TimeTicks time_transfer_size_next_report_; - int64_t reported_total_received_bytes_ = 0; int64_t total_written_bytes_ = 0; // Pointer to parent's information about the read buffer. Only non-null while @@ -171,6 +173,14 @@ class CONTENT_EXPORT MojoAsyncResourceHandler std::unique_ptr<network::UploadProgressTracker> upload_progress_tracker_; + // Timer to report transfer size after a read is completed but not reported. + // Gurantees that all received bytes will be reported eventually, regardless + // of read rate or completion, as long as the client is alive. + std::unique_ptr<base::OneShotTimer> report_transfer_size_async_timer_; + // The time transfer size should be reported next. + base::TimeTicks earliest_time_next_transfer_size_report_; + int64_t reported_total_received_bytes_ = 0; + base::WeakPtrFactory<MojoAsyncResourceHandler> weak_factory_; DISALLOW_COPY_AND_ASSIGN(MojoAsyncResourceHandler); }; diff --git a/chromium/content/browser/loader/mojo_async_resource_handler_unittest.cc b/chromium/content/browser/loader/mojo_async_resource_handler_unittest.cc index a4170df9f58..68e6a784850 100644 --- a/chromium/content/browser/loader/mojo_async_resource_handler_unittest.cc +++ b/chromium/content/browser/loader/mojo_async_resource_handler_unittest.cc @@ -19,6 +19,7 @@ #include "base/run_loop.h" #include "base/test/gtest_util.h" #include "base/test/test_simple_task_runner.h" +#include "base/timer/mock_timer.h" #include "content/browser/loader/mock_resource_loader.h" #include "content/browser/loader/resource_controller.h" #include "content/browser/loader/resource_dispatcher_host_impl.h" @@ -1523,6 +1524,62 @@ TEST_F(MojoAsyncResourceHandlerTest, EXPECT_LT(0, request_->GetTotalReceivedBytes()); } +TEST_F(MojoAsyncResourceHandlerTest, + TransferSizeUpdateCalledWithoutResponseComplete) { + const char kResponseHeaders[] = "response headers"; + const char kResponseData[] = "response data"; + // Create a mock timer to control when the final transfersizeupdate is sent. + auto timer = std::make_unique<base::MockOneShotTimer>(); + auto* raw_timer = timer.get(); + handler_->set_report_transfer_size_async_timer_for_testing(std::move(timer)); + + // Create a test job so the underlying URLRequest will receive bytes. + net::URLRequestJobFactoryImpl test_job_factory_; + auto test_job = std::make_unique<net::URLRequestTestJob>( + request_.get(), request_context_->network_delegate(), kResponseHeaders, + kResponseData, true); + auto test_job_interceptor = std::make_unique<net::TestJobInterceptor>(); + net::TestJobInterceptor* raw_test_job_interceptor = + test_job_interceptor.get(); + EXPECT_TRUE(test_job_factory_.SetProtocolHandler( + url::kHttpScheme, std::move(test_job_interceptor))); + request_context_->set_job_factory(&test_job_factory_); + raw_test_job_interceptor->set_main_intercept_job(std::move(test_job)); + request_->Start(); + + // Prepare for loader read complete. + ASSERT_TRUE(CallOnWillStartAndOnResponseStarted()); + EXPECT_EQ(MockResourceLoader::Status::IDLE, + mock_loader_->OnWillStart(request_->url())); + EXPECT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead()); + // Only headers are read by the time the response is started. + mock_loader_->OnReadCompleted(kResponseHeaders); + + // Make the loader process another read of the rest of the URLTestJob data. + EXPECT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead()); + mock_loader_->OnReadCompleted(kResponseData); + + // Process the entire URL request. + url_request_delegate_.RunUntilComplete(); + + // All data received by the request. + EXPECT_EQ( + request_->GetTotalReceivedBytes(), + static_cast<int64_t>(strlen(kResponseHeaders) + strlen(kResponseData))); + + // Wait for a transfer size update to be received. + url_loader_client_.RunUntilTransferSizeUpdated(); + // Only the first read caused a transfer size update. + EXPECT_EQ(static_cast<int64_t>(strlen(kResponseHeaders)), + url_loader_client_.body_transfer_size()); + // Firing the timer will cause the rest of the bytes to be reported. + // Without timer fire no transfer size updates would be received. + raw_timer->Fire(); + url_loader_client_.RunUntilTransferSizeUpdated(); + EXPECT_EQ(request_->GetTotalReceivedBytes(), + url_loader_client_.body_transfer_size()); +} + INSTANTIATE_TEST_CASE_P(MojoAsyncResourceHandlerWithAllocationSizeTest, MojoAsyncResourceHandlerWithAllocationSizeTest, ::testing::Values(8, 32 * 2014)); diff --git a/chromium/content/browser/loader/navigation_loader_interceptor.h b/chromium/content/browser/loader/navigation_loader_interceptor.h index 2f4ad19e0ba..ca2f3ad6d5f 100644 --- a/chromium/content/browser/loader/navigation_loader_interceptor.h +++ b/chromium/content/browser/loader/navigation_loader_interceptor.h @@ -35,23 +35,49 @@ class CONTENT_EXPORT NavigationLoaderInterceptor { using LoaderCallback = base::OnceCallback<void(SingleRequestURLLoaderFactory::RequestHandler)>; + using FallbackCallback = + base::OnceCallback<void(bool /* reset_subresource_loader_params */)>; // Asks this handler to handle this resource load request. // The handler must invoke |callback| eventually with either a non-null // RequestHandler indicating its willingness to handle the request, or a null // RequestHandler to indicate that someone else should handle the request. + // + // The |tentative_resource_request| passed to this function and the resource + // request later passed to the RequestHandler given to |callback| may not be + // exactly the same, because URLLoaderThrottles may rewrite the request + // between the two calls. However the URL must remain constant between the + // two, as any modifications on the URL done by URLLoaderThrottles must result + // in an (internal) redirect, which must restart the request with a new + // MaybeCreateLoader(). + // + // This handler might initially elect to handle the request, but later decide + // to fall back to the default behavior. In that case, it can invoke + // |fallback_callback| to do so. An example of this is when a service worker + // decides to handle the request because it is in-scope, but the service + // worker JavaScript execution does not result in a response provided, so + // fallback to network is required. + // + // If |fallback_callback| is called, it must be called prior to the + // RequestHandler making any URLLoaderClient calls. The + // |reset_subresource_loader_params| parameter to |fallback_callback| + // indicates whether to discard the subresource loader params previously + // returned by MaybeCreateSubresourceLoaderParams(). virtual void MaybeCreateLoader( - const network::ResourceRequest& resource_request, + const network::ResourceRequest& tentative_resource_request, ResourceContext* resource_context, - LoaderCallback callback) = 0; + LoaderCallback callback, + FallbackCallback fallback_callback) = 0; // Returns a SubresourceLoaderParams if any to be used for subsequent URL // requests going forward. Subclasses who want to set-up custom loader for // subresource requests may want to override this. + // // Note that the handler can return a null callback to MaybeCreateLoader(), // and at the same time can return non-null SubresourceLoaderParams here if it // does NOT want to handle the specific request given to MaybeCreateLoader() - // but wants to handle the subsequent resource requests. + // but wants to handle the subsequent resource requests or ensure other + // interceptors are skipped. virtual base::Optional<SubresourceLoaderParams> MaybeCreateSubresourceLoaderParams(); diff --git a/chromium/content/browser/loader/navigation_url_loader_delegate.h b/chromium/content/browser/loader/navigation_url_loader_delegate.h index c7d238283e1..c1f56144194 100644 --- a/chromium/content/browser/loader/navigation_url_loader_delegate.h +++ b/chromium/content/browser/loader/navigation_url_loader_delegate.h @@ -11,6 +11,7 @@ #include "base/memory/ref_counted.h" #include "base/optional.h" #include "content/common/content_export.h" +#include "content/public/common/previews_state.h" #include "services/network/public/mojom/url_loader.mojom.h" namespace net { @@ -45,6 +46,7 @@ class CONTENT_EXPORT NavigationURLLoaderDelegate { // necessary info to create a custom subresource loader in the renderer // process if the navigated context is controlled by a request interceptor // like AppCache or ServiceWorker. + // |previews_state| is updated in common params. virtual void OnResponseStarted( const scoped_refptr<network::ResourceResponse>& response, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, @@ -52,6 +54,7 @@ class CONTENT_EXPORT NavigationURLLoaderDelegate { const GlobalRequestID& request_id, bool is_download, bool is_stream, + PreviewsState previews_state, base::Optional<SubresourceLoaderParams> subresource_loader_params) = 0; // Called if the request fails before receving a response. Specific diff --git a/chromium/content/browser/loader/navigation_url_loader_impl.cc b/chromium/content/browser/loader/navigation_url_loader_impl.cc index 0fb15c7c1be..f14b46b86a8 100644 --- a/chromium/content/browser/loader/navigation_url_loader_impl.cc +++ b/chromium/content/browser/loader/navigation_url_loader_impl.cc @@ -10,11 +10,13 @@ #include "base/bind_helpers.h" #include "base/feature_list.h" #include "base/metrics/histogram_macros.h" +#include "base/optional.h" #include "base/stl_util.h" -#include "base/task_scheduler/post_task.h" +#include "base/task/post_task.h" #include "base/trace_event/trace_event.h" #include "components/download/public/common/download_stats.h" #include "content/browser/appcache/appcache_navigation_handle.h" +#include "content/browser/appcache/appcache_navigation_handle_core.h" #include "content/browser/appcache/appcache_request_handler.h" #include "content/browser/blob_storage/chrome_blob_storage_context.h" #include "content/browser/devtools/render_frame_devtools_agent_host.h" @@ -39,6 +41,7 @@ #include "content/browser/web_package/signed_exchange_utils.h" #include "content/browser/webui/url_data_manager_backend.h" #include "content/browser/webui/web_ui_url_loader_factory_internal.h" +#include "content/common/mime_sniffing_throttle.h" #include "content/common/navigation_subresource_loader_params.h" #include "content/common/net/record_load_histograms.h" #include "content/common/throttling_url_loader.h" @@ -59,8 +62,11 @@ #include "content/public/common/url_utils.h" #include "content/public/common/webplugininfo.h" #include "net/base/load_flags.h" +#include "net/cert/sct_status_flags.h" +#include "net/cert/signed_certificate_timestamp_and_status.h" #include "net/http/http_content_disposition.h" #include "net/http/http_request_headers.h" +#include "net/ssl/ssl_info.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/redirect_util.h" #include "net/url_request/url_request.h" @@ -90,11 +96,13 @@ class NavigationLoaderInterceptorBrowserContainer ~NavigationLoaderInterceptorBrowserContainer() override = default; - void MaybeCreateLoader(const network::ResourceRequest& resource_request, - ResourceContext* resource_context, - LoaderCallback callback) override { - browser_interceptor_->MaybeCreateLoader(resource_request, resource_context, - std::move(callback)); + void MaybeCreateLoader( + const network::ResourceRequest& tentative_resource_request, + ResourceContext* resource_context, + LoaderCallback callback, + FallbackCallback fallback_callback) override { + browser_interceptor_->MaybeCreateLoader( + tentative_resource_request, resource_context, std::move(callback)); } private: @@ -290,13 +298,62 @@ std::unique_ptr<NavigationRequestInfo> CreateNavigationRequestInfoForRedirect( } // Called for requests that we don't have a URLLoaderFactory for. -void UnknownSchemeCallback(bool handled_externally, - network::mojom::URLLoaderRequest request, - network::mojom::URLLoaderClientPtr client) { +void UnknownSchemeCallback( + bool handled_externally, + const network::ResourceRequest& /* resource_request */, + network::mojom::URLLoaderRequest request, + network::mojom::URLLoaderClientPtr client) { client->OnComplete(network::URLLoaderCompletionStatus( handled_externally ? net::ERR_ABORTED : net::ERR_UNKNOWN_URL_SCHEME)); } +// Returns whether this URL can be handled by the default network service +// URLLoader. +bool IsURLHandledByDefaultLoader(const GURL& url) { + // Data URLs are only handled by the network service if + // |enable_data_url_support| is set in NetworkContextParams. This is set to + // true for the context used by NavigationURLLoaderImpl, so in addition to + // checking whether the URL is handled by the network service, we also need to + // check for the data scheme. + return IsURLHandledByNetworkService(url) || url.SchemeIs(url::kDataScheme); +} + +// Determines whether it is safe to redirect from |from_url| to |to_url|. +bool IsRedirectSafe(const GURL& from_url, + const GURL& to_url, + ResourceContext* resource_context) { + return IsSafeRedirectTarget(from_url, to_url) && + GetContentClient()->browser()->IsSafeRedirectTarget(to_url, + resource_context); +} + +// URLLoaderFactory for handling about: URLs. This treats everything as +// about:blank since no other about: features should be available to web +// content. +class AboutURLLoaderFactory : public network::mojom::URLLoaderFactory { + private: + // network::mojom::URLLoaderFactory: + void CreateLoaderAndStart(network::mojom::URLLoaderRequest loader, + int32_t routing_id, + int32_t request_id, + uint32_t options, + const network::ResourceRequest& request, + network::mojom::URLLoaderClientPtr client, + const net::MutableNetworkTrafficAnnotationTag& + traffic_annotation) override { + network::ResourceResponseHead response; + response.mime_type = "text/html"; + client->OnReceiveResponse(response); + client->OnComplete(network::URLLoaderCompletionStatus(net::OK)); + } + + void Clone(network::mojom::URLLoaderFactoryRequest loader) override { + bindings_.AddBinding(this, std::move(loader)); + } + + mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_; +}; + } // namespace // Kept around during the lifetime of the navigation request, and is @@ -315,19 +372,23 @@ class NavigationURLLoaderImpl::URLLoaderRequestController std::unique_ptr<network::ResourceRequest> resource_request, ResourceContext* resource_context, const GURL& url, + bool is_main_frame, network::mojom::URLLoaderFactoryRequest proxied_factory_request, network::mojom::URLLoaderFactoryPtrInfo proxied_factory_info, std::set<std::string> known_schemes, + bool bypass_redirect_checks, const base::WeakPtr<NavigationURLLoaderImpl>& owner) : interceptors_(std::move(initial_interceptors)), resource_request_(std::move(resource_request)), resource_context_(resource_context), url_(url), + is_main_frame_(is_main_frame), owner_(owner), response_loader_binding_(this), proxied_factory_request_(std::move(proxied_factory_request)), proxied_factory_info_(std::move(proxied_factory_info)), known_schemes_(std::move(known_schemes)), + bypass_redirect_checks_(bypass_redirect_checks), weak_factory_(this) {} ~URLLoaderRequestController() override { @@ -347,13 +408,19 @@ class NavigationURLLoaderImpl::URLLoaderRequestController } static uint32_t GetURLLoaderOptions(bool is_main_frame) { - uint32_t options = network::mojom::kURLLoadOptionSendSSLInfoWithResponse; - if (is_main_frame) + uint32_t options = network::mojom::kURLLoadOptionNone; + + // Ensure that Mime sniffing works. + options |= network::mojom::kURLLoadOptionSniffMimeType; + + if (is_main_frame) { + // SSLInfo is not needed on subframe responses because users can inspect + // only the certificate for the main frame when using the info bubble. + options |= network::mojom::kURLLoadOptionSendSSLInfoWithResponse; options |= network::mojom::kURLLoadOptionSendSSLInfoForCertificateError; + } - if (base::FeatureList::IsEnabled(network::features::kNetworkService)) { - options |= network::mojom::kURLLoadOptionSniffMimeType; - } else { + if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) { // TODO(arthursonzogni): This is a temporary option. Remove this as soon // as the InterceptingResourceHandler is removed. // See https://crbug.com/791049. @@ -400,14 +467,22 @@ class NavigationURLLoaderImpl::URLLoaderRequestController std::unique_ptr<NavigationRequestInfo> request_info, ServiceWorkerNavigationHandleCore* service_worker_navigation_handle_core, AppCacheNavigationHandleCore* appcache_handle_core, + const network::ResourceRequest& /* resource_request */, network::mojom::URLLoaderRequest url_loader, network::mojom::URLLoaderClientPtr url_loader_client) { + // |resource_request| is unused here. Its info may not be the same as + // |request_info|, because URLLoaderThrottles may have rewritten it. We + // don't propagate the fields to |request_info| here because the request + // will usually go to ResourceDispatcherHost which does its own request + // modification independent of URLLoaderThrottles. DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService)); DCHECK(started_); default_loader_used_ = true; if (signed_exchange_utils::IsSignedExchangeHandlingEnabled()) { + // TODO(falken): Understand and add a comment about why + // SignedExchangeRequestHandler is the only interceptor being added here. DCHECK(!network_loader_factory_); // It is safe to pass the callback of CreateURLLoaderThrottles with the // unretained |this|, because the passed callback will be used by a @@ -426,8 +501,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController resource_context_, url_request_context_getter), base::BindRepeating( &URLLoaderRequestController::CreateURLLoaderThrottles, - base::Unretained(this)), - url_request_context_getter)); + base::Unretained(this)))); } uint32_t options = GetURLLoaderOptions(request_info->is_main_frame); @@ -549,7 +623,6 @@ class NavigationURLLoaderImpl::URLLoaderRequestController } void Start( - net::URLRequestContextGetter* url_request_context_getter, std::unique_ptr<network::SharedURLLoaderFactoryInfo> network_loader_factory_info, ServiceWorkerNavigationHandleCore* service_worker_navigation_handle_core, @@ -615,8 +688,8 @@ class NavigationURLLoaderImpl::URLLoaderRequestController if (appcache_handle_core) { std::unique_ptr<NavigationLoaderInterceptor> appcache_interceptor = - AppCacheRequestHandler::InitializeForNavigationNetworkService( - *resource_request_, appcache_handle_core, + AppCacheRequestHandler::InitializeForMainResourceNetworkService( + *resource_request_, appcache_handle_core->host()->GetWeakPtr(), network_loader_factory_); if (appcache_interceptor) interceptors_.push_back(std::move(appcache_interceptor)); @@ -642,8 +715,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController request_info->begin_params->load_flags, network_loader_factory_, base::BindRepeating( &URLLoaderRequestController::CreateURLLoaderThrottles, - base::Unretained(this)), - url_request_context_getter)); + base::Unretained(this)))); } std::vector<std::unique_ptr<URLLoaderRequestInterceptor>> @@ -670,9 +742,15 @@ class NavigationURLLoaderImpl::URLLoaderRequestController // the restarted request to use a new loader, instead of, e.g., reusing the // AppCache or service worker loader. For an optimization, we keep and reuse // the default url loader if the all |interceptors_| doesn't handle the - // redirected request. - if (!default_loader_used_) + // redirected request. If the network service is enabled, only certain + // schemes are handled by the default URL loader. We need to make sure the + // redirected URL is a handled scheme, otherwise reset the loader so the + // correct non-network service loader can be used. + if (!default_loader_used_ || + (base::FeatureList::IsEnabled(network::features::kNetworkService) && + !IsURLHandledByDefaultLoader(resource_request_->url))) { url_loader_.reset(); + } interceptor_index_ = 0; received_response_ = false; MaybeStartLoader(nullptr /* interceptor */, @@ -694,11 +772,19 @@ class NavigationURLLoaderImpl::URLLoaderRequestController // |interceptor| wants to handle the request with // |single_request_handler|. DCHECK(interceptor); + + std::vector<std::unique_ptr<URLLoaderThrottle>> throttles = + CreateURLLoaderThrottles(); + // Intercepted requests need MimeSniffingThrottle to do mime sniffing. + // Non-intercepted requests usually go through the regular network + // URLLoader, which does mime sniffing. + throttles.push_back(std::make_unique<MimeSniffingThrottle>()); + default_loader_used_ = false; url_loader_ = ThrottlingURLLoader::CreateLoaderAndStart( base::MakeRefCounted<SingleRequestURLLoaderFactory>( std::move(single_request_handler)), - CreateURLLoaderThrottles(), frame_tree_node_id_, + std::move(throttles), frame_tree_node_id_, global_request_id_.request_id, network::mojom::kURLLoadOptionNone, resource_request_.get(), this, kNavigationUrlLoaderTrafficAnnotation, base::ThreadTaskRunnerHandle::Get()); @@ -714,9 +800,9 @@ class NavigationURLLoaderImpl::URLLoaderRequestController // that case we will just fall back to the default loader (i.e. won't go on // to the next interceptors) but send the subresource_loader_params to the // child process. This is necessary for correctness in the cases where, e.g. - // there's a controlling ServiceWorker that doesn't handle main resource - // loading, but may still want to control the page and/or handle subresource - // loading. In that case we want to skip AppCache. + // there's a controlling service worker that doesn't have a fetch event + // handler so it doesn't intercept requests. In that case we still want to + // skip AppCache. if (interceptor) { subresource_loader_params_ = interceptor->MaybeCreateSubresourceLoaderParams(); @@ -733,13 +819,16 @@ class NavigationURLLoaderImpl::URLLoaderRequestController next_interceptor->MaybeCreateLoader( *resource_request_, resource_context_, base::BindOnce(&URLLoaderRequestController::MaybeStartLoader, - base::Unretained(this), next_interceptor)); + base::Unretained(this), next_interceptor), + base::BindOnce( + &URLLoaderRequestController::FallbackToNonInterceptedRequest, + base::Unretained(this))); return; } - // If we already have the default |url_loader_| we must come here after - // a redirect. No interceptors wanted to intercept the redirected request, - // so let it just follow the redirect. + // If we already have the default |url_loader_| we must come here after a + // redirect. No interceptors wanted to intercept the redirected request, so + // let the loader just follow the redirect. if (url_loader_) { DCHECK(!redirect_info_.new_url.is_empty()); url_loader_->FollowRedirect( @@ -747,48 +836,87 @@ class NavigationURLLoaderImpl::URLLoaderRequestController return; } - // TODO(https://crbug.com/796425): We temporarily wrap raw - // mojom::URLLoaderFactory pointers into SharedURLLoaderFactory. Need to - // further refactor the factory getters to avoid this. - scoped_refptr<network::SharedURLLoaderFactory> factory; - DCHECK_EQ(interceptors_.size(), interceptor_index_); + // No interceptors wanted to handle this request. + uint32_t options = network::mojom::kURLLoadOptionNone; + scoped_refptr<network::SharedURLLoaderFactory> factory = + PrepareForNonInterceptedRequest(&options); + url_loader_ = ThrottlingURLLoader::CreateLoaderAndStart( + std::move(factory), CreateURLLoaderThrottles(), frame_tree_node_id_, + global_request_id_.request_id, options, resource_request_.get(), + this /* client */, kNavigationUrlLoaderTrafficAnnotation, + base::ThreadTaskRunnerHandle::Get()); + } + // This is the |fallback_callback| passed to + // NavigationLoaderInterceptor::MaybeCreateLoader. It allows an interceptor + // to initially elect to handle a request, and later decide to fallback to + // the default behavior. This is needed for service worker network fallback. + void FallbackToNonInterceptedRequest(bool reset_subresource_loader_params) { + if (reset_subresource_loader_params) + subresource_loader_params_.reset(); + + // Non-NetworkService: + // Cancel state on ResourceDispatcherHostImpl so it doesn't complain about + // reusing the request_id after redirects. Otherwise the following sequence + // can happen: + // RDHI Start(request_id) -> Redirect -> SW interception -> SW fallback to + // network -> RDHI Start(request_id). + if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) { + DCHECK(ResourceDispatcherHostImpl::Get()); + ResourceDispatcherHostImpl::Get()->CancelRequest( + global_request_id_.child_id, global_request_id_.request_id); + } + + // |url_loader_| is using the factory for the interceptor that decided to + // fallback, so restart it with the non-interceptor factory. + DCHECK(url_loader_); + uint32_t options = network::mojom::kURLLoadOptionNone; + scoped_refptr<network::SharedURLLoaderFactory> factory = + PrepareForNonInterceptedRequest(&options); + url_loader_->RestartWithFactory(std::move(factory), options); + } + + scoped_refptr<network::SharedURLLoaderFactory> + PrepareForNonInterceptedRequest(uint32_t* out_options) { // If NetworkService is not enabled (which means we come here because one of // the loader interceptors is enabled), use the default request handler // instead of going through the NetworkService path. if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) { DCHECK(!interceptors_.empty()); DCHECK(default_request_handler_factory_); - // The only way to come here is to enable ServiceWorkerServicification - // without NetworkService. We know that the service worker's request - // interceptor has already intercepted and decided not to handle the + // The only way to come here is to enable ServiceWorkerServicification or + // SignedExchange without NetworkService. We know that their request + // interceptors have already intercepted and decided not to handle the // request. - DCHECK(blink::ServiceWorkerUtils::IsServicificationEnabled()); + DCHECK(blink::ServiceWorkerUtils::IsServicificationEnabled() || + signed_exchange_utils::IsSignedExchangeHandlingEnabled()); default_loader_used_ = true; // Update |request_info_| when following a redirect. if (url_chain_.size() > 0) { request_info_ = CreateNavigationRequestInfoForRedirect( *request_info_, *resource_request_); } + // When |subresource_loader_params_| has its value, the request should not // be intercepted by any other interceptors since it means that a request // interceptor already intercepted the request and it attached its info to // the request. - url_loader_ = ThrottlingURLLoader::CreateLoaderAndStart( - base::MakeRefCounted<SingleRequestURLLoaderFactory>( - default_request_handler_factory_.Run( - subresource_loader_params_.has_value() - /* was_request_intercepted */)), - CreateURLLoaderThrottles(), frame_tree_node_id_, - global_request_id_.request_id, network::mojom::kURLLoadOptionNone, - resource_request_.get(), this /* client */, - kNavigationUrlLoaderTrafficAnnotation, - base::ThreadTaskRunnerHandle::Get()); - return; + bool was_request_intercepted = subresource_loader_params_.has_value(); + + // TODO(falken): Determine whether GetURLLoaderOptions() can be called + // here like below. It looks like |default_request_handler_factory_| just + // calls that. + *out_options = network::mojom::kURLLoadOptionNone; + return base::MakeRefCounted<SingleRequestURLLoaderFactory>( + default_request_handler_factory_.Run(was_request_intercepted)); } - if (!IsURLHandledByNetworkService(resource_request_->url) && - !resource_request_->url.SchemeIs(url::kDataScheme)) { + // TODO(https://crbug.com/796425): We temporarily wrap raw + // mojom::URLLoaderFactory pointers into SharedURLLoaderFactory. Need to + // further refactor the factory getters to avoid this. + scoped_refptr<network::SharedURLLoaderFactory> factory; + + if (!IsURLHandledByDefaultLoader(resource_request_->url)) { if (known_schemes_.find(resource_request_->url.scheme()) == known_schemes_.end()) { bool handled = GetContentClient()->browser()->HandleExternalProtocol( @@ -836,13 +964,9 @@ class NavigationURLLoaderImpl::URLLoaderRequestController } } url_chain_.push_back(resource_request_->url); - uint32_t options = GetURLLoaderOptions(resource_request_->resource_type == - RESOURCE_TYPE_MAIN_FRAME); - url_loader_ = ThrottlingURLLoader::CreateLoaderAndStart( - factory, CreateURLLoaderThrottles(), frame_tree_node_id_, - global_request_id_.request_id, options, resource_request_.get(), this, - kNavigationUrlLoaderTrafficAnnotation, - base::ThreadTaskRunnerHandle::Get()); + *out_options = GetURLLoaderOptions(resource_request_->resource_type == + RESOURCE_TYPE_MAIN_FRAME); + return factory; } void FollowRedirect( @@ -897,6 +1021,11 @@ class NavigationURLLoaderImpl::URLLoaderRequestController private: // network::mojom::URLLoaderClient implementation: void OnReceiveResponse(const network::ResourceResponseHead& head) override { + // Record the SCT histogram before checking if anything wants to intercept + // the response, so interceptors like AppCache and extensions can't hide + // values from the histograms. + RecordSCTHistogramIfNeeded(head.ssl_info); + received_response_ = true; // If the default loader (network) was used to handle the URL load request @@ -922,86 +1051,105 @@ class NavigationURLLoaderImpl::URLLoaderRequestController bool is_download; bool is_stream; + // If there is not an explicit PreviewsState set on the request, turn + // Previews off. + PreviewsState previews_state = PREVIEWS_OFF; std::unique_ptr<NavigationData> cloned_navigation_data; + if (IsLoaderInterceptionEnabled()) { bool must_download = download_utils::MustDownload( url_, head.headers.get(), head.mime_type); bool known_mime_type = blink::IsSupportedMimeType(head.mime_type); + bool is_download_if_not_handled_by_plugin = + !response_intercepted && (must_download || !known_mime_type); + #if BUILDFLAG(ENABLE_PLUGINS) if (!response_intercepted && !must_download && !known_mime_type) { CheckPluginAndContinueOnReceiveResponse( head, std::move(url_loader_client_endpoints), - std::vector<WebPluginInfo>()); + is_download_if_not_handled_by_plugin, std::vector<WebPluginInfo>()); return; } #endif - is_download = - !response_intercepted && (must_download || !known_mime_type); + is_download = is_download_if_not_handled_by_plugin; is_stream = false; - } else { - ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get(); - net::URLRequest* url_request = rdh->GetURLRequest(global_request_id_); - - // The |url_request| maybe have been removed from the resource dispatcher - // host during the time it took for OnReceiveResponse() to be received. - if (url_request) { - ResourceRequestInfoImpl* info = - ResourceRequestInfoImpl::ForRequest(url_request); - is_download = !response_intercepted && info->IsDownload(); - is_stream = info->is_stream(); - if (rdh->delegate()) { - NavigationData* navigation_data = - rdh->delegate()->GetNavigationData(url_request); - - // Clone the embedder's NavigationData before moving it to the UI - // thread. - if (navigation_data) - cloned_navigation_data = navigation_data->Clone(); - } - // This is similar to what is done in - // ServiceWorkerControlleeHandler::MaybeCreateSubresourceLoaderParams(). - // It takes the matching ControllerServiceWorkerInfo (if any) associated - // with the request. It will be sent to the renderer process and used to - // intercept requests. - // TODO(arthursonzogni): This is needed only for the - // non-S13nServiceWorker case. The S13nServiceWorker case is still not - // supported without the NetworkService. This block needs to be updated - // once support for it will be added. - ServiceWorkerProviderHost* sw_provider_host = - ServiceWorkerRequestHandler::GetProviderHost(url_request); - if (sw_provider_host && sw_provider_host->controller()) { - subresource_loader_params_ = SubresourceLoaderParams(); - subresource_loader_params_->controller_service_worker_info = - mojom::ControllerServiceWorkerInfo::New(); - subresource_loader_params_->controller_service_worker_info->mode = - sw_provider_host->GetControllerMode(); - base::WeakPtr<ServiceWorkerObjectHost> sw_object_host = - sw_provider_host->GetOrCreateServiceWorkerObjectHost( - sw_provider_host->controller()); - if (sw_object_host) { - subresource_loader_params_->controller_service_worker_object_host = - sw_object_host; - subresource_loader_params_->controller_service_worker_info - ->object_info = sw_object_host->CreateIncompleteObjectInfo(); - } + // If NetworkService is on, or an interceptor handled the request, the + // request doesn't use ResourceDispatcherHost so + // CallOnReceivedResponse and return here. + if (base::FeatureList::IsEnabled(network::features::kNetworkService) || + !default_loader_used_) { + CallOnReceivedResponse(head, std::move(url_loader_client_endpoints), + std::move(cloned_navigation_data), is_download, + is_stream, previews_state); + return; + } + } + + // NetworkService is off and an interceptor didn't handle the request, + // so it went to ResourceDispatcherHost. + ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get(); + net::URLRequest* url_request = rdh->GetURLRequest(global_request_id_); + + // The |url_request| maybe have been removed from the resource dispatcher + // host during the time it took for OnReceiveResponse() to be received. + if (url_request) { + ResourceRequestInfoImpl* info = + ResourceRequestInfoImpl::ForRequest(url_request); + is_download = !response_intercepted && info->IsDownload(); + is_stream = info->is_stream(); + previews_state = info->GetPreviewsState(); + if (rdh->delegate()) { + NavigationData* navigation_data = + rdh->delegate()->GetNavigationData(url_request); + + // Clone the embedder's NavigationData before moving it to the UI + // thread. + if (navigation_data) + cloned_navigation_data = navigation_data->Clone(); + } + + // non-S13nServiceWorker: + // This is similar to what is done in + // ServiceWorkerControlleeHandler::MaybeCreateSubresourceLoaderParams() + // (which is used when S13nServiceWorker is on). It takes the matching + // ControllerServiceWorkerInfo (if any) associated with the request. It + // will be sent to the renderer process and used to intercept requests. + ServiceWorkerProviderHost* sw_provider_host = + ServiceWorkerRequestHandler::GetProviderHost(url_request); + if (sw_provider_host && sw_provider_host->controller()) { + DCHECK(!blink::ServiceWorkerUtils::IsServicificationEnabled()); + subresource_loader_params_ = SubresourceLoaderParams(); + subresource_loader_params_->controller_service_worker_info = + mojom::ControllerServiceWorkerInfo::New(); + subresource_loader_params_->controller_service_worker_info->mode = + sw_provider_host->GetControllerMode(); + base::WeakPtr<ServiceWorkerObjectHost> sw_object_host = + sw_provider_host->GetOrCreateServiceWorkerObjectHost( + sw_provider_host->controller()); + if (sw_object_host) { + subresource_loader_params_->controller_service_worker_object_host = + sw_object_host; + subresource_loader_params_->controller_service_worker_info + ->object_info = sw_object_host->CreateIncompleteObjectInfo(); } - } else { - is_download = is_stream = false; } + } else { + is_download = is_stream = false; } CallOnReceivedResponse(head, std::move(url_loader_client_endpoints), std::move(cloned_navigation_data), is_download, - is_stream); + is_stream, previews_state); } #if BUILDFLAG(ENABLE_PLUGINS) void CheckPluginAndContinueOnReceiveResponse( const network::ResourceResponseHead& head, network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, + bool is_download_if_not_handled_by_plugin, const std::vector<WebPluginInfo>& plugins) { bool stale; WebPluginInfo plugin; @@ -1020,16 +1168,16 @@ class NavigationURLLoaderImpl::URLLoaderRequestController PluginService::GetInstance()->GetPlugins(base::BindOnce( &URLLoaderRequestController::CheckPluginAndContinueOnReceiveResponse, weak_factory_.GetWeakPtr(), head, - std::move(url_loader_client_endpoints))); + std::move(url_loader_client_endpoints), + is_download_if_not_handled_by_plugin)); return; } - bool is_download = - !has_plugin && - (!head.headers || head.headers->response_code() / 100 == 2); + bool is_download = !has_plugin && is_download_if_not_handled_by_plugin; CallOnReceivedResponse(head, std::move(url_loader_client_endpoints), - nullptr, is_download, false /* is_stream */); + nullptr, is_download, false /* is_stream */, + PREVIEWS_OFF /* previews_state */); } #endif @@ -1038,7 +1186,8 @@ class NavigationURLLoaderImpl::URLLoaderRequestController network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, std::unique_ptr<NavigationData> cloned_navigation_data, bool is_download, - bool is_stream) { + bool is_stream, + PreviewsState previews_state) { scoped_refptr<network::ResourceResponse> response( new network::ResourceResponse()); response->head = head; @@ -1055,11 +1204,18 @@ class NavigationURLLoaderImpl::URLLoaderRequestController response->DeepCopy(), std::move(url_loader_client_endpoints), std::move(cloned_navigation_data), global_request_id_, - is_download, is_stream)); + is_download, is_stream, previews_state)); } void OnReceiveRedirect(const net::RedirectInfo& redirect_info, const network::ResourceResponseHead& head) override { + if (base::FeatureList::IsEnabled(network::features::kNetworkService) && + !bypass_redirect_checks_ && + !IsRedirectSafe(url_, redirect_info.new_url, resource_context_)) { + OnComplete(network::URLLoaderCompletionStatus(net::ERR_UNSAFE_REDIRECT)); + return; + } + if (--redirect_limit_ == 0) { OnComplete( network::URLLoaderCompletionStatus(net::ERR_TOO_MANY_REDIRECTS)); @@ -1100,6 +1256,8 @@ class NavigationURLLoaderImpl::URLLoaderRequestController } void OnComplete(const network::URLLoaderCompletionStatus& status) override { + RecordSCTHistogramIfNeeded(status.ssl_info); + UMA_HISTOGRAM_BOOLEAN( "Navigation.URLLoaderNetworkService.OnCompleteHasSSLInfo", status.ssl_info.has_value()); @@ -1138,6 +1296,8 @@ class NavigationURLLoaderImpl::URLLoaderRequestController if (interceptor->MaybeCreateLoaderForResponse( response, &response_url_loader_, &response_client_request, url_loader_.get())) { + if (response_loader_binding_.is_bound()) + response_loader_binding_.Close(); response_loader_binding_.Bind(std::move(response_client_request)); default_loader_used_ = false; url_loader_.reset(); @@ -1147,8 +1307,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController return false; } - std::vector<std::unique_ptr<content::URLLoaderThrottle>> - CreateURLLoaderThrottles() { + std::vector<std::unique_ptr<URLLoaderThrottle>> CreateURLLoaderThrottles() { return GetContentClient()->browser()->CreateURLLoaderThrottles( *resource_request_, resource_context_, web_contents_getter_, navigation_ui_data_.get(), frame_tree_node_id_); @@ -1168,7 +1327,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController storage::BlobStorageContext* blob_storage_context = GetBlobStorageContext( GetChromeBlobStorageContextForResourceContext(resource_context_)); return ServiceWorkerRequestHandler::InitializeForNavigationNetworkService( - *resource_request_, resource_context_, + resource_request_->url, resource_context_, service_worker_navigation_handle_core, blob_storage_context, request_info.begin_params->skip_service_worker, resource_type, request_info.begin_params->request_context_type, frame_type, @@ -1176,6 +1335,21 @@ class NavigationURLLoaderImpl::URLLoaderRequestController web_contents_getter_); } + void RecordSCTHistogramIfNeeded( + const base::Optional<net::SSLInfo>& ssl_info) { + if (is_main_frame_ && url_.SchemeIsCryptographic() && + ssl_info.has_value()) { + int num_valid_scts = 0; + for (const auto& signed_certificate_timestamps : + ssl_info->signed_certificate_timestamps) { + if (signed_certificate_timestamps.status == net::ct::SCT_STATUS_OK) + ++num_valid_scts; + } + UMA_HISTOGRAM_COUNTS_100( + "Net.CertificateTransparency.MainFrameValidSCTCount", num_valid_scts); + } + } + std::vector<std::unique_ptr<NavigationLoaderInterceptor>> interceptors_; size_t interceptor_index_ = 0; @@ -1204,6 +1378,8 @@ class NavigationURLLoaderImpl::URLLoaderRequestController // Current URL that is being navigated, updated after redirection. GURL url_; + const bool is_main_frame_; + // Currently used by the AppCache loader to pass its factory to the // renderer which enables it to handle subresources. base::Optional<SubresourceLoaderParams> subresource_loader_params_; @@ -1266,6 +1442,9 @@ class NavigationURLLoaderImpl::URLLoaderRequestController // protocol handlers. std::set<std::string> known_schemes_; + // If true, redirect checks will be handled in a proxy, and not here. + bool bypass_redirect_checks_; + mutable base::WeakPtrFactory<URLLoaderRequestController> weak_factory_; DISALLOW_COPY_AND_ASSIGN(URLLoaderRequestController); @@ -1312,10 +1491,10 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl( /* initial_interceptors = */ std::vector<std::unique_ptr<NavigationLoaderInterceptor>>(), std::move(new_request), resource_context, - request_info->common_params.url, + request_info->common_params.url, request_info->is_main_frame, /* proxied_url_loader_factory_request */ nullptr, /* proxied_url_loader_factory_info */ nullptr, std::set<std::string>(), - weak_factory_.GetWeakPtr()); + /* bypass_redirect_checks */ false, weak_factory_.GetWeakPtr()); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, @@ -1344,6 +1523,7 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl( network::mojom::URLLoaderFactoryPtrInfo proxied_factory_info; network::mojom::URLLoaderFactoryRequest proxied_factory_request; + bool bypass_redirect_checks = false; auto* partition = static_cast<StoragePartitionImpl*>(storage_partition); if (frame_tree_node) { // |frame_tree_node| may be null in some unit test environments. @@ -1360,7 +1540,8 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl( auto factory_request = mojo::MakeRequest(&factory_info); bool use_proxy = GetContentClient()->browser()->WillCreateURLLoaderFactory( partition->browser_context(), frame_tree_node->current_frame_host(), - true /* is_navigation */, &factory_request); + true /* is_navigation */, new_request->url, &factory_request); + bypass_redirect_checks = use_proxy; if (RenderFrameDevToolsAgentHost::WillCreateURLLoaderFactory( frame_tree_node->current_frame_host(), true, false, &factory_request)) { @@ -1379,11 +1560,14 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl( storage_domain); } + non_network_url_loader_factories_[url::kAboutScheme] = + std::make_unique<AboutURLLoaderFactory>(); + non_network_url_loader_factories_[url::kFileScheme] = std::make_unique<FileURLLoaderFactory>( partition->browser_context()->GetPath(), base::CreateSequencedTaskRunnerWithTraits( - {base::MayBlock(), base::TaskPriority::BACKGROUND, + {base::MayBlock(), base::TaskPriority::BEST_EFFORT, base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})); std::set<std::string> known_schemes; for (auto& iter : non_network_url_loader_factories_) @@ -1392,15 +1576,15 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl( DCHECK(!request_controller_); request_controller_ = std::make_unique<URLLoaderRequestController>( std::move(initial_interceptors), std::move(new_request), resource_context, - request_info->common_params.url, std::move(proxied_factory_request), - std::move(proxied_factory_info), std::move(known_schemes), + request_info->common_params.url, request_info->is_main_frame, + std::move(proxied_factory_request), std::move(proxied_factory_info), + std::move(known_schemes), bypass_redirect_checks, weak_factory_.GetWeakPtr()); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, base::BindOnce( &URLLoaderRequestController::Start, base::Unretained(request_controller_.get()), - base::RetainedRef(storage_partition->GetURLRequestContext()), partition->url_loader_factory_getter()->GetNetworkFactoryInfo(), service_worker_navigation_handle_core, appcache_handle_core, std::move(request_info), std::move(navigation_ui_data), @@ -1432,7 +1616,8 @@ void NavigationURLLoaderImpl::OnReceiveResponse( std::unique_ptr<NavigationData> navigation_data, const GlobalRequestID& global_request_id, bool is_download, - bool is_stream) { + bool is_stream, + PreviewsState previews_state) { TRACE_EVENT_ASYNC_END2("navigation", "Navigation timeToResponseStarted", this, "&NavigationURLLoaderImpl", this, "success", true); @@ -1442,7 +1627,7 @@ void NavigationURLLoaderImpl::OnReceiveResponse( delegate_->OnResponseStarted( std::move(response), std::move(url_loader_client_endpoints), std::move(navigation_data), global_request_id, - allow_download_ && is_download, is_stream, + allow_download_ && is_download, is_stream, previews_state, request_controller_->TakeSubresourceLoaderParams()); } @@ -1491,7 +1676,7 @@ void NavigationURLLoaderImpl::BindNonNetworkURLLoaderFactoryRequest( auto* frame = frame_tree_node->current_frame_host(); GetContentClient()->browser()->WillCreateURLLoaderFactory( frame->GetSiteInstance()->GetBrowserContext(), frame, - true /* is_navigation */, &factory); + true /* is_navigation */, url, &factory); it->second->Clone(std::move(factory)); } diff --git a/chromium/content/browser/loader/navigation_url_loader_impl.h b/chromium/content/browser/loader/navigation_url_loader_impl.h index ac5e384a04d..1bcc9efb46d 100644 --- a/chromium/content/browser/loader/navigation_url_loader_impl.h +++ b/chromium/content/browser/loader/navigation_url_loader_impl.h @@ -12,6 +12,7 @@ #include "content/browser/loader/navigation_url_loader.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/ssl_status.h" +#include "content/public/common/previews_state.h" #include "services/network/public/mojom/url_loader.mojom.h" #include "services/network/public/mojom/url_loader_factory.mojom.h" @@ -22,9 +23,9 @@ struct RedirectInfo; namespace content { class NavigationData; +class NavigationLoaderInterceptor; class ResourceContext; class StoragePartition; -class NavigationLoaderInterceptor; struct GlobalRequestID; class CONTENT_EXPORT NavigationURLLoaderImpl : public NavigationURLLoader { @@ -56,7 +57,8 @@ class CONTENT_EXPORT NavigationURLLoaderImpl : public NavigationURLLoader { std::unique_ptr<NavigationData> navigation_data, const GlobalRequestID& global_request_id, bool is_download, - bool is_stream); + bool is_stream, + PreviewsState previews_state); void OnReceiveRedirect(const net::RedirectInfo& redirect_info, scoped_refptr<network::ResourceResponse> response); void OnComplete(const network::URLLoaderCompletionStatus& status); diff --git a/chromium/content/browser/loader/navigation_url_loader_impl_unittest.cc b/chromium/content/browser/loader/navigation_url_loader_impl_unittest.cc index 538274d42a9..37cf04dda21 100644 --- a/chromium/content/browser/loader/navigation_url_loader_impl_unittest.cc +++ b/chromium/content/browser/loader/navigation_url_loader_impl_unittest.cc @@ -71,13 +71,13 @@ class TestNavigationLoaderInterceptor : public NavigationLoaderInterceptor { void MaybeCreateLoader(const network::ResourceRequest& resource_request, ResourceContext* resource_context, - LoaderCallback callback) override { - std::move(callback).Run( - base::BindOnce(&TestNavigationLoaderInterceptor::StartLoader, - base::Unretained(this), resource_request)); + LoaderCallback callback, + FallbackCallback fallback_callback) override { + std::move(callback).Run(base::BindOnce( + &TestNavigationLoaderInterceptor::StartLoader, base::Unretained(this))); } - void StartLoader(network::ResourceRequest resource_request, + void StartLoader(const network::ResourceRequest& resource_request, network::mojom::URLLoaderRequest request, network::mojom::URLLoaderClientPtr client) { *most_recent_resource_request_ = resource_request; @@ -145,6 +145,10 @@ class NavigationURLLoaderImplTest : public testing::Test { } ~NavigationURLLoaderImplTest() override { + // The context needs to be deleted before ServiceManagerConnection is + // destroyed, so the storage partition in the context does not try to + // reconnect to the network service after ServiceManagerConnection is dead. + browser_context_.reset(); ServiceManagerConnection::DestroyForProcess(); } diff --git a/chromium/content/browser/loader/prefetch_browsertest.cc b/chromium/content/browser/loader/prefetch_browsertest.cc index fb0726ef37c..b3776b2bac6 100644 --- a/chromium/content/browser/loader/prefetch_browsertest.cc +++ b/chromium/content/browser/loader/prefetch_browsertest.cc @@ -29,11 +29,8 @@ namespace content { struct PrefetchBrowserTestParam { - PrefetchBrowserTestParam(bool network_service_enabled, - bool signed_exchange_enabled) - : network_service_enabled(network_service_enabled), - signed_exchange_enabled(signed_exchange_enabled) {} - const bool network_service_enabled; + PrefetchBrowserTestParam(bool signed_exchange_enabled) + : signed_exchange_enabled(signed_exchange_enabled) {} const bool signed_exchange_enabled; }; @@ -69,8 +66,6 @@ class PrefetchBrowserTest void SetUp() override { std::vector<base::Feature> enable_features; - if (GetParam().network_service_enabled) - enable_features.push_back(network::features::kNetworkService); if (GetParam().signed_exchange_enabled) enable_features.push_back(features::kSignedHTTPExchange); feature_list_.InitWithFeatures(enable_features, {}); @@ -110,12 +105,16 @@ class PrefetchBrowserTest return nullptr; } - void WatchURLAndRunClosure(const std::string& relative_url, - int* visit_count, - base::OnceClosure closure, - const net::test_server::HttpRequest& request) { + void WatchURLAndRunClosure( + const std::string& relative_url, + int* visit_count, + net::test_server::HttpRequest::HeaderMap* out_headers, + base::OnceClosure closure, + const net::test_server::HttpRequest& request) { if (request.relative_url == relative_url) { (*visit_count)++; + if (out_headers) + *out_headers = request.headers; if (closure) std::move(closure).Run(); } @@ -124,8 +123,7 @@ class PrefetchBrowserTest void OnPrefetchURLLoaderCalled() { prefetch_url_loader_called_++; } bool CheckPrefetchURLLoaderCountIfSupported(int expected) const { - if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchange) && - !base::FeatureList::IsEnabled(network::features::kNetworkService)) + if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchange)) return true; return prefetch_url_loader_called_ == expected; } @@ -153,7 +151,7 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, Simple) { base::RunLoop prefetch_waiter; embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - target_url, &target_fetch_count, prefetch_waiter.QuitClosure())); + target_url, &target_fetch_count, nullptr, prefetch_waiter.QuitClosure())); embedded_test_server()->RegisterRequestHandler(base::BindRepeating( &PrefetchBrowserTest::ServeResponses, base::Unretained(this))); ASSERT_TRUE(embedded_test_server()->Start()); @@ -191,7 +189,7 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, DoublePrefetch) { base::RunLoop prefetch_waiter; embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - target_url, &target_fetch_count, prefetch_waiter.QuitClosure())); + target_url, &target_fetch_count, nullptr, prefetch_waiter.QuitClosure())); embedded_test_server()->RegisterRequestHandler(base::BindRepeating( &PrefetchBrowserTest::ServeResponses, base::Unretained(this))); ASSERT_TRUE(embedded_test_server()->Start()); @@ -238,10 +236,12 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, NoCacheAndNoStore) { base::RunLoop nostore_waiter; embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - nocache_url, &nocache_fetch_count, nocache_waiter.QuitClosure())); + nocache_url, &nocache_fetch_count, nullptr, + nocache_waiter.QuitClosure())); embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - nostore_url, &nostore_fetch_count, nostore_waiter.QuitClosure())); + nostore_url, &nostore_fetch_count, nullptr, + nostore_waiter.QuitClosure())); embedded_test_server()->RegisterRequestHandler(base::BindRepeating( &PrefetchBrowserTest::ServeResponses, base::Unretained(this))); ASSERT_TRUE(embedded_test_server()->Start()); @@ -298,10 +298,11 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, WithPreload) { base::RunLoop preload_waiter; embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - target_url, &target_fetch_count, base::RepeatingClosure())); + target_url, &target_fetch_count, nullptr, base::RepeatingClosure())); embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - preload_url, &preload_fetch_count, preload_waiter.QuitClosure())); + preload_url, &preload_fetch_count, nullptr, + preload_waiter.QuitClosure())); embedded_test_server()->RegisterRequestHandler(base::BindRepeating( &PrefetchBrowserTest::ServeResponses, base::Unretained(this))); ASSERT_TRUE(embedded_test_server()->Start()); @@ -344,27 +345,30 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, WebPackageWithPreload) { RegisterResponse( target_sxg, // We mock the SignedExchangeHandler, so just return a HTML content - // as "application/signed-exchange;v=b0". + // as "application/signed-exchange;v=b2". ResponseEntry("<head><title>Prefetch Target (SXG)</title></head>", - "application/signed-exchange;v=b0")); + "application/signed-exchange;v=b2")); RegisterResponse(preload_url_in_sxg, ResponseEntry("function foo() {}", "text/javascript")); base::RunLoop preload_waiter; base::RunLoop prefetch_waiter; + net::test_server::HttpRequest::HeaderMap prefetch_headers; embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - target_sxg, &target_fetch_count, prefetch_waiter.QuitClosure())); + target_sxg, &target_fetch_count, &prefetch_headers, + prefetch_waiter.QuitClosure())); embedded_test_server()->RegisterRequestMonitor(base::BindRepeating( &PrefetchBrowserTest::WatchURLAndRunClosure, base::Unretained(this), - preload_url_in_sxg, &preload_fetch_count, preload_waiter.QuitClosure())); + preload_url_in_sxg, &preload_fetch_count, nullptr, + preload_waiter.QuitClosure())); embedded_test_server()->RegisterRequestHandler(base::BindRepeating( &PrefetchBrowserTest::ServeResponses, base::Unretained(this))); ASSERT_TRUE(embedded_test_server()->Start()); EXPECT_TRUE(CheckPrefetchURLLoaderCountIfSupported(0)); MockSignedExchangeHandlerFactory factory( - net::OK, GURL(target_url), "text/html", + net::OK, GURL(embedded_test_server()->GetURL(target_url)), "text/html", {base::StringPrintf( "Link: <%s>;rel=\"preload\";as=\"script\"", embedded_test_server()->GetURL(preload_url_in_sxg).spec().c_str())}); @@ -376,13 +380,14 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, WebPackageWithPreload) { prefetch_waiter.Run(); EXPECT_EQ(1, target_fetch_count); EXPECT_TRUE(CheckPrefetchURLLoaderCountIfSupported(1)); - - // Test after this point requires SignedHTTPExchange support, which is now - // disabled when Network Service is enabled. - // TODO(https://crbug.com/849935): Remove the second condition once we - // re-enable Signed Exchange with Network Service. - if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchange) || - base::FeatureList::IsEnabled(network::features::kNetworkService)) + if (base::FeatureList::IsEnabled(features::kSignedHTTPExchange)) + EXPECT_EQ(prefetch_headers["Accept"], + "application/signed-exchange;v=b2;q=0.9,*/*;q=0.8"); + else + EXPECT_EQ(prefetch_headers["Accept"], "*/*"); + + // Test after this point requires SignedHTTPExchange support + if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchange)) return; // If the header in the .sxg file is correctly extracted, we should @@ -393,10 +398,7 @@ IN_PROC_BROWSER_TEST_P(PrefetchBrowserTest, WebPackageWithPreload) { INSTANTIATE_TEST_CASE_P(PrefetchBrowserTest, PrefetchBrowserTest, - testing::Values(PrefetchBrowserTestParam(true, true), - PrefetchBrowserTestParam(true, false), - PrefetchBrowserTestParam(false, true), - PrefetchBrowserTestParam(false, - false))); + testing::Values(PrefetchBrowserTestParam(true), + PrefetchBrowserTestParam(false))); } // namespace content diff --git a/chromium/content/browser/loader/resource_dispatcher_host_impl.cc b/chromium/content/browser/loader/resource_dispatcher_host_impl.cc index 061ad189c46..2ed617dbab8 100644 --- a/chromium/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/chromium/content/browser/loader/resource_dispatcher_host_impl.cc @@ -28,8 +28,8 @@ #include "base/metrics/histogram_macros.h" #include "base/stl_util.h" #include "base/strings/string_util.h" -#include "base/task_scheduler/post_task.h" -#include "base/task_scheduler/task_traits.h" +#include "base/task/post_task.h" +#include "base/task/task_traits.h" #include "base/third_party/dynamic_annotations/dynamic_annotations.h" #include "base/threading/thread_task_runner_handle.h" #include "base/timer/timer.h" @@ -91,7 +91,6 @@ #include "net/base/registry_controlled_domains/registry_controlled_domain.h" #include "net/base/upload_data_stream.h" #include "net/base/url_util.h" -#include "net/cert/cert_status_flags.h" #include "net/cookies/cookie_monster.h" #include "net/http/http_request_headers.h" #include "net/http/http_response_headers.h" @@ -169,11 +168,6 @@ void AbortRequestBeforeItStarts( url_loader_client->OnComplete(status); } -bool IsValidatedSCT( - const net::SignedCertificateTimestampAndStatus& sct_status) { - return sct_status.status == net::ct::SCT_STATUS_OK; -} - // Returns the PreviewsState for enabled previews after requesting it from // the delegate. The PreviewsState is a bitmask of potentially several // Previews optimizations that are initially enabled for a navigation. @@ -605,18 +599,6 @@ void ResourceDispatcherHostImpl::DidReceiveResponse( void ResourceDispatcherHostImpl::DidFinishLoading(ResourceLoader* loader) { ResourceRequestInfoImpl* info = loader->GetRequestInfo(); - // Record final result of all resource loads. - if (info->GetResourceType() == RESOURCE_TYPE_MAIN_FRAME) { - if (loader->request()->url().SchemeIsCryptographic()) { - int num_valid_scts = std::count_if( - loader->request()->ssl_info().signed_certificate_timestamps.begin(), - loader->request()->ssl_info().signed_certificate_timestamps.end(), - IsValidatedSCT); - UMA_HISTOGRAM_COUNTS_100( - "Net.CertificateTransparency.MainFrameValidSCTCount", num_valid_scts); - } - } - if (delegate_) delegate_->RequestComplete(loader->request()); @@ -1078,11 +1060,11 @@ ResourceDispatcherHostImpl::CreateResourceHandler( requester_info->IsNavigationPreload() || requester_info->IsCertificateFetcherForSignedExchange()); // Construct the IPC resource handler. - std::unique_ptr<ResourceHandler> handler; - handler = CreateBaseResourceHandler( - request, url_loader_options, std::move(mojo_request), - std::move(url_loader_client), - static_cast<ResourceType>(request_data.resource_type)); + std::unique_ptr<ResourceHandler> handler = + std::make_unique<MojoAsyncResourceHandler>( + request, this, std::move(mojo_request), std::move(url_loader_client), + static_cast<ResourceType>(request_data.resource_type), + url_loader_options); // Prefetches outlive their child process. if (request_data.resource_type == RESOURCE_TYPE_PREFETCH) { @@ -1097,22 +1079,8 @@ ResourceDispatcherHostImpl::CreateResourceHandler( request, static_cast<ResourceType>(request_data.resource_type), resource_context, request_data.fetch_request_mode, static_cast<RequestContextType>(request_data.fetch_request_context_type), - requester_info->appcache_service(), child_id, route_id, - std::move(handler)); -} - -std::unique_ptr<ResourceHandler> -ResourceDispatcherHostImpl::CreateBaseResourceHandler( - net::URLRequest* request, - uint32_t url_loader_options, - network::mojom::URLLoaderRequest mojo_request, - network::mojom::URLLoaderClientPtr url_loader_client, - ResourceType resource_type) { - std::unique_ptr<ResourceHandler> handler; - handler.reset(new MojoAsyncResourceHandler( - request, this, std::move(mojo_request), std::move(url_loader_client), - resource_type, url_loader_options)); - return handler; + url_loader_options, requester_info->appcache_service(), child_id, + route_id, std::move(handler)); } std::unique_ptr<ResourceHandler> @@ -1122,6 +1090,7 @@ ResourceDispatcherHostImpl::AddStandardHandlers( ResourceContext* resource_context, network::mojom::FetchRequestMode fetch_request_mode, RequestContextType fetch_request_context_type, + uint32_t url_loader_options, AppCacheService* appcache_service, int child_id, int route_id, @@ -1190,9 +1159,11 @@ ResourceDispatcherHostImpl::AddStandardHandlers( // Note: all ResourceHandler following the MimeSniffingResourceHandler // should expect OnWillRead to be called *before* OnResponseStarted as // part of the mime sniffing process. - handler.reset(new MimeSniffingResourceHandler( - std::move(handler), this, plugin_service, intercepting_handler, request, - fetch_request_context_type)); + if (url_loader_options & network::mojom::kURLLoadOptionSniffMimeType) { + handler.reset(new MimeSniffingResourceHandler( + std::move(handler), this, plugin_service, intercepting_handler, request, + fetch_request_context_type)); + } // Add the pre mime sniffing throttles. handler.reset(new ThrottlingResourceHandler( @@ -1644,7 +1615,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest( handler = AddStandardHandlers( new_request.get(), resource_type, resource_context, network::mojom::FetchRequestMode::kNoCORS, - info.begin_params->request_context_type, + info.begin_params->request_context_type, url_loader_options, appcache_handle_core ? appcache_handle_core->GetAppCacheService() : nullptr, -1, // child_id @@ -1927,6 +1898,7 @@ net::URLRequest* ResourceDispatcherHostImpl::GetURLRequest( } // static +// This is duplicated in services/network/network_service.cc. bool ResourceDispatcherHostImpl::LoadInfoIsMoreInteresting(const LoadInfo& a, const LoadInfo& b) { // Set |*_uploading_size| to be the size of the corresponding upload body if diff --git a/chromium/content/browser/loader/resource_dispatcher_host_impl.h b/chromium/content/browser/loader/resource_dispatcher_host_impl.h index b8a81809fff..ec89329e361 100644 --- a/chromium/content/browser/loader/resource_dispatcher_host_impl.h +++ b/chromium/content/browser/loader/resource_dispatcher_host_impl.h @@ -29,10 +29,10 @@ #include "base/single_thread_task_runner.h" #include "base/time/time.h" #include "content/browser/blob_storage/chrome_blob_storage_context.h" -#include "content/browser/loader/global_routing_id.h" #include "content/browser/loader/resource_loader_delegate.h" #include "content/common/content_export.h" #include "content/public/browser/global_request_id.h" +#include "content/public/browser/global_routing_id.h" #include "content/public/browser/resource_dispatcher_host.h" #include "content/public/browser/resource_request_info.h" #include "content/public/browser/stream_handle.h" @@ -330,6 +330,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl private: class ScheduledResourceRequestAdapter; + friend class NetworkServiceClient; friend class ResourceDispatcherHostTest; FRIEND_TEST_ALL_PREFIXES(ResourceDispatcherHostTest, @@ -587,14 +588,6 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl network::mojom::URLLoaderRequest mojo_request, network::mojom::URLLoaderClientPtr url_loader_client); - // Creates either MojoAsyncResourceHandler or AsyncResourceHandler. - std::unique_ptr<ResourceHandler> CreateBaseResourceHandler( - net::URLRequest* request, - uint32_t url_loader_options, - network::mojom::URLLoaderRequest mojo_request, - network::mojom::URLLoaderClientPtr url_loader_client, - ResourceType resource_type); - // Wraps |handler| in the standard resource handlers for normal resource // loading and navigation requests. This adds MimeTypeResourceHandler and // ResourceThrottles. @@ -604,6 +597,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl ResourceContext* resource_context, network::mojom::FetchRequestMode fetch_request_mode, RequestContextType fetch_request_context_type, + uint32_t url_loader_options, AppCacheService* appcache_service, int child_id, int route_id, diff --git a/chromium/content/browser/loader/resource_dispatcher_host_unittest.cc b/chromium/content/browser/loader/resource_dispatcher_host_unittest.cc index d8d744e64ff..38053a77162 100644 --- a/chromium/content/browser/loader/resource_dispatcher_host_unittest.cc +++ b/chromium/content/browser/loader/resource_dispatcher_host_unittest.cc @@ -17,8 +17,8 @@ #include "base/single_thread_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" -#include "base/task_scheduler/post_task.h" -#include "base/task_scheduler/task_traits.h" +#include "base/task/post_task.h" +#include "base/task/task_traits.h" #include "base/test/scoped_feature_list.h" #include "base/threading/thread_task_runner_handle.h" #include "base/unguessable_token.h" @@ -59,6 +59,7 @@ #include "content/test/test_navigation_url_loader_delegate.h" #include "mojo/public/cpp/system/data_pipe_utils.h" #include "net/base/chunked_upload_data_stream.h" +#include "net/base/completion_once_callback.h" #include "net/base/elements_upload_data_stream.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" @@ -106,6 +107,7 @@ static network::ResourceRequest CreateResourceRequest(const char* method, request.resource_type = type; request.appcache_host_id = kAppCacheNoHostId; request.should_reset_appcache = false; + request.render_frame_id = 0; request.is_main_frame = true; request.transition_type = ui::PAGE_TRANSITION_LINK; request.allow_download = true; @@ -311,7 +313,7 @@ class URLRequestBigJob : public net::URLRequestSimpleJob { int GetData(std::string* mime_type, std::string* charset, std::string* data, - const net::CompletionCallback& callback) const override { + net::CompletionOnceCallback callback) const override { *mime_type = "text/plain"; *charset = "UTF-8"; @@ -897,7 +899,7 @@ void ResourceDispatcherHostTest::MakeTestRequestWithRenderFrame( request.render_frame_id = render_frame_id; filter_->CreateLoaderAndStart( std::move(loader_request), render_view_id, request_id, - network::mojom::kURLLoadOptionNone, request, std::move(client), + network::mojom::kURLLoadOptionSniffMimeType, request, std::move(client), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); } @@ -912,7 +914,7 @@ void ResourceDispatcherHostTest::MakeTestRequestWithResourceType( network::ResourceRequest request = CreateResourceRequest("GET", type, url); filter->CreateLoaderAndStart( std::move(loader_request), render_view_id, request_id, - network::mojom::kURLLoadOptionNone, request, std::move(client), + network::mojom::kURLLoadOptionSniffMimeType, request, std::move(client), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); } @@ -1551,9 +1553,9 @@ TEST_F(ResourceDispatcherHostTest, TestProcessCancelDetachedTimesOut) { // messages. base::RunLoop run_loop; base::OneShotTimer timer; - timer.Start( - FROM_HERE, base::TimeDelta::FromMilliseconds(210), - base::Bind(&base::RunLoop::QuitWhenIdle, base::Unretained(&run_loop))); + timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(210), + base::BindOnce(&base::RunLoop::QuitWhenIdle, + base::Unretained(&run_loop))); run_loop.Run(); // The prefetch should be cancelled by now. @@ -2010,6 +2012,7 @@ TEST_F(ResourceDispatcherHostTest, MimeSniffed) { client.RunUntilResponseReceived(); EXPECT_EQ("text/html", client.response_head().mime_type); + EXPECT_TRUE(client.response_head().did_mime_sniff); } // Tests that we don't sniff the mime type when the server provides one. @@ -2031,6 +2034,7 @@ TEST_F(ResourceDispatcherHostTest, MimeNotSniffed) { client.RunUntilResponseReceived(); EXPECT_EQ("image/jpeg", client.response_head().mime_type); + EXPECT_FALSE(client.response_head().did_mime_sniff); } // Tests that we don't sniff the mime type when there is no message body. diff --git a/chromium/content/browser/loader/resource_hints_impl.cc b/chromium/content/browser/loader/resource_hints_impl.cc index 3ebe0570196..bdb06548a3a 100644 --- a/chromium/content/browser/loader/resource_hints_impl.cc +++ b/chromium/content/browser/loader/resource_hints_impl.cc @@ -102,7 +102,7 @@ int PreresolveUrl(net::URLRequestContextGetter* getter, resolve_info.set_is_speculative(true); return resolver->Resolve( resolve_info, net::IDLE, raw_addresses, - base::Bind(&OnResolveComplete, base::Passed(&addresses), callback), + base::BindOnce(&OnResolveComplete, std::move(addresses), callback), out_request, net::NetLogWithSource()); } diff --git a/chromium/content/browser/loader/resource_loader.cc b/chromium/content/browser/loader/resource_loader.cc index 2435233851b..ff351ba64cf 100644 --- a/chromium/content/browser/loader/resource_loader.cc +++ b/chromium/content/browser/loader/resource_loader.cc @@ -87,12 +87,6 @@ void PopulateResourceResponse( response->head.network_accessed = response_info.network_accessed; response->head.async_revalidation_requested = response_info.async_revalidation_requested; - const content::ResourceRequestInfo* request_info = - content::ResourceRequestInfo::ForRequest(request); - if (request_info) { - response->head.previews_state = - static_cast<int>(request_info->GetPreviewsState()); - } if (info->ShouldReportRawHeaders()) { response->head.raw_request_response_info = network::BuildRawRequestResponseInfo(*request, raw_request_headers, diff --git a/chromium/content/browser/loader/resource_request_info_impl.cc b/chromium/content/browser/loader/resource_request_info_impl.cc index 3db8fd33813..7fe59fe5542 100644 --- a/chromium/content/browser/loader/resource_request_info_impl.cc +++ b/chromium/content/browser/loader/resource_request_info_impl.cc @@ -5,13 +5,13 @@ #include "content/browser/loader/resource_request_info_impl.h" #include "content/browser/frame_host/frame_tree_node.h" -#include "content/browser/loader/global_routing_id.h" #include "content/browser/loader/resource_message_filter.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/common/net/url_request_service_worker_data.h" #include "content/common/net/url_request_user_data.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/global_request_id.h" +#include "content/public/browser/global_routing_id.h" #include "content/public/common/browser_side_navigation_policy.h" #include "content/public/common/process_type.h" #include "net/url_request/url_request.h" @@ -314,6 +314,10 @@ PreviewsState ResourceRequestInfoImpl::GetPreviewsState() const { return previews_state_; } +void ResourceRequestInfoImpl::SetPreviewsState(PreviewsState previews_state) { + previews_state_ = previews_state; +} + NavigationUIData* ResourceRequestInfoImpl::GetNavigationUIData() const { return navigation_ui_data_.get(); } diff --git a/chromium/content/browser/loader/resource_request_info_impl.h b/chromium/content/browser/loader/resource_request_info_impl.h index 0bc8ea93804..ec89c14e24b 100644 --- a/chromium/content/browser/loader/resource_request_info_impl.h +++ b/chromium/content/browser/loader/resource_request_info_impl.h @@ -96,6 +96,7 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo, bool IsDownload() const override; // Returns a bitmask of potentially several Previews optimizations. PreviewsState GetPreviewsState() const override; + void SetPreviewsState(PreviewsState previews_state) override; NavigationUIData* GetNavigationUIData() const override; DevToolsStatus GetDevToolsStatus() const override; void SetResourceRequestBlockedReason( diff --git a/chromium/content/browser/loader/resource_requester_info.cc b/chromium/content/browser/loader/resource_requester_info.cc index 862ff7ff379..f692a6fa920 100644 --- a/chromium/content/browser/loader/resource_requester_info.cc +++ b/chromium/content/browser/loader/resource_requester_info.cc @@ -4,6 +4,8 @@ #include "content/browser/loader/resource_requester_info.h" +#include <utility> + #include "base/feature_list.h" #include "base/logging.h" #include "content/browser/appcache/chrome_appcache_service.h" @@ -105,23 +107,16 @@ ResourceRequesterInfo::CreateForDownloadOrPageSave(int child_id) { scoped_refptr<ResourceRequesterInfo> ResourceRequesterInfo::CreateForNavigationPreload( ResourceRequesterInfo* original_request_info) { - GetContextsCallback get_contexts_callback = - original_request_info->get_contexts_callback_; - if (IsBrowserSideNavigationEnabled()) { - DCHECK(original_request_info->IsBrowserSideNavigation()); - DCHECK(!get_contexts_callback); - DCHECK(original_request_info->service_worker_context()); - // The requester info for browser side navigation doesn't have the - // get_contexts_callback. So create the callback here which gets the - // ResourceContext and the URLRequestContext form ServiceWorkerContext. - get_contexts_callback = - base::Bind(&GetContextsCallbackForNavigationPreload, - scoped_refptr<ServiceWorkerContextWrapper>( - original_request_info->service_worker_context())); - } else { - DCHECK(original_request_info->IsRenderer()); - DCHECK(get_contexts_callback); - } + DCHECK(original_request_info->IsBrowserSideNavigation()); + DCHECK(original_request_info->service_worker_context()); + DCHECK(!original_request_info->get_contexts_callback_); + // The requester info for browser side navigation doesn't have the + // get_contexts_callback. So create the callback here which gets the + // ResourceContext and the URLRequestContext from ServiceWorkerContext. + auto get_contexts_callback = + base::BindRepeating(&GetContextsCallbackForNavigationPreload, + scoped_refptr<ServiceWorkerContextWrapper>( + original_request_info->service_worker_context())); return scoped_refptr<ResourceRequesterInfo>(new ResourceRequesterInfo( RequesterType::NAVIGATION_PRELOAD, ChildProcessHost::kInvalidUniqueID, diff --git a/chromium/content/browser/loader/url_loader_factory_impl_unittest.cc b/chromium/content/browser/loader/url_loader_factory_impl_unittest.cc index 9385955b5c5..168eed74072 100644 --- a/chromium/content/browser/loader/url_loader_factory_impl_unittest.cc +++ b/chromium/content/browser/loader/url_loader_factory_impl_unittest.cc @@ -149,7 +149,8 @@ TEST_P(URLLoaderFactoryImplTest, GetResponse) { request.request_initiator = url::Origin::Create(request.url); factory_->CreateLoaderAndStart( mojo::MakeRequest(&loader), kRoutingId, kRequestId, - network::mojom::kURLLoadOptionNone, request, client.CreateInterfacePtr(), + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); ASSERT_FALSE(client.has_received_response()); @@ -224,8 +225,9 @@ TEST_P(URLLoaderFactoryImplTest, GetFailedResponse) { // Need to set same-site |request_initiator| for non main frame type request. request.request_initiator = url::Origin::Create(request.url); factory_->CreateLoaderAndStart( - mojo::MakeRequest(&loader), 2, 1, network::mojom::kURLLoadOptionNone, - request, client.CreateInterfacePtr(), + mojo::MakeRequest(&loader), 2, 1, + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); client.RunUntilComplete(); @@ -253,8 +255,9 @@ TEST_P(URLLoaderFactoryImplTest, GetFailedResponse2) { // Need to set same-site |request_initiator| for non main frame type request. request.request_initiator = url::Origin::Create(request.url); factory_->CreateLoaderAndStart( - mojo::MakeRequest(&loader), 2, 1, network::mojom::kURLLoadOptionNone, - request, client.CreateInterfacePtr(), + mojo::MakeRequest(&loader), 2, 1, + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); client.RunUntilComplete(); @@ -281,8 +284,9 @@ TEST_P(URLLoaderFactoryImplTest, InvalidURL) { request.request_initiator = url::Origin::Create(request.url); ASSERT_FALSE(request.url.is_valid()); factory_->CreateLoaderAndStart( - mojo::MakeRequest(&loader), 2, 1, network::mojom::kURLLoadOptionNone, - request, client.CreateInterfacePtr(), + mojo::MakeRequest(&loader), 2, 1, + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); client.RunUntilComplete(); @@ -310,8 +314,9 @@ TEST_P(URLLoaderFactoryImplTest, ShouldNotRequestURL) { // Need to set same-site |request_initiator| for non main frame type request. request.request_initiator = url::Origin::Create(request.url); factory_->CreateLoaderAndStart( - mojo::MakeRequest(&loader), 2, 1, network::mojom::kURLLoadOptionNone, - request, client.CreateInterfacePtr(), + mojo::MakeRequest(&loader), 2, 1, + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); client.RunUntilComplete(); @@ -343,7 +348,8 @@ TEST_P(URLLoaderFactoryImplTest, OnTransferSizeUpdated) { request.report_raw_headers = true; factory_->CreateLoaderAndStart( mojo::MakeRequest(&loader), kRoutingId, kRequestId, - network::mojom::kURLLoadOptionNone, request, client.CreateInterfacePtr(), + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); client.RunUntilComplete(); @@ -403,7 +409,8 @@ TEST_P(URLLoaderFactoryImplTest, CancelFromRenderer) { request.request_initiator = url::Origin::Create(request.url); factory_->CreateLoaderAndStart( mojo::MakeRequest(&loader), kRoutingId, kRequestId, - network::mojom::kURLLoadOptionNone, request, client.CreateInterfacePtr(), + network::mojom::kURLLoadOptionSniffMimeType, request, + client.CreateInterfacePtr(), net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); base::RunLoop().RunUntilIdle(); |