diff options
| -rw-r--r-- | chrome/browser/automation/url_request_automation_job.cc | 20 | ||||
| -rw-r--r-- | chrome_frame/urlmon_url_request.cc | 110 | ||||
| -rw-r--r-- | chrome_frame/urlmon_url_request.h | 6 | ||||
| -rw-r--r-- | net/http/http_response_headers.cc | 17 | ||||
| -rw-r--r-- | net/http/http_response_headers.h | 4 |
5 files changed, 107 insertions, 50 deletions
diff --git a/chrome/browser/automation/url_request_automation_job.cc b/chrome/browser/automation/url_request_automation_job.cc index f6d4ecd..97522d2 100644 --- a/chrome/browser/automation/url_request_automation_job.cc +++ b/chrome/browser/automation/url_request_automation_job.cc @@ -198,20 +198,12 @@ int URLRequestAutomationJob::GetResponseCode() const { bool URLRequestAutomationJob::IsRedirectResponse( GURL* location, int* http_status_code) { - static const int kDefaultHttpRedirectResponseCode = 301; - - if (!redirect_url_.empty()) { - DLOG_IF(ERROR, redirect_status_ == 0) << "Missing redirect status?"; - *http_status_code = redirect_status_ ? redirect_status_ : - kDefaultHttpRedirectResponseCode; - *location = GURL(redirect_url_); - return true; - } else { - DCHECK(redirect_status_ == 0) - << "Unexpectedly have redirect status but no URL"; - } + if (!net::HttpResponseHeaders::IsRedirectResponseCode(redirect_status_)) + return false; - return false; + *http_status_code = redirect_status_; + *location = GURL(redirect_url_); + return true; } bool URLRequestAutomationJob::MayFilterMessage(const IPC::Message& message, @@ -255,7 +247,7 @@ void URLRequestAutomationJob::OnRequestStarted(int tab, int id, GURL url_for_cookies = GURL(redirect_url_.empty() ? request_->url().spec().c_str() : - redirect_url_.c_str()); + redirect_url_.c_str()); URLRequestContext* ctx = request_->context(); diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc index 71aee9b..6124979 100644 --- a/chrome_frame/urlmon_url_request.cc +++ b/chrome_frame/urlmon_url_request.cc @@ -27,7 +27,8 @@ UrlmonUrlRequest::UrlmonUrlRequest() thread_(PlatformThread::CurrentId()), redirect_status_(0), parent_window_(NULL), - worker_thread_(NULL) { + worker_thread_(NULL), + ignore_redirect_stop_binding_error_(false) { DLOG(INFO) << StringPrintf("Created request. Obj: %X", this) << " Count: " << ++instance_count_; } @@ -177,27 +178,32 @@ STDMETHODIMP UrlmonUrlRequest::OnLowResource(DWORD reserved) { STDMETHODIMP UrlmonUrlRequest::OnProgress(ULONG progress, ULONG max_progress, ULONG status_code, LPCWSTR status_text) { + static const int kDefaultHttpRedirectCode = 302; + switch (status_code) { - case BINDSTATUS_REDIRECTING: + case BINDSTATUS_REDIRECTING: { + // Fetch the redirect status as they aren't all equal (307 in particular + // retains the HTTP request verb). + // We assume that valid redirect codes are 301, 302, 303 and 307. If we + // receive anything else we would abort the request which would + // eventually result in the request getting cancelled in Chrome. + int redirect_status = GetHttpResponseStatus(); DCHECK(status_text != NULL); DLOG(INFO) << "URL: " << url() << " redirected to " << status_text; redirect_url_ = status_text; - // Fetch the redirect status as they aren't all equal (307 in particular - // retains the HTTP request verb). - redirect_status_ = GetHttpResponseStatus(); - // NOTE: Even though RFC 2616 says to preserve the request method when - // following a 302 redirect, normal browsers don't do that. Instead they - // all convert a POST into a GET in response to a 302 and so shall we. - // For 307 redirects, browsers preserve the method. The RFC says to - // prompt the user to confirm the generation of a new POST request, but - // IE omits this prompt and so shall we. - if (redirect_status_ != 307 && - LowerCaseEqualsASCII(method(), "post")) { - set_method("get"); - ClearPostData(); - } - break; + redirect_status_ = + redirect_status > 0 ? redirect_status : kDefaultHttpRedirectCode; + // Chrome should decide whether a redirect has to be followed. To achieve + // this we send over a fake response to Chrome and abort the redirect. + std::string headers = GetHttpHeaders(); + OnResponse(0, UTF8ToWide(headers).c_str(), NULL, NULL); + ignore_redirect_stop_binding_error_ = true; + DCHECK(binding_ != NULL); + binding_->Abort(); + binding_ = NULL; + return E_ABORT; + } default: DLOG(INFO) << " Obj: " << std::hex << this << " OnProgress(" << url() @@ -215,6 +221,7 @@ STDMETHODIMP UrlmonUrlRequest::OnStopBinding(HRESULT result, LPCWSTR error) { DLOG(INFO) << StringPrintf("URL: %s Obj: %X", url().c_str(), this) << " - Request stopped, Result: " << std::hex << result << " Status: " << status_.status(); + if (FAILED(result)) { status_.set_status(URLRequestStatus::FAILED); status_.set_os_error(HresultToNetError(result)); @@ -607,20 +614,27 @@ HRESULT UrlmonUrlRequest::StartAsyncDownload() { void UrlmonUrlRequest::EndRequest() { DLOG(INFO) << __FUNCTION__; - // Special case. If the last request was a redirect and the current OS - // error value is E_ACCESSDENIED, that means an unsafe redirect was attempted. - // In that case, correct the OS error value to be the more specific - // ERR_UNSAFE_REDIRECT error value. - if (!status_.is_success() && status_.os_error() == net::ERR_ACCESS_DENIED) { - int status = GetHttpResponseStatus(); - if (status >= 300 && status < 400) { - redirect_status_ = status; // store the latest redirect status value. - status_.set_os_error(net::ERR_UNSAFE_REDIRECT); + + // In case of a redirect notification we prevent urlmon from following the + // redirect and rely on Chrome, in which case AutomationMsg_RequestEnd + // IPC will be sent over by Chrome to end this request. + if (!ignore_redirect_stop_binding_error_) { + // Special case. If the last request was a redirect and the current OS + // error value is E_ACCESSDENIED, that means an unsafe redirect was + // attempted. In that case, correct the OS error value to be the more + // specific ERR_UNSAFE_REDIRECT error value. + if (!status_.is_success() && status_.os_error() == net::ERR_ACCESS_DENIED) { + int status = GetHttpResponseStatus(); + if (status >= 300 && status < 400) { + redirect_status_ = status; // store the latest redirect status value. + status_.set_os_error(net::ERR_UNSAFE_REDIRECT); + } } + OnResponseEnd(status_); + } else { + ignore_redirect_stop_binding_error_ = false; } - OnResponseEnd(status_); - // Remove the request mapping and release the outstanding reference to us in // the context of the UI thread. PostTask(FROM_HERE, @@ -649,8 +663,10 @@ int UrlmonUrlRequest::GetHttpResponseStatus() const { if (SUCCEEDED(info.QueryFrom(binding_))) { char status[10] = {0}; DWORD buf_size = sizeof(status); + DWORD flags = 0; + DWORD reserved = 0; if (SUCCEEDED(info->QueryInfo(HTTP_QUERY_STATUS_CODE, status, &buf_size, - 0, NULL))) { + &flags, &reserved))) { http_status = StringToInt(status); } else { NOTREACHED() << "Failed to get HTTP status"; @@ -662,6 +678,42 @@ int UrlmonUrlRequest::GetHttpResponseStatus() const { return http_status; } +std::string UrlmonUrlRequest::GetHttpHeaders() const { + if (binding_ == NULL) { + DLOG(WARNING) << "GetHttpResponseStatus - no binding_"; + return std::string(); + } + + ScopedComPtr<IWinInetHttpInfo> info; + if (FAILED(info.QueryFrom(binding_))) { + DLOG(WARNING) << "Failed to QI for IWinInetHttpInfo"; + return std::string(); + } + + scoped_ptr<char> buffer; + DWORD size = 0; + DWORD flags = 0; + DWORD reserved = 0; + HRESULT hr = info->QueryInfo(HTTP_QUERY_RAW_HEADERS_CRLF, NULL, &size, + &flags, &reserved); + if (!size) { + DLOG(WARNING) << "Failed to query HTTP headers size. Error 0x%x" << hr; + return std::string(); + } + + buffer.reset(new char[size]); + memset(buffer.get(), 0, size); + + hr = info->QueryInfo(HTTP_QUERY_RAW_HEADERS_CRLF, buffer.get(), + &size, &flags, &reserved); + if (FAILED(hr)) { + DLOG(WARNING) << "Failed to query HTTP headers. Error 0x%x" << hr; + return std::string(); + } + + return buffer.get(); +} + // // UrlmonUrlRequest::Cache implementation. // diff --git a/chrome_frame/urlmon_url_request.h b/chrome_frame/urlmon_url_request.h index c95f47d..0612f33 100644 --- a/chrome_frame/urlmon_url_request.h +++ b/chrome_frame/urlmon_url_request.h @@ -9,8 +9,8 @@ #include <atlbase.h> #include <atlcom.h> #include <atlwin.h> - #include <algorithm> +#include <string> #include "base/lock.h" #include "base/platform_thread.h" @@ -239,6 +239,7 @@ END_MSG_MAP() // reference to us. It also deletes the request mapping for this instance. void EndRequestInternal(); int GetHttpResponseStatus() const; + std::string GetHttpHeaders() const; static net::Error HresultToNetError(HRESULT hr); @@ -255,6 +256,9 @@ END_MSG_MAP() PlatformThreadId thread_; static int instance_count_; HWND parent_window_; + // Set to true if a redirect notification was aborted. + bool ignore_redirect_stop_binding_error_; + DISALLOW_COPY_AND_ASSIGN(UrlmonUrlRequest); }; diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc index 9348006..e342d3b 100644 --- a/net/http/http_response_headers.cc +++ b/net/http/http_response_headers.cc @@ -724,12 +724,7 @@ bool HttpResponseHeaders::GetCharset(std::string* charset) const { } bool HttpResponseHeaders::IsRedirect(std::string* location) const { - // Users probably want to see 300 (multiple choice) pages, so we don't count - // them as redirects that need to be followed. - if (!(response_code_ == 301 || - response_code_ == 302 || - response_code_ == 303 || - response_code_ == 307)) + if (!IsRedirectResponseCode(response_code_)) return false; // If we lack a Location header, then we can't treat this as a redirect. @@ -753,6 +748,16 @@ bool HttpResponseHeaders::IsRedirect(std::string* location) const { return true; } +// static +bool HttpResponseHeaders::IsRedirectResponseCode(int response_code) { + // Users probably want to see 300 (multiple choice) pages, so we don't count + // them as redirects that need to be followed. + return (response_code == 301 || + response_code == 302 || + response_code == 303 || + response_code == 307); +} + // From RFC 2616 section 13.2.4: // // The calculation to determine if a response has expired is quite simple: diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h index f1512b4..c071389 100644 --- a/net/http/http_response_headers.h +++ b/net/http/http_response_headers.h @@ -171,6 +171,10 @@ class HttpResponseHeaders // location of the redirect is optionally returned if location is non-null. bool IsRedirect(std::string* location) const; + // Returns true if the HTTP response code passed in corresponds to a + // redirect. + static bool IsRedirectResponseCode(int response_code); + // Returns true if the response cannot be reused without validation. The // result is relative to the current_time parameter, which is a parameter to // support unit testing. The request_time parameter indicates the time at |