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

Unified Diff: pdf/document_loader.cc

Issue 2455663004: Add test to ensure that URLs that redirect inside the PDF plugin fail to load (Closed)
Patch Set: Created 4 years, 1 month 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: pdf/document_loader.cc
diff --git a/pdf/document_loader.cc b/pdf/document_loader.cc
index ea4b664b27b53afccc3f6ccb50e23eae456a7ab9..ebc7174e84e3cf445c61c83b5141e92cc1ba86bb 100644
--- a/pdf/document_loader.cc
+++ b/pdf/document_loader.cc
@@ -19,6 +19,20 @@ namespace chrome_pdf {
namespace {
+// Return true if the HTTP response of |loader| is a succesful one and loading
Lei Zhang 2016/11/03 07:10:45 typo
raymes 2016/11/07 03:25:44 Done.
+// should continue. 4xx error indicate subsequent requests will fail too.
+// e.g. resource has been removed from the server while loading it. 301
+// indicates a redirect was returned which won't be successful because we
+// disable following redirects for PDF loading (we assume they are already
+// resolved by the browser.
+bool ResponseStatusSuccess(const pp::URLLoader& loader) {
+ int32_t http_code = loader.GetResponseInfo().GetStatusCode();
+ if ((http_code >= 400 && http_code < 500) || http_code == 301)
Lei Zhang 2016/11/03 07:10:45 Just: return (http_code < 400 && http_code != 301)
raymes 2016/11/07 03:25:44 Done.
+ return false;
+
+ return true;
+}
+
// If the headers have a byte-range response, writes the start and end
// positions and returns true if at least the start position was parsed.
// The end position will be set to 0 if it was not found or parsed from the
@@ -102,6 +116,10 @@ bool DocumentLoader::Init(const pp::URLLoader& loader,
url_ = url;
loader_ = loader;
+ // Check that the initial response status is a valid one.
+ if (!ResponseStatusSuccess(loader_))
+ return false;
+
std::string response_headers;
if (!headers.empty()) {
response_headers = headers;
@@ -358,12 +376,8 @@ void DocumentLoader::DidOpen(int32_t result) {
return;
}
- int32_t http_code = loader_.GetResponseInfo().GetStatusCode();
- if (http_code >= 400 && http_code < 500) {
- // Error accessing resource. 4xx error indicate subsequent requests
- // will fail too.
- // E.g. resource has been removed from the server while loading it.
- // https://code.google.com/p/chromium/issues/detail?id=414827
+ if (!ResponseStatusSuccess(loader_)) {
+ client_->DocumentLoadFailed();
return;
}

Powered by Google App Engine
This is Rietveld 408576698