Commit 8f9ca770 authored by John Abd-El-Malek's avatar John Abd-El-Malek Committed by Commit Bot

Revert "Ensure header modification for a redirect's onBeforeSendHeaders is...

Revert "Ensure header modification for a redirect's onBeforeSendHeaders is applied with network service."

This reverts commit b5e1969b.

Bug: 949478

Original change's description:
> Ensure header modification for a redirect's onBeforeSendHeaders is applied with network service.
> 
> This currently worked if 'extraHeaders' was specified, but not without it.
> 
> Bug: 942062
> Change-Id: Ic962ad256d89384082e25a5983181bf563021ffb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1548673
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Ken Rockot <rockot@google.com>
> Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#647160}

TBR=jam@chromium.org,rockot@google.com,karandeepb@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 942062
Change-Id: I8aadb1bb0bcb0562a607c2a74d3c2cf1060c560f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1557746Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648758}
parent a8177a9c
...@@ -2073,12 +2073,10 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) { ...@@ -2073,12 +2073,10 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
deltas.push_back(std::move(d0)); deltas.push_back(std::move(d0));
} }
bool request_headers_modified0; bool request_headers_modified0;
std::set<std::string> ignore1, ignore2;
net::HttpRequestHeaders headers0; net::HttpRequestHeaders headers0;
headers0.MergeFrom(base_headers); headers0.MergeFrom(base_headers);
MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers0, &ignored_actions, MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers0, &ignored_actions,
&logger, &ignore1, &ignore2, &logger, &request_headers_modified0);
&request_headers_modified0);
ASSERT_TRUE(headers0.GetHeader("key1", &header_value)); ASSERT_TRUE(headers0.GetHeader("key1", &header_value));
EXPECT_EQ("value 1", header_value); EXPECT_EQ("value 1", header_value);
ASSERT_TRUE(headers0.GetHeader("key2", &header_value)); ASSERT_TRUE(headers0.GetHeader("key2", &header_value));
...@@ -2098,14 +2096,11 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) { ...@@ -2098,14 +2096,11 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
deltas.sort(&InDecreasingExtensionInstallationTimeOrder); deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
ignored_actions.clear(); ignored_actions.clear();
logger.clear(); logger.clear();
ignore1.clear();
ignore2.clear();
bool request_headers_modified1; bool request_headers_modified1;
net::HttpRequestHeaders headers1; net::HttpRequestHeaders headers1;
headers1.MergeFrom(base_headers); headers1.MergeFrom(base_headers);
MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers1, &ignored_actions, MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers1, &ignored_actions,
&logger, &ignore1, &ignore2, &logger, &request_headers_modified1);
&request_headers_modified1);
EXPECT_FALSE(headers1.HasHeader("key1")); EXPECT_FALSE(headers1.HasHeader("key1"));
ASSERT_TRUE(headers1.GetHeader("key2", &header_value)); ASSERT_TRUE(headers1.GetHeader("key2", &header_value));
EXPECT_EQ("value 3", header_value); EXPECT_EQ("value 3", header_value);
...@@ -2127,14 +2122,11 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) { ...@@ -2127,14 +2122,11 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
deltas.sort(&InDecreasingExtensionInstallationTimeOrder); deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
ignored_actions.clear(); ignored_actions.clear();
logger.clear(); logger.clear();
ignore1.clear();
ignore2.clear();
bool request_headers_modified2; bool request_headers_modified2;
net::HttpRequestHeaders headers2; net::HttpRequestHeaders headers2;
headers2.MergeFrom(base_headers); headers2.MergeFrom(base_headers);
MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers2, &ignored_actions, MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers2, &ignored_actions,
&logger, &ignore1, &ignore2, &logger, &request_headers_modified2);
&request_headers_modified2);
EXPECT_FALSE(headers2.HasHeader("key1")); EXPECT_FALSE(headers2.HasHeader("key1"));
ASSERT_TRUE(headers2.GetHeader("key2", &header_value)); ASSERT_TRUE(headers2.GetHeader("key2", &header_value));
EXPECT_EQ("value 3", header_value); EXPECT_EQ("value 3", header_value);
...@@ -2160,14 +2152,11 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) { ...@@ -2160,14 +2152,11 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
deltas.sort(&InDecreasingExtensionInstallationTimeOrder); deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
ignored_actions.clear(); ignored_actions.clear();
logger.clear(); logger.clear();
ignore1.clear();
ignore2.clear();
bool request_headers_modified3; bool request_headers_modified3;
net::HttpRequestHeaders headers3; net::HttpRequestHeaders headers3;
headers3.MergeFrom(base_headers); headers3.MergeFrom(base_headers);
MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers3, &ignored_actions, MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers3, &ignored_actions,
&logger, &ignore1, &ignore2, &logger, &request_headers_modified3);
&request_headers_modified3);
EXPECT_FALSE(headers3.HasHeader("key1")); EXPECT_FALSE(headers3.HasHeader("key1"));
ASSERT_TRUE(headers3.GetHeader("key2", &header_value)); ASSERT_TRUE(headers3.GetHeader("key2", &header_value));
EXPECT_EQ("value 3", header_value); EXPECT_EQ("value 3", header_value);
...@@ -2229,13 +2218,11 @@ TEST(ExtensionWebRequestHelpersTest, ...@@ -2229,13 +2218,11 @@ TEST(ExtensionWebRequestHelpersTest,
} }
deltas.sort(&InDecreasingExtensionInstallationTimeOrder); deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
bool request_headers_modified1; bool request_headers_modified1;
std::set<std::string> ignore1, ignore2;
net::HttpRequestHeaders headers1; net::HttpRequestHeaders headers1;
headers1.MergeFrom(base_headers); headers1.MergeFrom(base_headers);
ignored_actions.clear(); ignored_actions.clear();
MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers1, &ignored_actions, MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers1, &ignored_actions,
&logger, &ignore1, &ignore2, &logger, &request_headers_modified1);
&request_headers_modified1);
EXPECT_TRUE(headers1.HasHeader("Cookie")); EXPECT_TRUE(headers1.HasHeader("Cookie"));
ASSERT_TRUE(headers1.GetHeader("Cookie", &header_value)); ASSERT_TRUE(headers1.GetHeader("Cookie", &header_value));
EXPECT_EQ("name=new value; name2=new value; name4=\"value 4\"", header_value); EXPECT_EQ("name=new value; name2=new value; name4=\"value 4\"", header_value);
......
...@@ -197,22 +197,13 @@ int ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest( ...@@ -197,22 +197,13 @@ int ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest(
return result; return result;
} }
namespace {
void OnHeadersReceivedAdapter(net::CompletionOnceCallback callback,
const std::set<std::string>& removed_headers,
const std::set<std::string>& set_headers,
int error_code) {
std::move(callback).Run(error_code);
}
} // namespace
int ChromeExtensionsNetworkDelegateImpl::OnBeforeStartTransaction( int ChromeExtensionsNetworkDelegateImpl::OnBeforeStartTransaction(
net::URLRequest* request, net::URLRequest* request,
net::CompletionOnceCallback callback, net::CompletionOnceCallback callback,
net::HttpRequestHeaders* headers) { net::HttpRequestHeaders* headers) {
return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders( return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders(
profile_, extension_info_map_.get(), GetWebRequestInfo(request), profile_, extension_info_map_.get(), GetWebRequestInfo(request),
base::BindOnce(OnHeadersReceivedAdapter, std::move(callback)), headers); std::move(callback), headers);
} }
void ChromeExtensionsNetworkDelegateImpl::OnStartTransaction( void ChromeExtensionsNetworkDelegateImpl::OnStartTransaction(
......
...@@ -8,56 +8,6 @@ function getSetCookieUrl(name, value) { ...@@ -8,56 +8,6 @@ function getSetCookieUrl(name, value) {
return getServerURL('set-cookie?' + name + '=' + value); return getServerURL('set-cookie?' + name + '=' + value);
} }
function testModifyHeadersOnRedirect(useExtraHeaders) {
// Use /echoheader instead of observing headers in onSendHeaders to
// ensure we're looking at what the server receives. This avoids bugs in the
// webRequest implementation from being masked.
var finalURL = getServerURL('echoheader?User-Agent&Accept&X-New-Header');
var url = getServerURL('server-redirect?' + finalURL);
var listener = callbackPass(function(details) {
var headers = details.requestHeaders;
// Test modification.
var accept_value;
for (var i = 0; i < headers.length; i++) {
if (headers[i].name.toLowerCase() === 'user-agent') {
headers[i].value = 'foo';
} else if (headers[i].name.toLowerCase() === 'accept') {
accept_value = headers[i].value;
}
}
// Test removal.
chrome.test.assertTrue(accept_value.indexOf('image/webp') >= 0);
removeHeader(headers, 'accept');
// Test addition.
headers.push({name: 'X-New-Header', value: 'Baz'});
return {requestHeaders: headers};
});
var extraInfo = ['requestHeaders', 'blocking'];
if (useExtraHeaders)
extraInfo.push('extraHeaders');
chrome.webRequest.onBeforeSendHeaders.addListener(listener,
{urls: [finalURL]}, extraInfo);
navigateAndWait(url, function(tab) {
chrome.webRequest.onBeforeSendHeaders.removeListener(listener);
chrome.tabs.executeScript(tab.id, {
code: 'document.body.innerText'
}, callbackPass(function(results) {
chrome.test.assertTrue(results[0].indexOf('foo') >= 0,
'User-Agent should be modified.');
chrome.test.assertTrue(results[0].indexOf('image/webp') == -1,
'Accept should be removed.');
chrome.test.assertTrue(results[0].indexOf('Baz') >= 0,
'X-New-Header should be added.');
}));
});
}
runTests([ runTests([
function testSpecialRequestHeadersVisible() { function testSpecialRequestHeadersVisible() {
// Set a cookie so the cookie request header is set. // Set a cookie so the cookie request header is set.
...@@ -250,14 +200,6 @@ runTests([ ...@@ -250,14 +200,6 @@ runTests([
}); });
}, },
function testModifyHeadersOnRedirectWithoutExtraHeaders() {
testModifyHeadersOnRedirect(false);
},
function testModifyHeadersOnRedirectWithExtraHeaders() {
testModifyHeadersOnRedirect(true);
},
// Successful Set-Cookie modification is tested in test_blocking_cookie.js. // Successful Set-Cookie modification is tested in test_blocking_cookie.js.
function testCannotModifySpecialResponseHeadersWithoutExtraHeaders() { function testCannotModifySpecialResponseHeadersWithoutExtraHeaders() {
// Use unique name and value so other tests don't interfere. // Use unique name and value so other tests don't interfere.
......
...@@ -852,20 +852,6 @@ struct ExtensionWebRequestEventRouter::BlockedRequest { ...@@ -852,20 +852,6 @@ struct ExtensionWebRequestEventRouter::BlockedRequest {
// The callback to call when we get a response from all event handlers. // The callback to call when we get a response from all event handlers.
net::CompletionOnceCallback callback; net::CompletionOnceCallback callback;
// The callback to invoke for onBeforeSendHeaders. If
// |before_send_headers_callback.is_null()| is false, |callback| must be NULL.
// Only valid for OnBeforeSendHeaders.
BeforeSendHeadersCallback before_send_headers_callback;
// The callback to invoke for auth. If |auth_callback.is_null()| is false,
// |callback| must be NULL.
// Only valid for OnAuthRequired.
net::NetworkDelegate::AuthCallback auth_callback;
// If non-empty, this contains the auth credentials that may be filled in.
// Only valid for OnAuthRequired.
net::AuthCredentials* auth_credentials = nullptr;
// If non-empty, this contains the new URL that the request will redirect to. // If non-empty, this contains the new URL that the request will redirect to.
// Only valid for OnBeforeRequest and OnHeadersReceived. // Only valid for OnBeforeRequest and OnHeadersReceived.
GURL* new_url = nullptr; GURL* new_url = nullptr;
...@@ -882,6 +868,15 @@ struct ExtensionWebRequestEventRouter::BlockedRequest { ...@@ -882,6 +868,15 @@ struct ExtensionWebRequestEventRouter::BlockedRequest {
// OnHeadersReceived. // OnHeadersReceived.
scoped_refptr<net::HttpResponseHeaders>* override_response_headers = nullptr; scoped_refptr<net::HttpResponseHeaders>* override_response_headers = nullptr;
// If non-empty, this contains the auth credentials that may be filled in.
// Only valid for OnAuthRequired.
net::AuthCredentials* auth_credentials = nullptr;
// The callback to invoke for auth. If |auth_callback.is_null()| is false,
// |callback| must be NULL.
// Only valid for OnAuthRequired.
net::NetworkDelegate::AuthCallback auth_callback;
// Time the request was paused. Used for logging purposes. // Time the request was paused. Used for logging purposes.
base::Time blocking_time; base::Time blocking_time;
...@@ -1095,7 +1090,7 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders( ...@@ -1095,7 +1090,7 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
void* browser_context, void* browser_context,
const InfoMap* extension_info_map, const InfoMap* extension_info_map,
const WebRequestInfo* request, const WebRequestInfo* request,
BeforeSendHeadersCallback callback, net::CompletionOnceCallback callback,
net::HttpRequestHeaders* headers) { net::HttpRequestHeaders* headers) {
if (ShouldHideEvent(browser_context, extension_info_map, *request)) if (ShouldHideEvent(browser_context, extension_info_map, *request))
return net::OK; return net::OK;
...@@ -1133,7 +1128,7 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders( ...@@ -1133,7 +1128,7 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
blocked_request.event = kOnBeforeSendHeaders; blocked_request.event = kOnBeforeSendHeaders;
blocked_request.is_incognito |= IsIncognitoBrowserContext(browser_context); blocked_request.is_incognito |= IsIncognitoBrowserContext(browser_context);
blocked_request.request = request; blocked_request.request = request;
blocked_request.before_send_headers_callback = std::move(callback); blocked_request.callback = std::move(callback);
blocked_request.request_headers = headers; blocked_request.request_headers = headers;
if (blocked_request.num_handlers_blocking == 0) { if (blocked_request.num_handlers_blocking == 0) {
...@@ -2218,9 +2213,6 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context, ...@@ -2218,9 +2213,6 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context,
bool request_headers_modified = false; bool request_headers_modified = false;
bool response_headers_modified = false; bool response_headers_modified = false;
bool credentials_set = false; bool credentials_set = false;
// The set of request headers which were removed or set to new values.
std::set<std::string> request_headers_removed;
std::set<std::string> request_headers_set;
deltas.sort(&helpers::InDecreasingExtensionInstallationTimeOrder); deltas.sort(&helpers::InDecreasingExtensionInstallationTimeOrder);
...@@ -2235,12 +2227,11 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context, ...@@ -2235,12 +2227,11 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context,
request->url, blocked_request.response_deltas, blocked_request.new_url, request->url, blocked_request.response_deltas, blocked_request.new_url,
&ignored_actions, request->logger.get()); &ignored_actions, request->logger.get());
} else if (blocked_request.event == kOnBeforeSendHeaders) { } else if (blocked_request.event == kOnBeforeSendHeaders) {
CHECK(!blocked_request.before_send_headers_callback.is_null()); CHECK(!blocked_request.callback.is_null());
helpers::MergeOnBeforeSendHeadersResponses( helpers::MergeOnBeforeSendHeadersResponses(
request->url, blocked_request.response_deltas, request->url, blocked_request.response_deltas,
blocked_request.request_headers, &ignored_actions, blocked_request.request_headers, &ignored_actions,
request->logger.get(), &request_headers_removed, &request_headers_set, request->logger.get(), &request_headers_modified);
&request_headers_modified);
} else if (blocked_request.event == kOnHeadersReceived) { } else if (blocked_request.event == kOnHeadersReceived) {
CHECK(!blocked_request.callback.is_null()); CHECK(!blocked_request.callback.is_null());
helpers::MergeOnHeadersReceivedResponses( helpers::MergeOnHeadersReceivedResponses(
...@@ -2300,13 +2291,6 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context, ...@@ -2300,13 +2291,6 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context,
blocked_requests_.erase(request->id); blocked_requests_.erase(request->id);
if (call_callback) if (call_callback)
std::move(callback).Run(rv); std::move(callback).Run(rv);
} else if (!blocked_request.before_send_headers_callback.is_null()) {
auto callback = std::move(blocked_request.before_send_headers_callback);
// Ensure that request is removed before callback because the callback
// might trigger the next event.
blocked_requests_.erase(request->id);
if (call_callback)
std::move(callback).Run(request_headers_removed, request_headers_set, rv);
} else if (!blocked_request.auth_callback.is_null()) { } else if (!blocked_request.auth_callback.is_null()) {
net::NetworkDelegate::AuthRequiredResponse response; net::NetworkDelegate::AuthRequiredResponse response;
if (canceled) if (canceled)
......
...@@ -359,11 +359,6 @@ class ExtensionWebRequestEventRouter { ...@@ -359,11 +359,6 @@ class ExtensionWebRequestEventRouter {
GURL* new_url, GURL* new_url,
bool* should_collapse_initiator); bool* should_collapse_initiator);
using BeforeSendHeadersCallback =
base::OnceCallback<void(const std::set<std::string>& removed_headers,
const std::set<std::string>& set_headers,
int error_code)>;
// Dispatches the onBeforeSendHeaders event. This is fired for HTTP(s) // Dispatches the onBeforeSendHeaders event. This is fired for HTTP(s)
// requests only, and allows modification of the outgoing request headers. // requests only, and allows modification of the outgoing request headers.
// Returns net::ERR_IO_PENDING if an extension is intercepting the request, OK // Returns net::ERR_IO_PENDING if an extension is intercepting the request, OK
...@@ -371,7 +366,7 @@ class ExtensionWebRequestEventRouter { ...@@ -371,7 +366,7 @@ class ExtensionWebRequestEventRouter {
int OnBeforeSendHeaders(void* browser_context, int OnBeforeSendHeaders(void* browser_context,
const extensions::InfoMap* extension_info_map, const extensions::InfoMap* extension_info_map,
const WebRequestInfo* request, const WebRequestInfo* request,
BeforeSendHeadersCallback callback, net::CompletionOnceCallback callback,
net::HttpRequestHeaders* headers); net::HttpRequestHeaders* headers);
// Dispatches the onSendHeaders event. This is fired for HTTP(s) requests // Dispatches the onSendHeaders event. This is fired for HTTP(s) requests
......
...@@ -709,14 +709,15 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -709,14 +709,15 @@ void MergeOnBeforeSendHeadersResponses(
net::HttpRequestHeaders* request_headers, net::HttpRequestHeaders* request_headers,
IgnoredActions* ignored_actions, IgnoredActions* ignored_actions,
extensions::WebRequestInfo::Logger* logger, extensions::WebRequestInfo::Logger* logger,
std::set<std::string>* removed_headers,
std::set<std::string>* set_headers,
bool* request_headers_modified) { bool* request_headers_modified) {
DCHECK(request_headers_modified); DCHECK(request_headers_modified);
DCHECK(removed_headers->empty());
DCHECK(set_headers->empty());
*request_headers_modified = false; *request_headers_modified = false;
// Here we collect which headers we have removed or set to new values
// so far due to extensions of higher precedence.
std::set<std::string> removed_headers;
std::set<std::string> set_headers;
// We assume here that the deltas are sorted in decreasing extension // We assume here that the deltas are sorted in decreasing extension
// precedence (i.e. decreasing extension installation time). // precedence (i.e. decreasing extension installation time).
for (const auto& delta : deltas) { for (const auto& delta : deltas) {
...@@ -738,14 +739,14 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -738,14 +739,14 @@ void MergeOnBeforeSendHeadersResponses(
const std::string& value = modification.value(); const std::string& value = modification.value();
// We must not delete anything that has been modified before. // We must not delete anything that has been modified before.
if (removed_headers->find(key) != removed_headers->end() && if (removed_headers.find(key) != removed_headers.end() &&
!extension_conflicts) { !extension_conflicts) {
extension_conflicts = true; extension_conflicts = true;
} }
// We must not modify anything that has been set to a *different* // We must not modify anything that has been set to a *different*
// value before. // value before.
if (set_headers->find(key) != set_headers->end() && if (set_headers.find(key) != set_headers.end() &&
!extension_conflicts) { !extension_conflicts) {
std::string current_value; std::string current_value;
if (!request_headers->GetHeader(key, &current_value) || if (!request_headers->GetHeader(key, &current_value) ||
...@@ -762,7 +763,7 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -762,7 +763,7 @@ void MergeOnBeforeSendHeadersResponses(
for (auto key = delta.deleted_request_headers.begin(); for (auto key = delta.deleted_request_headers.begin();
key != delta.deleted_request_headers.end() && !extension_conflicts; key != delta.deleted_request_headers.end() && !extension_conflicts;
++key) { ++key) {
if (set_headers->find(*key) != set_headers->end()) { if (set_headers.find(*key) != set_headers.end()) {
std::string current_value; std::string current_value;
request_headers->GetHeader(*key, &current_value); request_headers->GetHeader(*key, &current_value);
extension_conflicts = true; extension_conflicts = true;
...@@ -779,14 +780,14 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -779,14 +780,14 @@ void MergeOnBeforeSendHeadersResponses(
net::HttpRequestHeaders::Iterator modification( net::HttpRequestHeaders::Iterator modification(
delta.modified_request_headers); delta.modified_request_headers);
while (modification.GetNext()) while (modification.GetNext())
set_headers->insert(modification.name()); set_headers.insert(modification.name());
} }
// Perform all deletions and record which keys were deleted. // Perform all deletions and record which keys were deleted.
{ {
for (const auto& header : delta.deleted_request_headers) { for (const auto& header : delta.deleted_request_headers) {
request_headers->RemoveHeader(header); request_headers->RemoveHeader(header);
removed_headers->insert(header); removed_headers.insert(header);
} }
} }
logger->LogEvent(net::NetLogEventType::CHROME_EXTENSION_MODIFIED_HEADERS, logger->LogEvent(net::NetLogEventType::CHROME_EXTENSION_MODIFIED_HEADERS,
...@@ -814,7 +815,7 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -814,7 +815,7 @@ void MergeOnBeforeSendHeadersResponses(
{"referer", WebRequestSpecialRequestHeaderModification::kReferer}, {"referer", WebRequestSpecialRequestHeaderModification::kReferer},
}; };
int special_headers_removed = 0; int special_headers_removed = 0;
for (const auto& header : *removed_headers) { for (const auto& header : removed_headers) {
auto it = kHeaderMap.find(base::ToLowerASCII(header)); auto it = kHeaderMap.find(base::ToLowerASCII(header));
if (it != kHeaderMap.end()) { if (it != kHeaderMap.end()) {
special_headers_removed++; special_headers_removed++;
...@@ -830,7 +831,7 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -830,7 +831,7 @@ void MergeOnBeforeSendHeadersResponses(
} }
int special_headers_changed = 0; int special_headers_changed = 0;
for (const auto& header : *set_headers) { for (const auto& header : set_headers) {
auto it = kHeaderMap.find(base::ToLowerASCII(header)); auto it = kHeaderMap.find(base::ToLowerASCII(header));
if (it != kHeaderMap.end()) { if (it != kHeaderMap.end()) {
special_headers_changed++; special_headers_changed++;
......
...@@ -332,8 +332,6 @@ void MergeOnBeforeSendHeadersResponses( ...@@ -332,8 +332,6 @@ void MergeOnBeforeSendHeadersResponses(
net::HttpRequestHeaders* request_headers, net::HttpRequestHeaders* request_headers,
IgnoredActions* ignored_actions, IgnoredActions* ignored_actions,
extensions::WebRequestInfo::Logger* logger, extensions::WebRequestInfo::Logger* logger,
std::set<std::string>* removed_headers,
std::set<std::string>* set_headers,
bool* request_headers_modified); bool* request_headers_modified);
// Modifies the "Set-Cookie" headers in |override_response_headers| according to // Modifies the "Set-Cookie" headers in |override_response_headers| according to
// |deltas.response_cookie_modifications|. If |override_response_headers| is // |deltas.response_cookie_modifications|. If |override_response_headers| is
......
...@@ -26,11 +26,6 @@ ...@@ -26,11 +26,6 @@
namespace extensions { namespace extensions {
WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams::
FollowRedirectParams() = default;
WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams::
~FollowRedirectParams() = default;
WebRequestProxyingURLLoaderFactory::InProgressRequest::InProgressRequest( WebRequestProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
WebRequestProxyingURLLoaderFactory* factory, WebRequestProxyingURLLoaderFactory* factory,
uint64_t request_id, uint64_t request_id,
...@@ -85,12 +80,7 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() { ...@@ -85,12 +80,7 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() {
} }
void WebRequestProxyingURLLoaderFactory::InProgressRequest::Restart() { void WebRequestProxyingURLLoaderFactory::InProgressRequest::Restart() {
UpdateRequestInfo(); request_completed_ = false;
RestartInternal();
}
void WebRequestProxyingURLLoaderFactory::InProgressRequest::
UpdateRequestInfo() {
// Derive a new WebRequestInfo value any time |Restart()| is called, because // Derive a new WebRequestInfo value any time |Restart()| is called, because
// the details in |request_| may have changed e.g. if we've been redirected. // the details in |request_| may have changed e.g. if we've been redirected.
// |request_initiator| can be modified on redirects, but we keep the original // |request_initiator| can be modified on redirects, but we keep the original
...@@ -111,12 +101,6 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest:: ...@@ -111,12 +101,6 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
ExtensionWebRequestEventRouter::GetInstance() ExtensionWebRequestEventRouter::GetInstance()
->HasExtraHeadersListenerForRequest( ->HasExtraHeadersListenerForRequest(
factory_->browser_context_, factory_->info_map_, &info_.value()); factory_->browser_context_, factory_->info_map_, &info_.value());
}
void WebRequestProxyingURLLoaderFactory::InProgressRequest::RestartInternal() {
DCHECK_EQ(info_->url, request_.url)
<< "UpdateRequestInfo must have been called first";
request_completed_ = false;
// If the header client will be used, we start the request immediately, and // If the header client will be used, we start the request immediately, and
// OnBeforeSendHeaders and OnSendHeaders will be handled there. Otherwise, // OnBeforeSendHeaders and OnSendHeaders will be handled there. Otherwise,
...@@ -178,29 +162,11 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirect( ...@@ -178,29 +162,11 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirect(
request_.headers.RemoveHeader(header); request_.headers.RemoveHeader(header);
request_.headers.MergeFrom(modified_headers); request_.headers.MergeFrom(modified_headers);
// Call this before checking |current_request_uses_header_client_| as it
// calculates it.
UpdateRequestInfo();
if (target_loader_.is_bound()) { if (target_loader_.is_bound()) {
// If header_client_ is used, then we have to call FollowRedirect now as target_loader_->FollowRedirect(removed_headers, modified_headers, new_url);
// that's what triggers the network service calling back to
// OnBeforeSendHeaders(). Otherwise, don't call FollowRedirect now. Wait for
// the onBeforeSendHeaders callback(s) to run as these may modify request
// headers and if so we'll pass these modifications to FollowRedirect.
if (current_request_uses_header_client_) {
target_loader_->FollowRedirect(removed_headers, modified_headers,
new_url);
} else {
auto params = std::make_unique<FollowRedirectParams>();
params->removed_headers = removed_headers;
params->modified_headers = modified_headers;
params->new_url = new_url;
pending_follow_redirect_params_ = std::move(params);
}
} }
RestartInternal(); Restart();
} }
void WebRequestProxyingURLLoaderFactory::InProgressRequest:: void WebRequestProxyingURLLoaderFactory::InProgressRequest::
...@@ -481,8 +447,7 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest:: ...@@ -481,8 +447,7 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
DCHECK_EQ(net::OK, result); DCHECK_EQ(net::OK, result);
} }
ContinueToSendHeaders(std::set<std::string>(), std::set<std::string>(), ContinueToSendHeaders(net::OK);
net::OK);
} }
void WebRequestProxyingURLLoaderFactory::InProgressRequest:: void WebRequestProxyingURLLoaderFactory::InProgressRequest::
...@@ -525,9 +490,7 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest:: ...@@ -525,9 +490,7 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
} }
void WebRequestProxyingURLLoaderFactory::InProgressRequest:: void WebRequestProxyingURLLoaderFactory::InProgressRequest::
ContinueToSendHeaders(const std::set<std::string>& removed_headers, ContinueToSendHeaders(int error_code) {
const std::set<std::string>& set_headers,
int error_code) {
if (error_code != net::OK) { if (error_code != net::OK) {
OnRequestError(network::URLLoaderCompletionStatus(error_code)); OnRequestError(network::URLLoaderCompletionStatus(error_code));
return; return;
...@@ -537,26 +500,6 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest:: ...@@ -537,26 +500,6 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
DCHECK(on_before_send_headers_callback_); DCHECK(on_before_send_headers_callback_);
std::move(on_before_send_headers_callback_) std::move(on_before_send_headers_callback_)
.Run(error_code, request_.headers); .Run(error_code, request_.headers);
} else if (pending_follow_redirect_params_) {
pending_follow_redirect_params_->removed_headers.insert(
pending_follow_redirect_params_->removed_headers.end(),
removed_headers.begin(), removed_headers.end());
for (auto& set_header : set_headers) {
std::string header_value;
if (request_.headers.GetHeader(set_header, &header_value)) {
pending_follow_redirect_params_->modified_headers.SetHeader(
set_header, header_value);
} else {
NOTREACHED();
}
}
target_loader_->FollowRedirect(
pending_follow_redirect_params_->removed_headers,
pending_follow_redirect_params_->modified_headers,
pending_follow_redirect_params_->new_url);
pending_follow_redirect_params_.reset();
} }
if (proxied_client_binding_.is_bound()) if (proxied_client_binding_.is_bound())
......
...@@ -103,14 +103,8 @@ class WebRequestProxyingURLLoaderFactory ...@@ -103,14 +103,8 @@ class WebRequestProxyingURLLoaderFactory
OnHeadersReceivedCallback callback) override; OnHeadersReceivedCallback callback) override;
private: private:
// These two methods combined form the implementation of Restart().
void UpdateRequestInfo();
void RestartInternal();
void ContinueToBeforeSendHeaders(int error_code); void ContinueToBeforeSendHeaders(int error_code);
void ContinueToSendHeaders(const std::set<std::string>& removed_headers, void ContinueToSendHeaders(int error_code);
const std::set<std::string>& set_headers,
int error_code);
void ContinueToStartRequest(int error_code); void ContinueToStartRequest(int error_code);
void ContinueToHandleOverrideHeaders(int error_code); void ContinueToHandleOverrideHeaders(int error_code);
void ContinueToResponseStarted(int error_code); void ContinueToResponseStarted(int error_code);
...@@ -174,21 +168,6 @@ class WebRequestProxyingURLLoaderFactory ...@@ -174,21 +168,6 @@ class WebRequestProxyingURLLoaderFactory
OnHeadersReceivedCallback on_headers_received_callback_; OnHeadersReceivedCallback on_headers_received_callback_;
mojo::Binding<network::mojom::TrustedHeaderClient> header_client_binding_; mojo::Binding<network::mojom::TrustedHeaderClient> header_client_binding_;
// If |has_any_extra_headers_listeners_| is set to false and a redirect is
// in progress, this stores the parameters to FollowRedirect that came from
// the client. That way we can combine it with any other changes that
// extensions made to headers in their callbacks.
struct FollowRedirectParams {
FollowRedirectParams();
~FollowRedirectParams();
std::vector<std::string> removed_headers;
net::HttpRequestHeaders modified_headers;
base::Optional<GURL> new_url;
DISALLOW_COPY_AND_ASSIGN(FollowRedirectParams);
};
std::unique_ptr<FollowRedirectParams> pending_follow_redirect_params_;
base::WeakPtrFactory<InProgressRequest> weak_factory_; base::WeakPtrFactory<InProgressRequest> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(InProgressRequest); DISALLOW_COPY_AND_ASSIGN(InProgressRequest);
......
...@@ -379,14 +379,10 @@ void WebRequestProxyingWebSocket::OnBeforeRequestComplete(int error_code) { ...@@ -379,14 +379,10 @@ void WebRequestProxyingWebSocket::OnBeforeRequestComplete(int error_code) {
return; return;
DCHECK_EQ(net::OK, result); DCHECK_EQ(net::OK, result);
OnBeforeSendHeadersComplete(std::set<std::string>(), std::set<std::string>(), OnBeforeSendHeadersComplete(net::OK);
net::OK);
} }
void WebRequestProxyingWebSocket::OnBeforeSendHeadersComplete( void WebRequestProxyingWebSocket::OnBeforeSendHeadersComplete(int error_code) {
const std::set<std::string>& removed_headers,
const std::set<std::string>& set_headers,
int error_code) {
DCHECK(binding_as_header_client_ || !binding_as_client_.is_bound()); DCHECK(binding_as_header_client_ || !binding_as_client_.is_bound());
if (error_code != net::OK) { if (error_code != net::OK) {
OnError(error_code); OnError(error_code);
......
...@@ -112,9 +112,7 @@ class WebRequestProxyingWebSocket ...@@ -112,9 +112,7 @@ class WebRequestProxyingWebSocket
private: private:
void OnBeforeRequestComplete(int error_code); void OnBeforeRequestComplete(int error_code);
void OnBeforeSendHeadersComplete(const std::set<std::string>& removed_headers, void OnBeforeSendHeadersComplete(int error_code);
const std::set<std::string>& set_headers,
int error_code);
void ContinueToStartRequest(int error_code); void ContinueToStartRequest(int error_code);
void OnHeadersReceivedComplete(int error_code); void OnHeadersReceivedComplete(int error_code);
void ContinueToHeadersReceived(); void ContinueToHeadersReceived();
......
...@@ -70,22 +70,13 @@ int ShellNetworkDelegate::OnBeforeURLRequest( ...@@ -70,22 +70,13 @@ int ShellNetworkDelegate::OnBeforeURLRequest(
return result; return result;
} }
namespace {
void OnHeadersReceivedAdapter(net::CompletionOnceCallback callback,
const std::set<std::string>& removed_headers,
const std::set<std::string>& set_headers,
int error_code) {
std::move(callback).Run(error_code);
}
} // namespace
int ShellNetworkDelegate::OnBeforeStartTransaction( int ShellNetworkDelegate::OnBeforeStartTransaction(
net::URLRequest* request, net::URLRequest* request,
net::CompletionOnceCallback callback, net::CompletionOnceCallback callback,
net::HttpRequestHeaders* headers) { net::HttpRequestHeaders* headers) {
return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders( return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders(
browser_context_, extension_info_map_.get(), GetWebRequestInfo(request), browser_context_, extension_info_map_.get(), GetWebRequestInfo(request),
base::BindOnce(OnHeadersReceivedAdapter, std::move(callback)), headers); std::move(callback), headers);
} }
void ShellNetworkDelegate::OnStartTransaction( void ShellNetworkDelegate::OnStartTransaction(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment