diff options
| author | rob@robwu.nl <rob@robwu.nl@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-26 10:41:16 +0000 |
|---|---|---|
| committer | rob@robwu.nl <rob@robwu.nl@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-26 10:41:16 +0000 |
| commit | 5f71413b4cefc6db3f111b2da1681266e3c056ec (patch) | |
| tree | f137d56cdc889d2d411c55bbe59a402abbff95b0 | |
| parent | 1615f14112858597ccff27f6acab09be19dc40d7 (diff) | |
| download | chromium_src-5f71413b4cefc6db3f111b2da1681266e3c056ec.zip chromium_src-5f71413b4cefc6db3f111b2da1681266e3c056ec.tar.gz chromium_src-5f71413b4cefc6db3f111b2da1681266e3c056ec.tar.bz2 | |
Support redirectUrl at onHeadersReceived in WebRequest / DWR API
Add support for extension-initiated redirects via the (declarative) Web Request API.
The existing URL validation and conflict resolution logic of redirectUrl at onBeforeRequest is re-used for the implementation of redirectUrl at onHeadersReceived.
To make sure that redirects to data:// and chrome-extension:// URLs are not blocked, a new parameter has been added to the network delegate (allowed_unsafe_redirect_url).
BUG=280464,115940
TEST=browser_tests: ExtensionWebRequestApiTest.WebRequestBlocking:ExtensionWebRequestApiTest.WebRequestDeclarative1
unit_tests: ExtensionWebRequestTest.*:WebRequestActionWithThreadsTest.*
net_unittests: URLRequestTestHTTP.NetworkDelegateRedirectRequestOnHeadersReceived*: URLRequestTestHTTP.UnsafeRedirect*: URLRequestTestHTTP.*ReferenceFragment*
Review URL: https://codereview.chromium.org/154473002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259546 0039d316-1c4b-4281-b951-d872f2087c98
30 files changed, 609 insertions, 93 deletions
diff --git a/android_webview/browser/net/aw_network_delegate.cc b/android_webview/browser/net/aw_network_delegate.cc index c5aef1f..a18226c 100644 --- a/android_webview/browser/net/aw_network_delegate.cc +++ b/android_webview/browser/net/aw_network_delegate.cc @@ -45,7 +45,8 @@ int AwNetworkDelegate::OnHeadersReceived( net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) { + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { return net::OK; } diff --git a/android_webview/browser/net/aw_network_delegate.h b/android_webview/browser/net/aw_network_delegate.h index 6d344698..7b78dda 100644 --- a/android_webview/browser/net/aw_network_delegate.h +++ b/android_webview/browser/net/aw_network_delegate.h @@ -30,8 +30,8 @@ class AwNetworkDelegate : public net::NetworkDelegate { net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) - OVERRIDE; + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE; virtual void OnBeforeRedirect(net::URLRequest* request, const GURL& new_location) OVERRIDE; virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE; diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_action.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_action.cc index 8731d28..739d074 100644 --- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_action.cc +++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_action.cc @@ -582,7 +582,7 @@ LinkedPtrEventResponseDelta WebRequestCancelAction::CreateDelta( // WebRequestRedirectAction::WebRequestRedirectAction(const GURL& redirect_url) - : WebRequestAction(ON_BEFORE_REQUEST, + : WebRequestAction(ON_BEFORE_REQUEST | ON_HEADERS_RECEIVED, ACTION_REDIRECT_REQUEST, std::numeric_limits<int>::min(), STRATEGY_DEFAULT), @@ -619,7 +619,7 @@ LinkedPtrEventResponseDelta WebRequestRedirectAction::CreateDelta( WebRequestRedirectToTransparentImageAction:: WebRequestRedirectToTransparentImageAction() - : WebRequestAction(ON_BEFORE_REQUEST, + : WebRequestAction(ON_BEFORE_REQUEST | ON_HEADERS_RECEIVED, ACTION_REDIRECT_TO_TRANSPARENT_IMAGE, std::numeric_limits<int>::min(), STRATEGY_NONE) {} @@ -649,7 +649,7 @@ WebRequestRedirectToTransparentImageAction::CreateDelta( WebRequestRedirectToEmptyDocumentAction:: WebRequestRedirectToEmptyDocumentAction() - : WebRequestAction(ON_BEFORE_REQUEST, + : WebRequestAction(ON_BEFORE_REQUEST | ON_HEADERS_RECEIVED, ACTION_REDIRECT_TO_EMPTY_DOCUMENT, std::numeric_limits<int>::min(), STRATEGY_NONE) {} @@ -680,7 +680,7 @@ WebRequestRedirectToEmptyDocumentAction::CreateDelta( WebRequestRedirectByRegExAction::WebRequestRedirectByRegExAction( scoped_ptr<RE2> from_pattern, const std::string& to_pattern) - : WebRequestAction(ON_BEFORE_REQUEST, + : WebRequestAction(ON_BEFORE_REQUEST | ON_HEADERS_RECEIVED, ACTION_REDIRECT_BY_REGEX_DOCUMENT, std::numeric_limits<int>::min(), STRATEGY_DEFAULT), diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_action_unittest.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_action_unittest.cc index 4952dc8..fbb7aa6 100644 --- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_action_unittest.cc +++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_action_unittest.cc @@ -279,6 +279,7 @@ TEST_F(WebRequestActionWithThreadsTest, PermissionsToRedirect) { " \"redirectUrl\": \"http://www.foobar.com\"" "}]"; CheckActionNeedsAllUrls(kAction, ON_BEFORE_REQUEST); + CheckActionNeedsAllUrls(kAction, ON_HEADERS_RECEIVED); } TEST_F(WebRequestActionWithThreadsTest, PermissionsToRedirectByRegEx) { @@ -442,6 +443,10 @@ TEST_F(WebRequestActionWithThreadsTest, extension_->id(), action_set.get(), ON_BEFORE_REQUEST)); + EXPECT_TRUE(ActionWorksOnRequest("http://test.org", + extension_->id(), + action_set.get(), + ON_HEADERS_RECEIVED)); } TEST_F(WebRequestActionWithThreadsTest, PermissionsToRedirectToEmptyDocument) { @@ -456,6 +461,10 @@ TEST_F(WebRequestActionWithThreadsTest, PermissionsToRedirectToEmptyDocument) { extension_->id(), action_set.get(), ON_BEFORE_REQUEST)); + EXPECT_TRUE(ActionWorksOnRequest("http://test.org", + extension_->id(), + action_set.get(), + ON_HEADERS_RECEIVED)); } TEST_F(WebRequestActionWithThreadsTest, PermissionsToIgnore) { diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry_unittest.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry_unittest.cc index 7ef5524..0efc927 100644 --- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry_unittest.cc +++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry_unittest.cc @@ -667,26 +667,28 @@ TEST_F(WebRequestRulesRegistryTest, GetMatchesDifferentUrls) { TEST(WebRequestRulesRegistrySimpleTest, StageChecker) { // The contentType condition can only be evaluated during ON_HEADERS_RECEIVED - // but the redirect action can only be executed during ON_BEFORE_REQUEST. + // but the SetRequestHeader action can only be executed during + // ON_BEFORE_SEND_HEADERS. // Therefore, this is an inconsistent rule that needs to be flagged. const char kRule[] = - "{ \n" - " \"id\": \"rule1\", \n" - " \"conditions\": [ \n" - " { \n" - " \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n" - " \"url\": {\"hostSuffix\": \"foo.com\"}, \n" - " \"contentType\": [\"image/jpeg\"] \n" - " } \n" - " ], \n" - " \"actions\": [ \n" - " { \n" - " \"instanceType\": \"declarativeWebRequest.RedirectRequest\",\n" - " \"redirectUrl\": \"http://bar.com\" \n" - " } \n" - " ], \n" - " \"priority\": 200 \n" - "} "; + "{ \n" + " \"id\": \"rule1\", \n" + " \"conditions\": [ \n" + " { \n" + " \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n" + " \"url\": {\"hostSuffix\": \"foo.com\"}, \n" + " \"contentType\": [\"image/jpeg\"] \n" + " } \n" + " ], \n" + " \"actions\": [ \n" + " { \n" + " \"instanceType\": \"declarativeWebRequest.SetRequestHeader\",\n" + " \"name\": \"Content-Type\", \n" + " \"value\": \"text/plain\" \n" + " } \n" + " ], \n" + " \"priority\": 200 \n" + "} "; scoped_ptr<base::Value> value(base::JSONReader::Read(kRule)); ASSERT_TRUE(value); diff --git a/chrome/browser/extensions/api/web_request/web_request_api.cc b/chrome/browser/extensions/api/web_request/web_request_api.cc index dc884f8..e109569 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api.cc +++ b/chrome/browser/extensions/api/web_request/web_request_api.cc @@ -498,7 +498,7 @@ struct ExtensionWebRequestEventRouter::BlockedRequest { net::CompletionCallback callback; // If non-empty, this contains the new URL that the request will redirect to. - // Only valid for OnBeforeRequest. + // Only valid for OnBeforeRequest and OnHeadersReceived. GURL* new_url; // The request headers that will be issued along with this request. Only valid @@ -831,7 +831,8 @@ int ExtensionWebRequestEventRouter::OnHeadersReceived( net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) { + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { // We hide events from the system context as well as sensitive requests. if (!profile || WebRequestPermissions::HideRequest(extension_info_map, request)) @@ -881,6 +882,8 @@ int ExtensionWebRequestEventRouter::OnHeadersReceived( override_response_headers; blocked_requests_[request->identifier()].original_response_headers = original_response_headers; + blocked_requests_[request->identifier()].new_url = + allowed_unsafe_redirect_url; if (blocked_requests_[request->identifier()].num_handlers_blocking == 0) { // If there are no blocking handlers, only the declarative rules tried @@ -1557,8 +1560,12 @@ helpers::EventResponseDelta* CalculateDelta( helpers::ResponseHeaders* new_headers = response->response_headers.get(); return helpers::CalculateOnHeadersReceivedDelta( - response->extension_id, response->extension_install_time, - response->cancel, old_headers, new_headers); + response->extension_id, + response->extension_install_time, + response->cancel, + response->new_url, + old_headers, + new_headers); } case ExtensionWebRequestEventRouter::kOnAuthRequired: return helpers::CalculateOnAuthRequiredDelta( @@ -1848,6 +1855,7 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas( blocked_request.response_deltas, blocked_request.original_response_headers.get(), blocked_request.override_response_headers, + blocked_request.new_url, &warnings, blocked_request.net_log); } else if (blocked_request.event == kOnAuthRequired) { diff --git a/chrome/browser/extensions/api/web_request/web_request_api.h b/chrome/browser/extensions/api/web_request/web_request_api.h index 9efddb0..9ae447d 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api.h +++ b/chrome/browser/extensions/api/web_request/web_request_api.h @@ -205,9 +205,9 @@ class ExtensionWebRequestEventRouter // requests only, and allows modification of incoming response headers. // Returns net::ERR_IO_PENDING if an extension is intercepting the request, // OK otherwise. |original_response_headers| is reference counted. |callback| - // and |override_response_headers| are owned by a URLRequestJob. They are - // guaranteed to be valid until |callback| is called or OnURLRequestDestroyed - // is called (whatever comes first). + // |override_response_headers| and |allowed_unsafe_redirect_url| are owned by + // a URLRequestJob. They are guaranteed to be valid until |callback| is called + // or OnURLRequestDestroyed is called (whatever comes first). // Do not modify |original_response_headers| directly but write new ones // into |override_response_headers|. int OnHeadersReceived( @@ -216,7 +216,8 @@ class ExtensionWebRequestEventRouter net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers); + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url); // Dispatches the OnAuthRequired event to any extensions whose filters match // the given request. If the listener is not registered as "blocking", then diff --git a/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc b/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc index c48bae2..d3619de 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc +++ b/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc @@ -324,11 +324,13 @@ EventResponseDelta* CalculateOnHeadersReceivedDelta( const std::string& extension_id, const base::Time& extension_install_time, bool cancel, + const GURL& new_url, const net::HttpResponseHeaders* old_response_headers, ResponseHeaders* new_response_headers) { EventResponseDelta* result = new EventResponseDelta(extension_id, extension_install_time); result->cancel = cancel; + result->new_url = new_url; if (!new_response_headers) return result; @@ -403,14 +405,14 @@ void MergeCancelOfResponses( } } -// Helper function for MergeOnBeforeRequestResponses() that allows ignoring +// Helper function for MergeRedirectUrlOfResponses() that allows ignoring // all redirects but those to data:// urls and about:blank. This is important // to treat these URLs as "cancel urls", i.e. URLs that extensions redirect // to if they want to express that they want to cancel a request. This reduces // the number of conflicts that we need to flag, as canceling is considered // a higher precedence operation that redirects. // Returns whether a redirect occurred. -static bool MergeOnBeforeRequestResponsesHelper( +static bool MergeRedirectUrlOfResponsesHelper( const EventResponseDeltas& deltas, GURL* new_url, extensions::ExtensionWarningSet* conflicting_extensions, @@ -452,7 +454,7 @@ static bool MergeOnBeforeRequestResponsesHelper( return redirected; } -void MergeOnBeforeRequestResponses( +void MergeRedirectUrlOfResponses( const EventResponseDeltas& deltas, GURL* new_url, extensions::ExtensionWarningSet* conflicting_extensions, @@ -460,7 +462,7 @@ void MergeOnBeforeRequestResponses( // First handle only redirects to data:// URLs and about:blank. These are a // special case as they represent a way of cancelling a request. - if (MergeOnBeforeRequestResponsesHelper( + if (MergeRedirectUrlOfResponsesHelper( deltas, new_url, conflicting_extensions, net_log, true)) { // If any extension cancelled a request by redirecting to a data:// URL or // about:blank, we don't consider the other redirects. @@ -468,10 +470,18 @@ void MergeOnBeforeRequestResponses( } // Handle all other redirects. - MergeOnBeforeRequestResponsesHelper( + MergeRedirectUrlOfResponsesHelper( deltas, new_url, conflicting_extensions, net_log, false); } +void MergeOnBeforeRequestResponses( + const EventResponseDeltas& deltas, + GURL* new_url, + extensions::ExtensionWarningSet* conflicting_extensions, + const net::BoundNetLog* net_log) { + MergeRedirectUrlOfResponses(deltas, new_url, conflicting_extensions, net_log); +} + // Assumes that |header_value| is the cookie header value of a HTTP Request // following the cookie-string schema of RFC 6265, section 4.2.1, and returns // cookie name/value pairs. If cookie values are presented in double quotes, @@ -1100,6 +1110,7 @@ void MergeOnHeadersReceivedResponses( const EventResponseDeltas& deltas, const net::HttpResponseHeaders* original_response_headers, scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url, extensions::ExtensionWarningSet* conflicting_extensions, const net::BoundNetLog* net_log) { EventResponseDeltas::const_iterator delta; @@ -1181,6 +1192,23 @@ void MergeOnHeadersReceivedResponses( MergeCookiesInOnHeadersReceivedResponses(deltas, original_response_headers, override_response_headers, conflicting_extensions, net_log); + + GURL new_url; + MergeRedirectUrlOfResponses( + deltas, &new_url, conflicting_extensions, net_log); + if (new_url.is_valid()) { + // Only create a copy if we really want to modify the response headers. + if (override_response_headers->get() == NULL) { + *override_response_headers = new net::HttpResponseHeaders( + original_response_headers->raw_headers()); + } + (*override_response_headers)->ReplaceStatusLine("HTTP/1.1 302 Found"); + (*override_response_headers)->RemoveHeader("location"); + (*override_response_headers)->AddHeader("Location: " + new_url.spec()); + // Explicitly mark the URL as safe for redirection, to prevent the request + // from being blocked because of net::ERR_UNSAFE_REDIRECT. + *allowed_unsafe_redirect_url = new_url; + } } bool MergeOnAuthRequiredResponses( diff --git a/chrome/browser/extensions/api/web_request/web_request_api_helpers.h b/chrome/browser/extensions/api/web_request/web_request_api_helpers.h index bbaf846..5901d22 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api_helpers.h +++ b/chrome/browser/extensions/api/web_request/web_request_api_helpers.h @@ -214,6 +214,7 @@ EventResponseDelta* CalculateOnHeadersReceivedDelta( const std::string& extension_id, const base::Time& extension_install_time, bool cancel, + const GURL& new_url, const net::HttpResponseHeaders* old_response_headers, ResponseHeaders* new_response_headers); // Destructively moves the auth credentials from |auth_credentials| to the @@ -238,6 +239,14 @@ void MergeCancelOfResponses( // Stores in |*new_url| the redirect request of the extension with highest // precedence. Extensions that did not command to redirect the request are // ignored in this logic. +void MergeRedirectUrlOfResponses( + const EventResponseDeltas& deltas, + GURL* new_url, + extensions::ExtensionWarningSet* conflicting_extensions, + const net::BoundNetLog* net_log); +// Stores in |*new_url| the redirect request of the extension with highest +// precedence. Extensions that did not command to redirect the request are +// ignored in this logic. void MergeOnBeforeRequestResponses( const EventResponseDeltas& deltas, GURL* new_url, @@ -271,10 +280,14 @@ void MergeCookiesInOnHeadersReceivedResponses( // Stores a copy of |original_response_header| into |override_response_headers| // that is modified according to |deltas|. If |deltas| does not instruct to // modify the response headers, |override_response_headers| remains empty. +// Extension-initiated redirects are written to |override_response_headers| +// (to request redirection) and |*allowed_unsafe_redirect_url| (to make sure +// that the request is not cancelled with net::ERR_UNSAFE_REDIRECT). void MergeOnHeadersReceivedResponses( const EventResponseDeltas& deltas, const net::HttpResponseHeaders* original_response_headers, scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url, extensions::ExtensionWarningSet* conflicting_extensions, const net::BoundNetLog* net_log); // Merge the responses of blocked onAuthRequired handlers. The first diff --git a/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc b/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc index cafc124..eaeece0 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc +++ b/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc @@ -1274,9 +1274,15 @@ TEST(ExtensionWebRequestHelpersTest, TestCalculateOnHeadersReceivedDelta) { new_headers.push_back(ResponseHeader("Key2", "Value1")); // Modified // Key3 is deleted new_headers.push_back(ResponseHeader("Key4", "Value4")); // Added + GURL effective_new_url; - scoped_ptr<EventResponseDelta> delta(CalculateOnHeadersReceivedDelta( - "extid", base::Time::Now(), cancel, base_headers.get(), &new_headers)); + scoped_ptr<EventResponseDelta> delta( + CalculateOnHeadersReceivedDelta("extid", + base::Time::Now(), + cancel, + effective_new_url, + base_headers.get(), + &new_headers)); ASSERT_TRUE(delta.get()); EXPECT_TRUE(delta->cancel); EXPECT_EQ(2u, delta->added_response_headers.size()); @@ -1959,9 +1965,15 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnHeadersReceivedResponses) { new EventResponseDelta("extid0", base::Time::FromInternalValue(3000))); deltas.push_back(d0); scoped_refptr<net::HttpResponseHeaders> new_headers0; - MergeOnHeadersReceivedResponses(deltas, base_headers.get(), &new_headers0, - &warning_set, &net_log); + GURL allowed_unsafe_redirect_url0; + MergeOnHeadersReceivedResponses(deltas, + base_headers.get(), + &new_headers0, + &allowed_unsafe_redirect_url0, + &warning_set, + &net_log); EXPECT_FALSE(new_headers0.get()); + EXPECT_TRUE(allowed_unsafe_redirect_url0.is_empty()); EXPECT_EQ(0u, warning_set.size()); EXPECT_EQ(0u, capturing_net_log.GetSize()); @@ -1975,9 +1987,15 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnHeadersReceivedResponses) { warning_set.clear(); capturing_net_log.Clear(); scoped_refptr<net::HttpResponseHeaders> new_headers1; - MergeOnHeadersReceivedResponses( - deltas, base_headers.get(), &new_headers1, &warning_set, &net_log); + GURL allowed_unsafe_redirect_url1; + MergeOnHeadersReceivedResponses(deltas, + base_headers.get(), + &new_headers1, + &allowed_unsafe_redirect_url1, + &warning_set, + &net_log); ASSERT_TRUE(new_headers1.get()); + EXPECT_TRUE(allowed_unsafe_redirect_url1.is_empty()); std::multimap<std::string, std::string> expected1; expected1.insert(std::pair<std::string, std::string>("Key2", "Value3")); void* iter = NULL; @@ -2003,9 +2021,15 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnHeadersReceivedResponses) { warning_set.clear(); capturing_net_log.Clear(); scoped_refptr<net::HttpResponseHeaders> new_headers2; - MergeOnHeadersReceivedResponses( - deltas, base_headers.get(), &new_headers2, &warning_set, &net_log); + GURL allowed_unsafe_redirect_url2; + MergeOnHeadersReceivedResponses(deltas, + base_headers.get(), + &new_headers2, + &allowed_unsafe_redirect_url2, + &warning_set, + &net_log); ASSERT_TRUE(new_headers2.get()); + EXPECT_TRUE(allowed_unsafe_redirect_url2.is_empty()); iter = NULL; std::multimap<std::string, std::string> actual2; while (new_headers2->EnumerateHeaderLines(&iter, &name, &value)) { @@ -2043,9 +2067,15 @@ TEST(ExtensionWebRequestHelpersTest, d1->deleted_response_headers.push_back(ResponseHeader("KEY1", "Value2")); deltas.push_back(d1); scoped_refptr<net::HttpResponseHeaders> new_headers1; - MergeOnHeadersReceivedResponses( - deltas, base_headers.get(), &new_headers1, &warning_set, &net_log); + GURL allowed_unsafe_redirect_url1; + MergeOnHeadersReceivedResponses(deltas, + base_headers.get(), + &new_headers1, + &allowed_unsafe_redirect_url1, + &warning_set, + &net_log); ASSERT_TRUE(new_headers1.get()); + EXPECT_TRUE(allowed_unsafe_redirect_url1.is_empty()); std::multimap<std::string, std::string> expected1; expected1.insert(std::pair<std::string, std::string>("Key1", "Value1")); expected1.insert(std::pair<std::string, std::string>("Key1", "Value3")); @@ -2062,6 +2092,64 @@ TEST(ExtensionWebRequestHelpersTest, EXPECT_EQ(1u, capturing_net_log.GetSize()); } +// Tests whether onHeadersReceived can initiate a redirect. +// The URL merge logic is shared with onBeforeRequest, so we only need to test +// whether the URLs are merged at all. +TEST(ExtensionWebRequestHelpersTest, + TestMergeOnHeadersReceivedResponsesRedirect) { + EventResponseDeltas deltas; + net::CapturingBoundNetLog capturing_net_log; + net::BoundNetLog net_log = capturing_net_log.bound(); + ExtensionWarningSet warning_set; + + char base_headers_string[] = + "HTTP/1.0 200 OK\r\n" + "\r\n"; + scoped_refptr<net::HttpResponseHeaders> base_headers( + new net::HttpResponseHeaders(net::HttpUtil::AssembleRawHeaders( + base_headers_string, sizeof(base_headers_string)))); + + // No redirect + linked_ptr<EventResponseDelta> d0( + new EventResponseDelta("extid0", base::Time::FromInternalValue(0))); + deltas.push_back(d0); + scoped_refptr<net::HttpResponseHeaders> new_headers0; + GURL allowed_unsafe_redirect_url0; + MergeOnHeadersReceivedResponses(deltas, + base_headers.get(), + &new_headers0, + &allowed_unsafe_redirect_url0, + &warning_set, + &net_log); + EXPECT_FALSE(new_headers0.get()); + EXPECT_TRUE(allowed_unsafe_redirect_url0.is_empty()); + EXPECT_EQ(0u, warning_set.size()); + EXPECT_EQ(0u, capturing_net_log.GetSize()); + + // Single redirect. + GURL new_url_1("http://foo.com"); + linked_ptr<EventResponseDelta> d1( + new EventResponseDelta("extid1", base::Time::FromInternalValue(1000))); + d1->new_url = GURL(new_url_1); + deltas.push_back(d1); + deltas.sort(&InDecreasingExtensionInstallationTimeOrder); + capturing_net_log.Clear(); + scoped_refptr<net::HttpResponseHeaders> new_headers1; + GURL allowed_unsafe_redirect_url1; + MergeOnHeadersReceivedResponses(deltas, + base_headers.get(), + &new_headers1, + &allowed_unsafe_redirect_url1, + &warning_set, + &net_log); + + EXPECT_TRUE(new_headers1.get()); + EXPECT_TRUE(new_headers1->HasHeaderValue("Location", new_url_1.spec())); + EXPECT_EQ(new_url_1, allowed_unsafe_redirect_url1); + EXPECT_TRUE(warning_set.empty()); + EXPECT_EQ(1u, capturing_net_log.GetSize()); +} + TEST(ExtensionWebRequestHelpersTest, TestMergeOnAuthRequiredResponses) { net::CapturingBoundNetLog capturing_net_log; net::BoundNetLog net_log = capturing_net_log.bound(); diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc index 2e9d3e7..b590367 100644 --- a/chrome/browser/net/chrome_network_delegate.cc +++ b/chrome/browser/net/chrome_network_delegate.cc @@ -512,10 +512,16 @@ int ChromeNetworkDelegate::OnHeadersReceived( net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) { + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { return ExtensionWebRequestEventRouter::GetInstance()->OnHeadersReceived( - profile_, extension_info_map_.get(), request, callback, - original_response_headers, override_response_headers); + profile_, + extension_info_map_.get(), + request, + callback, + original_response_headers, + override_response_headers, + allowed_unsafe_redirect_url); } void ChromeNetworkDelegate::OnBeforeRedirect(net::URLRequest* request, diff --git a/chrome/browser/net/chrome_network_delegate.h b/chrome/browser/net/chrome_network_delegate.h index 6adf283..d6ecc7e 100644 --- a/chrome/browser/net/chrome_network_delegate.h +++ b/chrome/browser/net/chrome_network_delegate.h @@ -151,8 +151,8 @@ class ChromeNetworkDelegate : public net::NetworkDelegate { net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) - OVERRIDE; + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE; virtual void OnBeforeRedirect(net::URLRequest* request, const GURL& new_location) OVERRIDE; virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE; diff --git a/chrome/common/extensions/api/web_request.json b/chrome/common/extensions/api/web_request.json index 51a161e..5d279c7 100644 --- a/chrome/common/extensions/api/web_request.json +++ b/chrome/common/extensions/api/web_request.json @@ -66,7 +66,7 @@ "redirectUrl": { "type": "string", "optional": true, - "description": "Only used as a response to the onBeforeRequest event. If set, the original request is prevented from being sent and is instead redirected to the given URL." + "description": "Only used as a response to the onBeforeRequest and onHeadersReceived events. If set, the original request is prevented from being sent/completed and is instead redirected to the given URL. Redirections to non-HTTP schemes such as data: are allowed. Redirects initiated by a redirect action use the original request method for the redirect, with one exception: If the redirect is initiated at the onHeadersReceived stage, then the redirect will be issued using the GET method." }, "requestHeaders": { "$ref": "HttpHeaders", diff --git a/chrome/common/extensions/docs/templates/intros/declarativeWebRequest.html b/chrome/common/extensions/docs/templates/intros/declarativeWebRequest.html index 505c6be..7de1ce8 100644 --- a/chrome/common/extensions/docs/templates/intros/declarativeWebRequest.html +++ b/chrome/common/extensions/docs/templates/intros/declarativeWebRequest.html @@ -180,10 +180,10 @@ request stages that are compatible with conditions and actions. <tr><td>EditRequestCookie<td><td>✓<td><td> <tr><td>EditResponseCookie<td><td><td>✓<td> <tr><td>IgnoreRules<td>✓<td>✓<td>✓<td>✓ - <tr><td>RedirectByRegEx<td>✓<td><td><td> - <tr><td>RedirectRequest<td>✓<td><td><td> - <tr><td>RedirectToEmptyDocument<td>✓<td><td><td> - <tr><td>RedirectToTransparentImage<td>✓<td><td><td> + <tr><td>RedirectByRegEx<td>✓<td><td>✓<td> + <tr><td>RedirectRequest<td>✓<td><td>✓<td> + <tr><td>RedirectToEmptyDocument<td>✓<td><td>✓<td> + <tr><td>RedirectToTransparentImage<td>✓<td><td>✓<td> <tr><td>RemoveRequestCookie<td><td>✓<td><td> <tr><td>RemoveRequestHeader<td><td>✓<td><td> <tr><td>RemoveResponseCookie<td><td><td>✓<td> @@ -198,14 +198,19 @@ request stages that are compatible with conditions and actions. "stages" attribute. </p> <p> +<strong>Note:</strong> Redirects initiated by a redirect action use the original +request method for the redirect, with one exception: If the redirect is +initiated at the onHeadersReceived stage, then the redirect will be issued using +the GET method. +</p> +<p> <strong>Example:</strong> It is possible to combine a <code>new chrome.declarativeWebRequest.RequestMatcher({contentType: ["image/jpeg"]})</code> condition with a <code>new chrome.declarativeWebRequest.CancelRequest()</code> action because both of them can be evaluated in the onHeadersReceived stage. It is, however, impossible to combine the request matcher with a -<code>new chrome.declarativeWebRequest.RedirectToTransparentImage()</code> -because redirects cannot be executed any more by the time the content -type has been determined. +<code>new chrome.declarativeWebRequest.SetRequestHeader()</code> +because request headers cannot be set any more by the time the content type has been terminated. </p> <h2 id="precedences">Using priorities to override rules</h2> diff --git a/chrome/common/extensions/docs/templates/intros/webRequest.html b/chrome/common/extensions/docs/templates/intros/webRequest.html index 44b542d..b0a4930 100644 --- a/chrome/common/extensions/docs/templates/intros/webRequest.html +++ b/chrome/common/extensions/docs/templates/intros/webRequest.html @@ -58,7 +58,8 @@ event definitions:<br/> <dd>Fires each time that an HTTP(S) response header is received. Due to redirects and authentication requests this can happen multiple times per request. This event is intended to allow extensions to add, modify, and delete - response headers, such as incoming Set-Cookie headers.</dd> + response headers, such as incoming Set-Cookie headers. It also allows you to + redirect the request.</dd> <dt><code>onAuthRequired</code> (optionally synchronous)</dt> <dd>Fires when a request requires authentication of the user. This event can be handled synchronously to provide authentication credentials. Note that diff --git a/chrome/test/data/extensions/api_test/webrequest/test_blocking.js b/chrome/test/data/extensions/api_test/webrequest/test_blocking.js index 44f50dd..24232e9 100644 --- a/chrome/test/data/extensions/api_test/webrequest/test_blocking.js +++ b/chrome/test/data/extensions/api_test/webrequest/test_blocking.js @@ -600,6 +600,91 @@ runTests([ }); }, + // Navigates to a page with a blocking handler that redirects to a different + // non-http page during onHeadersReceived. The requested page should not be + // loaded, and the redirect should succeed. + function simpleLoadRedirectOnReceiveHeaders() { + expect( + [ // events + { label: "onBeforeRequest-1", + event: "onBeforeRequest", + details: { + method: "GET", + type: "main_frame", + url: getURLHttpSimpleLoad(), + frameUrl: getURLHttpSimpleLoad() + }, + }, + { label: "onBeforeSendHeaders", + event: "onBeforeSendHeaders", + details: { + url: getURLHttpSimpleLoad(), + // Note: no requestHeaders because we don't ask for them. + }, + }, + { label: "onSendHeaders", + event: "onSendHeaders", + details: { + url: getURLHttpSimpleLoad() + } + }, + { label: "onHeadersReceived", + event: "onHeadersReceived", + details: { + url: getURLHttpSimpleLoad(), + statusLine: "HTTP/1.1 200 OK", + }, + retval: {redirectUrl: getURL("simpleLoad/a.html")} + }, + { label: "onBeforeRedirect", + event: "onBeforeRedirect", + details: { + url: getURLHttpSimpleLoad(), + redirectUrl: getURL("simpleLoad/a.html"), + statusLine: "HTTP/1.1 302 Found", + statusCode: 302, + fromCache: false, + ip: "127.0.0.1", + } + }, + { label: "onBeforeRequest-2", + event: "onBeforeRequest", + details: { + url: getURL("simpleLoad/a.html"), + frameUrl: getURL("simpleLoad/a.html"), + }, + }, + { label: "onResponseStarted", + event: "onResponseStarted", + details: { + url: getURL("simpleLoad/a.html"), + fromCache: false, + statusCode: 200, + statusLine: "HTTP/1.1 200 OK", + // Request to chrome-extension:// url has no IP. + } + }, + { label: "onCompleted", + event: "onCompleted", + details: { + url: getURL("simpleLoad/a.html"), + fromCache: false, + statusCode: 200, + statusLine: "HTTP/1.1 200 OK", + // Request to chrome-extension:// url has no IP. + } + }, + ], + [ // event order + ["onBeforeRequest-1", "onBeforeSendHeaders", "onSendHeaders", + "onHeadersReceived", "onBeforeRedirect", "onBeforeRequest-2", + "onResponseStarted", "onCompleted"] + ], + {urls: ["<all_urls>"]}, // filter + ["blocking"]); + navigateAndWait(getURLHttpSimpleLoad()); + }, + // Checks that synchronous XHR requests from ourself are invisible to blocking // handlers. function syncXhrsFromOurselfAreInvisible() { diff --git a/chrome/test/data/extensions/api_test/webrequest/test_declarative1.js b/chrome/test/data/extensions/api_test/webrequest/test_declarative1.js index 8705ff9..bdd46aa 100644 --- a/chrome/test/data/extensions/api_test/webrequest/test_declarative1.js +++ b/chrome/test/data/extensions/api_test/webrequest/test_declarative1.js @@ -405,6 +405,62 @@ runTests([ ); }, + // Tests that a request is redirected during the onHeadersReceived stage + // when the conditions include a RequestMatcher with a contentType. + function testRedirectRequestByContentType() { + ignoreUnexpected = true; + expect( + [ + { label: "onBeforeRequest-a", + event: "onBeforeRequest", + details: { + type: "main_frame", + url: getURLHttpWithHeaders(), + frameUrl: getURLHttpWithHeaders() + }, + }, + { label: "onBeforeRedirect", + event: "onBeforeRedirect", + details: { + url: getURLHttpWithHeaders(), + redirectUrl: getURLHttpSimple(), + statusLine: "HTTP/1.1 302 Found", + statusCode: 302, + fromCache: false, + ip: "127.0.0.1", + } + }, + { label: "onBeforeRequest-b", + event: "onBeforeRequest", + details: { + type: "main_frame", + url: getURLHttpSimple(), + frameUrl: getURLHttpSimple(), + }, + }, + { label: "onCompleted", + event: "onCompleted", + details: { + ip: "127.0.0.1", + url: getURLHttpSimple(), + fromCache: false, + statusCode: 200, + statusLine: "HTTP/1.1 200 OK", + } + }, + ], + [ ["onBeforeRequest-a", "onBeforeRedirect", "onBeforeRequest-b", + "onCompleted"] ]); + + onRequest.addRules( + [ {'conditions': [new RequestMatcher({'contentType': ["text/plain"]})], + 'actions': [ + new RedirectRequest({'redirectUrl': getURLHttpSimple()})]} + ], + function() {navigateAndWait(getURLHttpWithHeaders());} + ); + }, + function testRedirectByRegEx() { ignoreUnexpected = true; expect( diff --git a/content/shell/browser/shell_network_delegate.cc b/content/shell/browser/shell_network_delegate.cc index 1a65163..5ed9634 100644 --- a/content/shell/browser/shell_network_delegate.cc +++ b/content/shell/browser/shell_network_delegate.cc @@ -47,7 +47,8 @@ int ShellNetworkDelegate::OnHeadersReceived( net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) { + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { return net::OK; } diff --git a/content/shell/browser/shell_network_delegate.h b/content/shell/browser/shell_network_delegate.h index 37c5082..09b0ee51 100644 --- a/content/shell/browser/shell_network_delegate.h +++ b/content/shell/browser/shell_network_delegate.h @@ -32,8 +32,8 @@ class ShellNetworkDelegate : public net::NetworkDelegate { net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* - override_response_headers) OVERRIDE; + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE; virtual void OnBeforeRedirect(net::URLRequest* request, const GURL& new_location) OVERRIDE; virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE; diff --git a/net/base/network_delegate.cc b/net/base/network_delegate.cc index 517868a..d30a4ab 100644 --- a/net/base/network_delegate.cc +++ b/net/base/network_delegate.cc @@ -39,12 +39,16 @@ int NetworkDelegate::NotifyHeadersReceived( URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) { + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { DCHECK(CalledOnValidThread()); DCHECK(original_response_headers); DCHECK(!callback.is_null()); - return OnHeadersReceived(request, callback, original_response_headers, - override_response_headers); + return OnHeadersReceived(request, + callback, + original_response_headers, + override_response_headers, + allowed_unsafe_redirect_url); } void NetworkDelegate::NotifyResponseStarted(URLRequest* request) { @@ -155,7 +159,8 @@ int NetworkDelegate::OnHeadersReceived( URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) { + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { return OK; } diff --git a/net/base/network_delegate.h b/net/base/network_delegate.h index 882701c..4f930cf 100644 --- a/net/base/network_delegate.h +++ b/net/base/network_delegate.h @@ -69,7 +69,8 @@ class NET_EXPORT NetworkDelegate : public base::NonThreadSafe { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers); + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url); void NotifyBeforeRedirect(URLRequest* request, const GURL& new_location); void NotifyResponseStarted(URLRequest* request); @@ -140,7 +141,8 @@ class NET_EXPORT NetworkDelegate : public base::NonThreadSafe { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers); + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url); // Called right after a redirect response code was received. // |new_location| is only valid until OnURLRequestDestroyed is called for this diff --git a/net/cronet/android/url_request_context_peer.cc b/net/cronet/android/url_request_context_peer.cc index 2c754db..e367632 100644 --- a/net/cronet/android/url_request_context_peer.cc +++ b/net/cronet/android/url_request_context_peer.cc @@ -44,7 +44,8 @@ class BasicNetworkDelegate : public net::NetworkDelegate { net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* _response_headers) OVERRIDE { + scoped_refptr<net::HttpResponseHeaders>* _response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE { return net::OK; } diff --git a/net/proxy/network_delegate_error_observer_unittest.cc b/net/proxy/network_delegate_error_observer_unittest.cc index a893cba..1f6330f 100644 --- a/net/proxy/network_delegate_error_observer_unittest.cc +++ b/net/proxy/network_delegate_error_observer_unittest.cc @@ -41,7 +41,8 @@ class TestNetworkDelegate : public net::NetworkDelegate { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) OVERRIDE { + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE { return net::OK; } virtual void OnBeforeRedirect(URLRequest* request, diff --git a/net/proxy/proxy_script_fetcher_impl_unittest.cc b/net/proxy/proxy_script_fetcher_impl_unittest.cc index 2dbcfed..f0505540 100644 --- a/net/proxy/proxy_script_fetcher_impl_unittest.cc +++ b/net/proxy/proxy_script_fetcher_impl_unittest.cc @@ -126,8 +126,8 @@ class BasicNetworkDelegate : public NetworkDelegate { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) - OVERRIDE { + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE { return OK; } diff --git a/net/url_request/url_request_context_builder.cc b/net/url_request/url_request_context_builder.cc index 744dc3b..8405163 100644 --- a/net/url_request/url_request_context_builder.cc +++ b/net/url_request/url_request_context_builder.cc @@ -64,8 +64,8 @@ class BasicNetworkDelegate : public NetworkDelegate { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) - OVERRIDE { + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE { return OK; } diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 8d93eea..41b1515 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -807,11 +807,13 @@ void URLRequestHttpJob::OnStartCompleted(int result) { // |on_headers_received_callback_| or // |NetworkDelegate::URLRequestDestroyed()| has been called. OnCallToDelegate(); + allowed_unsafe_redirect_url_ = GURL(); int error = network_delegate()->NotifyHeadersReceived( request_, on_headers_received_callback_, headers.get(), - &override_response_headers_); + &override_response_headers_, + &allowed_unsafe_redirect_url_); if (error != net::OK) { if (error == net::ERR_IO_PENDING) { awaiting_callback_ = true; @@ -1039,6 +1041,15 @@ bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) { (location.scheme() == "http" || location.scheme() == "https")) { return true; } + // Delegates may mark an URL as safe for redirection. + if (allowed_unsafe_redirect_url_.is_valid()) { + GURL::Replacements replacements; + replacements.ClearRef(); + if (allowed_unsafe_redirect_url_.ReplaceComponents(replacements) == + location.ReplaceComponents(replacements)) { + return true; + } + } // Query URLRequestJobFactory as to whether |location| would be safe to // redirect to. return request_->context()->job_factory() && diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index 8680619..95b77c9 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -256,6 +256,9 @@ class NET_EXPORT_PRIVATE URLRequestHttpJob : public URLRequestJob { // layers of the network stack. scoped_refptr<HttpResponseHeaders> override_response_headers_; + // The network delegate can mark a URL as safe for redirection. + GURL allowed_unsafe_redirect_url_; + // Flag used to verify that |this| is not deleted while we are awaiting // a callback from the NetworkDelegate. Used as a fail-fast mechanism. // True if we are waiting a callback and diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc index 4760410..17ad311 100644 --- a/net/url_request/url_request_test_util.cc +++ b/net/url_request/url_request_test_util.cc @@ -411,7 +411,8 @@ int TestNetworkDelegate::OnHeadersReceived( URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) { + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { int req_id = request->identifier(); event_order_[req_id] += "OnHeadersReceived\n"; InitRequestStatesIfNew(req_id); @@ -436,6 +437,9 @@ int TestNetworkDelegate::OnHeadersReceived( "Location: " + redirect_on_headers_received_url_.spec()); redirect_on_headers_received_url_ = GURL(); + + if (!allowed_unsafe_redirect_url_.is_empty()) + *allowed_unsafe_redirect_url = allowed_unsafe_redirect_url_; } return OK; diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h index ebfa839..0b2f48c 100644 --- a/net/url_request/url_request_test_util.h +++ b/net/url_request/url_request_test_util.h @@ -241,6 +241,10 @@ class TestNetworkDelegate : public NetworkDelegate { redirect_on_headers_received_url_ = redirect_on_headers_received_url; } + void set_allowed_unsafe_redirect_url(GURL allowed_unsafe_redirect_url) { + allowed_unsafe_redirect_url_ = allowed_unsafe_redirect_url; + } + void set_cookie_options(int o) {cookie_options_bit_mask_ = o; } int last_error() const { return last_error_; } @@ -273,7 +277,8 @@ class TestNetworkDelegate : public NetworkDelegate { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) OVERRIDE; + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE; virtual void OnBeforeRedirect(URLRequest* request, const GURL& new_location) OVERRIDE; virtual void OnResponseStarted(URLRequest* request) OVERRIDE; @@ -304,6 +309,8 @@ class TestNetworkDelegate : public NetworkDelegate { void InitRequestStatesIfNew(int request_id); GURL redirect_on_headers_received_url_; + // URL marked as safe for redirection at the onHeadersReceived stage. + GURL allowed_unsafe_redirect_url_; int last_error_; int error_count_; diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index 43d5d308..9d90cf8 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -368,7 +368,8 @@ class BlockingNetworkDelegate : public TestNetworkDelegate { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) OVERRIDE; + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE; virtual NetworkDelegate::AuthRequiredResponse OnAuthRequired( URLRequest* request, @@ -391,7 +392,7 @@ class BlockingNetworkDelegate : public TestNetworkDelegate { int retval_; // To be returned in non-auth stages. AuthRequiredResponse auth_retval_; - GURL redirect_url_; // Used if non-empty. + GURL redirect_url_; // Used if non-empty during OnBeforeURLRequest. int block_on_; // Bit mask: in which stages to block. // |auth_credentials_| will be copied to |*target_auth_credential_| on @@ -483,10 +484,13 @@ int BlockingNetworkDelegate::OnHeadersReceived( URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) { - TestNetworkDelegate::OnHeadersReceived( - request, callback, original_response_headers, - override_response_headers); + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { + TestNetworkDelegate::OnHeadersReceived(request, + callback, + original_response_headers, + override_response_headers, + allowed_unsafe_redirect_url); return MaybeBlockStage(ON_HEADERS_RECEIVED, callback); } @@ -2417,8 +2421,8 @@ class FixedDateNetworkDelegate : public TestNetworkDelegate { net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) - OVERRIDE; + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE; private: std::string fixed_date_; @@ -2430,7 +2434,8 @@ int FixedDateNetworkDelegate::OnHeadersReceived( net::URLRequest* request, const net::CompletionCallback& callback, const net::HttpResponseHeaders* original_response_headers, - scoped_refptr<net::HttpResponseHeaders>* override_response_headers) { + scoped_refptr<net::HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) { net::HttpResponseHeaders* new_response_headers = new net::HttpResponseHeaders(original_response_headers->raw_headers()); @@ -2441,7 +2446,8 @@ int FixedDateNetworkDelegate::OnHeadersReceived( return TestNetworkDelegate::OnHeadersReceived(request, callback, original_response_headers, - override_response_headers); + override_response_headers, + allowed_unsafe_redirect_url); } // Test that cookie expiration times are adjusted for server/client clock @@ -3008,6 +3014,39 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestPost) { EXPECT_EQ(1, network_delegate.destroyed_requests()); } +// Tests that the network delegate can block and redirect a request to a new +// URL during OnHeadersReceived. +TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestOnHeadersReceived) { + ASSERT_TRUE(test_server_.Start()); + + TestDelegate d; + BlockingNetworkDelegate network_delegate( + BlockingNetworkDelegate::AUTO_CALLBACK); + network_delegate.set_block_on(BlockingNetworkDelegate::ON_HEADERS_RECEIVED); + GURL redirect_url(test_server_.GetURL("simple.html")); + network_delegate.set_redirect_on_headers_received_url(redirect_url); + + TestURLRequestContextWithProxy context( + test_server_.host_port_pair().ToString(), &network_delegate); + + { + GURL original_url(test_server_.GetURL("empty.html")); + URLRequest r(original_url, DEFAULT_PRIORITY, &d, &context); + + r.Start(); + base::RunLoop().Run(); + + EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); + EXPECT_EQ(net::OK, r.status().error()); + EXPECT_EQ(redirect_url, r.url()); + EXPECT_EQ(original_url, r.original_url()); + EXPECT_EQ(2U, r.url_chain().size()); + EXPECT_EQ(2, network_delegate.created_requests()); + EXPECT_EQ(0, network_delegate.destroyed_requests()); + } + EXPECT_EQ(1, network_delegate.destroyed_requests()); +} + // Tests that the network delegate can synchronously complete OnAuthRequired // by taking no action. This indicates that the NetworkDelegate does not want to // handle the challenge, and is passing the buck along to the @@ -3920,10 +3959,13 @@ class AsyncLoggingNetworkDelegate : public TestNetworkDelegate { URLRequest* request, const CompletionCallback& callback, const HttpResponseHeaders* original_response_headers, - scoped_refptr<HttpResponseHeaders>* override_response_headers) OVERRIDE { - TestNetworkDelegate::OnHeadersReceived(request, callback, + scoped_refptr<HttpResponseHeaders>* override_response_headers, + GURL* allowed_unsafe_redirect_url) OVERRIDE { + TestNetworkDelegate::OnHeadersReceived(request, + callback, original_response_headers, - override_response_headers); + override_response_headers, + allowed_unsafe_redirect_url); return RunCallbackAsynchronously(request, callback); } @@ -5235,6 +5277,142 @@ TEST_F(URLRequestTestHTTP, NoCacheOnNetworkDelegateRedirect) { } } +// Tests that redirection to an unsafe URL is allowed when it has been marked as +// safe. +TEST_F(URLRequestTestHTTP, UnsafeRedirectToWhitelistedUnsafeURL) { + ASSERT_TRUE(test_server_.Start()); + + GURL unsafe_url("data:text/html,this-is-considered-an-unsafe-url"); + default_network_delegate_.set_redirect_on_headers_received_url(unsafe_url); + default_network_delegate_.set_allowed_unsafe_redirect_url(unsafe_url); + + TestDelegate d; + { + URLRequest r(test_server_.GetURL("whatever"), + DEFAULT_PRIORITY, + &d, + &default_context_); + + r.Start(); + base::RunLoop().Run(); + + EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); + + EXPECT_EQ(2U, r.url_chain().size()); + EXPECT_EQ(net::OK, r.status().error()); + EXPECT_EQ(unsafe_url, r.url()); + EXPECT_EQ("this-is-considered-an-unsafe-url", d.data_received()); + } +} + +// Tests that a redirect to a different unsafe URL is blocked, even after adding +// some other URL to the whitelist. +TEST_F(URLRequestTestHTTP, UnsafeRedirectToDifferentUnsafeURL) { + ASSERT_TRUE(test_server_.Start()); + + GURL unsafe_url("data:text/html,something"); + GURL different_unsafe_url("data:text/html,something-else"); + default_network_delegate_.set_redirect_on_headers_received_url(unsafe_url); + default_network_delegate_.set_allowed_unsafe_redirect_url( + different_unsafe_url); + + TestDelegate d; + { + URLRequest r(test_server_.GetURL("whatever"), + DEFAULT_PRIORITY, + &d, + &default_context_); + + r.Start(); + base::RunLoop().Run(); + + EXPECT_EQ(URLRequestStatus::FAILED, r.status().status()); + EXPECT_EQ(ERR_UNSAFE_REDIRECT, r.status().error()); + } +} + +// Redirects from an URL with fragment to an unsafe URL without fragment should +// be allowed. +TEST_F(URLRequestTestHTTP, UnsafeRedirectWithReferenceFragment) { + ASSERT_TRUE(test_server_.Start()); + + GURL original_url(test_server_.GetURL("original#fragment")); + GURL unsafe_url("data:,url-marked-safe-and-used-in-redirect"); + GURL expected_url("data:,url-marked-safe-and-used-in-redirect#fragment"); + + default_network_delegate_.set_redirect_on_headers_received_url(unsafe_url); + default_network_delegate_.set_allowed_unsafe_redirect_url(unsafe_url); + + TestDelegate d; + { + URLRequest r(original_url, DEFAULT_PRIORITY, &d, &default_context_); + + r.Start(); + base::RunLoop().Run(); + + EXPECT_EQ(2U, r.url_chain().size()); + EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); + EXPECT_EQ(net::OK, r.status().error()); + EXPECT_EQ(original_url, r.original_url()); + EXPECT_EQ(expected_url, r.url()); + } +} + +// Redirects from an URL with fragment to an unsafe URL with fragment should +// be allowed, and the reference fragment of the target URL should be preserved. +TEST_F(URLRequestTestHTTP, UnsafeRedirectWithDifferentReferenceFragment) { + ASSERT_TRUE(test_server_.Start()); + + GURL original_url(test_server_.GetURL("original#fragment1")); + GURL unsafe_url("data:,url-marked-safe-and-used-in-redirect#fragment2"); + GURL expected_url("data:,url-marked-safe-and-used-in-redirect#fragment2"); + + default_network_delegate_.set_redirect_on_headers_received_url(unsafe_url); + default_network_delegate_.set_allowed_unsafe_redirect_url(unsafe_url); + + TestDelegate d; + { + URLRequest r(original_url, DEFAULT_PRIORITY, &d, &default_context_); + + r.Start(); + base::RunLoop().Run(); + + EXPECT_EQ(2U, r.url_chain().size()); + EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); + EXPECT_EQ(net::OK, r.status().error()); + EXPECT_EQ(original_url, r.original_url()); + EXPECT_EQ(expected_url, r.url()); + } +} + +// When a delegate has specified a safe redirect URL, but it does not match the +// redirect target, then do not prevent the reference fragment from being added. +TEST_F(URLRequestTestHTTP, RedirectWithReferenceFragment) { + ASSERT_TRUE(test_server_.Start()); + + GURL original_url(test_server_.GetURL("original#expected-fragment")); + GURL unsafe_url("data:text/html,this-url-does-not-match-redirect-url"); + GURL redirect_url(test_server_.GetURL("target")); + GURL expected_redirect_url(test_server_.GetURL("target#expected-fragment")); + + default_network_delegate_.set_redirect_on_headers_received_url(redirect_url); + default_network_delegate_.set_allowed_unsafe_redirect_url(unsafe_url); + + TestDelegate d; + { + URLRequest r(original_url, DEFAULT_PRIORITY, &d, &default_context_); + + r.Start(); + base::RunLoop().Run(); + + EXPECT_EQ(2U, r.url_chain().size()); + EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status()); + EXPECT_EQ(net::OK, r.status().error()); + EXPECT_EQ(original_url, r.original_url()); + EXPECT_EQ(expected_redirect_url, r.url()); + } +} + TEST_F(URLRequestTestHTTP, NoUserPassInReferrer) { ASSERT_TRUE(test_server_.Start()); |