Skip to content

Commit

Permalink
Fix content-encoding handling with buggy servers.
Browse files Browse the repository at this point in the history
Some servers will serve compressed filetypes with "content-encoding: gzip"
because the file's contents were gzipped, even though the server is not
re-encoding the contents at all. The code in Filter::FixupEncodingTypes()
attempts to work around this by inferring whether the file is a gzip file with
bogus encoding based on the file extension in the URL, but this doesn't work
when the file being downloaded comes from a page whose extension doesn't reveal
the downloaded file type.

This change adds code to FixupEncodingTypes to also consider the download
filename, supplied by the "content-disposition" header, if any. This means that
we now behave correctly in the case where:

-> GET /foo.php?download
<- HTTP/1.1 200 OK
<- ...
<- Content-Disposition: attachment;filename="foo.tar.gz"
<- ...
<- Content-Encoding: gzip

BUG=83292
TEST=unit,trybot

Review URL: https://codereview.chromium.org/206503006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258729 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ellyjones@chromium.org committed Mar 22, 2014
1 parent 03cc885 commit 2621917
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 3 deletions.
10 changes: 7 additions & 3 deletions net/filter/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/strings/string_util.h"
#include "net/base/io_buffer.h"
#include "net/base/mime_util.h"
#include "net/base/net_util.h"
#include "net/filter/gzip_filter.h"
#include "net/filter/sdch_filter.h"

Expand Down Expand Up @@ -170,11 +171,14 @@ void Filter::FixupEncodingTypes(
encoding_types->clear();

GURL url;
std::string disposition;
success = filter_context.GetURL(&url);
DCHECK(success);
base::FilePath filename =
base::FilePath().AppendASCII(url.ExtractFileName());
base::FilePath::StringType extension = filename.Extension();
filter_context.GetContentDisposition(&disposition);
// Don't supply a MIME type here, since that may cause disk IO.
base::FilePath filepath = GenerateFileName(url, disposition, "UTF-8", "",
"", "");
base::FilePath::StringType extension = filepath.Extension();

if (filter_context.IsDownload()) {
// We don't want to decompress gzipped files when the user explicitly
Expand Down
4 changes: 4 additions & 0 deletions net/filter/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ class NET_EXPORT_PRIVATE FilterContext {
// Return false if gurl is not present.
virtual bool GetURL(GURL* gurl) const = 0;

// What Content-Disposition header came with this data?
// Return false if no header was present.
virtual bool GetContentDisposition(std::string* disposition) const = 0;

// When was this data requested from a server?
virtual base::Time GetRequestTime() const = 0;

Expand Down
17 changes: 17 additions & 0 deletions net/filter/filter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,23 @@ TEST(FilterTest, ApacheGzip) {
EXPECT_EQ(Filter::FILTER_TYPE_GZIP, encoding_types.front());
}

TEST(FilterTest, GzipContentDispositionFilename) {
MockFilterContext filter_context;
filter_context.SetSdchResponse(false);

const std::string kGzipMime("application/x-tar");
const std::string kContentDisposition("attachment; filename=\"foo.tgz\"");
const std::string kURL("http://foo.com/getfoo.php");
std::vector<Filter::FilterType> encoding_types;

encoding_types.push_back(Filter::FILTER_TYPE_GZIP);
filter_context.SetMimeType(kGzipMime);
filter_context.SetURL(GURL(kURL));
filter_context.SetContentDisposition(kContentDisposition);
Filter::FixupEncodingTypes(filter_context, &encoding_types);
ASSERT_EQ(0U, encoding_types.size());
}

TEST(FilterTest, SdchEncoding) {
// Handle content encodings including SDCH.
const std::string kTextHtmlMime("text/html");
Expand Down
7 changes: 7 additions & 0 deletions net/filter/mock_filter_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ bool MockFilterContext::GetURL(GURL* gurl) const {
return true;
}

bool MockFilterContext::GetContentDisposition(std::string* disposition) const {
if (content_disposition_.empty())
return false;
*disposition = content_disposition_;
return true;
}

// What was this data requested from a server?
base::Time MockFilterContext::GetRequestTime() const {
return request_time_;
Expand Down
8 changes: 8 additions & 0 deletions net/filter/mock_filter_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ class MockFilterContext : public FilterContext {

void SetMimeType(const std::string& mime_type) { mime_type_ = mime_type; }
void SetURL(const GURL& gurl) { gurl_ = gurl; }
void SetContentDisposition(const std::string& disposition) {
content_disposition_ = disposition;
}
void SetRequestTime(const base::Time time) { request_time_ = time; }
void SetCached(bool is_cached) { is_cached_content_ = is_cached; }
void SetDownload(bool is_download) { is_download_ = is_download; }
Expand All @@ -33,6 +36,10 @@ class MockFilterContext : public FilterContext {
// Return false if gurl is not present.
virtual bool GetURL(GURL* gurl) const OVERRIDE;

// What Content-Disposition did the server supply for this data?
// Return false if Content-Disposition was not present.
virtual bool GetContentDisposition(std::string* disposition) const OVERRIDE;

// What was this data requested from a server?
virtual base::Time GetRequestTime() const OVERRIDE;

Expand All @@ -55,6 +62,7 @@ class MockFilterContext : public FilterContext {
private:
int buffer_size_;
std::string mime_type_;
std::string content_disposition_;
GURL gurl_;
base::Time request_time_;
bool is_cached_content_;
Expand Down
9 changes: 9 additions & 0 deletions net/url_request/url_request_http_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "net/base/sdch_manager.h"
#include "net/cert/cert_status_flags.h"
#include "net/cookies/cookie_monster.h"
#include "net/http/http_content_disposition.h"
#include "net/http/http_network_session.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_response_headers.h"
Expand Down Expand Up @@ -58,6 +59,7 @@ class URLRequestHttpJob::HttpFilterContext : public FilterContext {
// FilterContext implementation.
virtual bool GetMimeType(std::string* mime_type) const OVERRIDE;
virtual bool GetURL(GURL* gurl) const OVERRIDE;
virtual bool GetContentDisposition(std::string* disposition) const OVERRIDE;
virtual base::Time GetRequestTime() const OVERRIDE;
virtual bool IsCachedContent() const OVERRIDE;
virtual bool IsDownload() const OVERRIDE;
Expand Down Expand Up @@ -96,6 +98,13 @@ bool URLRequestHttpJob::HttpFilterContext::GetURL(GURL* gurl) const {
return true;
}

bool URLRequestHttpJob::HttpFilterContext::GetContentDisposition(
std::string* disposition) const {
HttpResponseHeaders* headers = job_->GetResponseHeaders();
void *iter = NULL;
return headers->EnumerateHeader(&iter, "Content-Disposition", disposition);
}

base::Time URLRequestHttpJob::HttpFilterContext::GetRequestTime() const {
return job_->request() ? job_->request()->request_time() : base::Time();
}
Expand Down

0 comments on commit 2621917

Please sign in to comment.