Skip to content

Commit

Permalink
Fix WebRequestProxyingURLLoaderFactory crash after redirect
Browse files Browse the repository at this point in the history
When OnReceiveRedirect() is called, we expect to not receive any more
events for that request until FollowRedirect() has been called and the
request is restarted with the new URL. We were not clearing the
bindings in this case, so the URLLoader was staying alive and calling
OnBeforeSendHeaders() on the redirected URL. This could then result in
OnBeforeSendHeaders() being called twice, once for the original request
which gets redirected in //net, and once for the request in the correct
flow when it gets restarted after FollowRedirect().

I believe this is pretty similar to the issue described in
http://crbug.com/882661, except this one has to do with normal
redirects instead of webRequest generated redirects.

The existing redirect tests catch this issue when using the
'extraHeaders' spec, so I made a version of them that runs with that
enabled.

Bug: 918761
Change-Id: Ifa551400e85c13ea3a297fba02a05deb5ccb1821
Reviewed-on: https://chromium-review.googlesource.com/c/1409915
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622685}
  • Loading branch information
clarkduvall authored and Commit Bot committed Jan 15, 2019
1 parent 661e522 commit 066fecf
Show file tree
Hide file tree
Showing 4 changed files with 362 additions and 346 deletions.
16 changes: 16 additions & 0 deletions chrome/browser/extensions/api/web_request/web_request_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestRedirects) {
<< message_;
}

IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestRedirectsWithExtraHeaders) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(RunExtensionSubtestWithArg("webrequest", "test_redirects.html",
"useExtraHeaders"))
<< message_;
}

// Tests that redirects from secure to insecure don't send the referrer header.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestRedirectsToInsecure) {
Expand Down Expand Up @@ -588,6 +596,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
<< message_;
}

IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestSubresourceRedirectsWithExtraHeaders) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(RunExtensionSubtestWithArg(
"webrequest", "test_subresource_redirects.html", "useExtraHeaders"))
<< message_;
}

// Fails often on Windows dbg bots. http://crbug.com/177163
#if defined(OS_WIN)
#define MAYBE_WebRequestNewTab DISABLED_WebRequestNewTab
Expand Down

0 comments on commit 066fecf

Please sign in to comment.