Skip to content

Commit

Permalink
Fix modified headers from URLLoaderThrottles not being sent over the …
Browse files Browse the repository at this point in the history
…network.

r605518 fixed a modified URL from URLLoaderThrottles not being sent over the network. However it didn't fix the headers case, which is used by other policies. This fixes the ForceYouTubeRestrict and AllowedDomainsForApps policies.

Bug: 899268,900574
Change-Id: I5d9666a7497c2c7e5642e5c74d5bc65dc0407ea9
Reviewed-on: https://chromium-review.googlesource.com/c/1324932
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606247}
  • Loading branch information
John Abd-El-Malek committed Nov 8, 2018
1 parent 2e2956a commit c460cb6
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 15 deletions.
4 changes: 4 additions & 0 deletions content/browser/loader/loader_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,7 @@ class URLModifyingThrottle : public URLLoaderThrottle {
GURL::Replacements replacements;
replacements.SetQueryStr("foo=bar");
request->url = request->url.ReplaceComponents(replacements);
request->headers.SetHeader("Foo", "Bar");
}

private:
Expand Down Expand Up @@ -1182,10 +1183,12 @@ IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, URLLoaderThrottleModifyURL) {
SetBrowserClientForTesting(&content_browser_client);

std::set<GURL> urls_requested;
std::map<GURL, net::test_server::HttpRequest::HeaderMap> header_map;
embedded_test_server()->RegisterRequestMonitor(base::BindLambdaForTesting(
[&](const net::test_server::HttpRequest& request) {
base::AutoLock auto_lock(lock);
urls_requested.insert(request.GetURL());
header_map[request.GetURL()] = request.headers;
}));

ASSERT_TRUE(embedded_test_server()->Start());
Expand All @@ -1197,6 +1200,7 @@ IN_PROC_BROWSER_TEST_F(LoaderBrowserTest, URLLoaderThrottleModifyURL) {
GURL expected_url(url.spec() + "?foo=bar");
base::AutoLock auto_lock(lock);
ASSERT_TRUE(urls_requested.find(expected_url) != urls_requested.end());
ASSERT_TRUE(header_map[expected_url]["Foo"] == "Bar");
}

SetBrowserClientForTesting(old_content_browser_client);
Expand Down
4 changes: 4 additions & 0 deletions content/browser/loader/navigation_url_loader_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,10 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
kNavigationUrlLoaderTrafficAnnotation));
}

// A URLLoaderThrottle may have changed the headers.
request_info_->begin_params->headers =
resource_request_->headers.ToString();

// The ResourceDispatcherHostImpl can be null in unit tests.
if (!intercepted && ResourceDispatcherHostImpl::Get()) {
ResourceDispatcherHostImpl::Get()->BeginNavigationRequest(
Expand Down
18 changes: 3 additions & 15 deletions content/browser/service_worker/service_worker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3630,21 +3630,9 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(true, EvalJs(shell()->web_contents()->GetMainFrame(),
"!!navigator.serviceWorker.controller"));

if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
// The injected header should be present.
EXPECT_EQ("injected value", EvalJs(shell()->web_contents()->GetMainFrame(),
"document.body.textContent"));
} else {
// S13nServiceWorker: the injected header is not present. Throttle-modified
// headers are not propagated to network requests through
// ResourceDispatcherHost, because the legacy network code has its own code
// path that applies the same throttling independent from the navigation's
// URLLoaderThrottles.
DCHECK(base::FeatureList::IsEnabled(
blink::features::kServiceWorkerServicification));
EXPECT_EQ("None", EvalJs(shell()->web_contents()->GetMainFrame(),
"document.body.textContent"));
}
// The injected header should be present.
EXPECT_EQ("injected value", EvalJs(shell()->web_contents()->GetMainFrame(),
"document.body.textContent"));

SetBrowserClientForTesting(old_content_browser_client);
}
Expand Down

0 comments on commit c460cb6

Please sign in to comment.