Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(341)

Unified Diff: net/url_request/url_request_http_job.cc

Issue 154473002: Support redirectUrl at onHeadersReceived in WebRequest / DWR API (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add more tests Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 947686cb9f2d9184310d031780b0c46fb5265d62..a026add3f8647ea95621f515ce49989b9e70eaef 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -289,6 +289,11 @@ void URLRequestHttpJob::Kill() {
void URLRequestHttpJob::NotifyHeadersComplete() {
DCHECK(!response_info_);
+ if (GetResponseHeaders()->HasSafeRedirect()) {
+ // Do not store non-server-issued redirects in cache.
+ transaction_->StopCaching();
mmenke 2014/03/20 15:22:25 I don't think we actually need this. My cache com
robwu 2014/03/20 16:21:11 DoneReading() just calls DoneWritingToEntry(true);
mmenke 2014/03/20 16:40:28 We should have a test for that, too, then. Does t
mmenke 2014/03/20 17:22:38 Turns out this is a bug added back in September by
robwu 2014/03/20 17:28:14 Okay, great! Do you think whether it's acceptable
+ }
+
response_info_ = transaction_->GetResponseInfo();
// Save boolean, as we'll need this info at destruction time, and filters may
@@ -1037,6 +1042,9 @@ bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) {
(location.scheme() == "http" || location.scheme() == "https")) {
return true;
}
+ if (GetResponseHeaders()->IsSafeRedirect(location)) {
mmenke 2014/03/20 15:22:25 I don't think this should be part of the headers,
robwu 2014/03/20 16:21:11 So, basically the situation from patch set 1? Then
mmenke 2014/03/20 16:40:28 We allow overriding the entire headers, so I don't
robwu 2014/03/20 17:28:14 After re-reading your previous comment, I see that
mmenke 2014/03/20 17:31:59 Not sure you're completely following me... Here's
+ return true;
+ }
// Query URLRequestJobFactory as to whether |location| would be safe to
// redirect to.
return request_->context()->job_factory() &&

Powered by Google App Engine
This is Rietveld 408576698