Use HTTP status return code to make SDCH handling more robust.

At least one proxy replaces the SDCH content with an
error page (of HTML, without SDCH compression).  We can 
identify that scenario by spotting the 40x HTTP return
code (and the fact that the content is not SDCH encoded,
even though we advertised SDCH and a dictionary to the 
server).

This change list adds the ability to access the return code
via the FilterContext.  The bulk of the change is centered
on getting that access method to be const in all derived
classes.

bug=8916

r=wtc,huanr,openvcdiff
Review URL: http://codereview.chromium.org/56043

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@12784 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 222 insertions, 15 deletions

diff --git a/net/base/filter.h b/net/base/filter.h
index 504093d..e944918 100644
--- a/net/base/filter.h
+++ b/net/base/filter.h
@@ -72,6 +72,10 @@ class FilterContext {
   // pushed into a filter for processing)?
   virtual int64 GetByteReadCount() const = 0;
 
+  // What response code was received with the associated network transaction?
+  // For example: 200 is ok.   4xx are error codes. etc.
+  virtual int GetResponseCode() const = 0;
+
   // What is the desirable input buffer size for these filters?
   // This value is currently supplied by the context, and is constant for all
   // filters, even when they are part of a chain of filters. (i.e., we currently
diff --git a/net/base/filter_unittest.h b/net/base/filter_unittest.h
index cf0e13d..8ed572f 100644
--- a/net/base/filter_unittest.h
+++ b/net/base/filter_unittest.h
@@ -17,7 +17,8 @@ class MockFilterContext : public FilterContext {
   explicit MockFilterContext(int buffer_size)
     : buffer_size_(buffer_size),
       is_cached_content_(false),
-      is_sdch_response_(false) {
+      is_sdch_response_(false),
+      response_code_(-1) {
   }
 
   void SetBufferSize(int buffer_size) { buffer_size_ = buffer_size; }
@@ -25,6 +26,7 @@ class MockFilterContext : public FilterContext {
   void SetURL(const GURL& gurl) { gurl_ = gurl; }
   void SetRequestTime(const base::Time time) { request_time_ = time; }
   void SetCached(bool is_cached) { is_cached_content_ = is_cached; }
+  void SetResponseCode(int response_code) { response_code_ = response_code; }
   void SetSdchResponse(bool is_sdch_response) {
     is_sdch_response_ = is_sdch_response;
   }
@@ -55,6 +57,8 @@ class MockFilterContext : public FilterContext {
   // How many bytes were fed to filter(s) so far?
   virtual int64 GetByteReadCount() const { return 0; }
 
+  virtual int GetResponseCode() const { return response_code_; }
+
   // What is the desirable input buffer size for these filters?
   virtual int GetInputStreamBufferSize() const { return buffer_size_; }
 
@@ -66,6 +70,7 @@ class MockFilterContext : public FilterContext {
   base::Time request_time_;
   bool is_cached_content_;
   bool is_sdch_response_;
+  int response_code_;
 
   DISALLOW_COPY_AND_ASSIGN(MockFilterContext);
 };
diff --git a/net/base/gzip_filter_unittest.cc b/net/base/gzip_filter_unittest.cc
index c656450..3501a96 100644
--- a/net/base/gzip_filter_unittest.cc
+++ b/net/base/gzip_filter_unittest.cc
@@ -279,6 +279,9 @@ TEST_F(GZipUnitTest, DecodeGZipWithMistakenSdch) {
   filter_types.push_back(Filter::FILTER_TYPE_SDCH);
   filter_types.push_back(Filter::FILTER_TYPE_GZIP);
   MockFilterContext filter_context(kDefaultBufferSize);
+  // We need a good response code to be sure that a proxy isn't injecting an
+  // error page (As is done by BlueCoat proxies and described in bug 8916).
+  filter_context.SetResponseCode(200);
   scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context));
   ASSERT_TRUE(filter.get());
   memcpy(filter->stream_buffer()->data(), gzip_encode_buffer_,
diff --git a/net/base/sdch_filter.cc b/net/base/sdch_filter.cc
index 80d6991..1cea7fc0 100644
--- a/net/base/sdch_filter.cc
+++ b/net/base/sdch_filter.cc
@@ -227,7 +227,21 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
       DCHECK(DECODING_ERROR == decoding_status_);
       DCHECK(0 == dest_buffer_excess_index_);
       DCHECK(dest_buffer_excess_.empty());
-      if (possible_pass_through_) {
+      // This is where we try very hard to do error recovery, and make this
+      // protocol robust in teh face of proxies that do many different things.
+      // If we decide that things are looking very bad (too hard to recover),
+      // we may even issue a "meta-refresh" to reload the page without an SDCH
+      // advertisement (so that we are sure we're not hurting anything).  First
+      // we try for some light weight recovery, and teh final else clause below
+      // supports the last ditch meta-refresh approach.
+      //
+      // Watch out for an error page inserted by the proxy as part of a 40x
+      // error response.  When we see such content molestation, we certainly
+      // need to fall into the meta-refresh case.
+      bool successful_response = filter_context().GetResponseCode() == 200;
+      if (possible_pass_through_ && successful_response) {
+        // This is the most graceful response. There really was no error. We
+        // were just overly cautious.
         // We added the sdch coding tag, and it should not have been added.
         // This can happen in server experiments, where the server decides
         // not to use sdch, even though there is a dictionary.  To be
@@ -236,7 +250,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
         SdchManager::SdchErrorRecovery(SdchManager::DISCARD_TENTATIVE_SDCH);
         decoding_status_ = PASS_THROUGH;
         dest_buffer_excess_ = dictionary_hash_;  // Send what we scanned.
-      } else if (!dictionary_hash_is_plausible_) {
+      } else if (successful_response && !dictionary_hash_is_plausible_) {
         // One of the first 9 bytes precluded consideration as a hash.
         // This can't be an SDCH payload, even though the server said it was.
         // This is a major error, as the server or proxy tagged this SDCH even
@@ -246,11 +260,16 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
         decoding_status_ = PASS_THROUGH;
         dest_buffer_excess_ = dictionary_hash_;  // Send what we scanned.
       } else {
-        // We don't have the dictionary that was demanded.
+        // This is where we try to do the expensive meta-refresh.
+        // Either this was an error response (probably an error page inserted
+        // by a proxy, as in bug 8916) or else we don't have the dictionary that
+        // was demanded.
         // With very low probability, random garbage data looked like a
         // dictionary specifier (8 ASCII characters followed by a null), but
         // that is sufficiently unlikely that we ignore it.
-        if (std::string::npos == mime_type_.find_first_of("text/html")) {
+        if (std::string::npos == mime_type_.find("text/html")) {
+          // Since we can't do a meta-refresh (along with an exponential
+          // backoff), we'll just make sure this NEVER happens again.
           SdchManager::BlacklistDomainForever(url_);
           if (was_cached_)
             SdchManager::SdchErrorRecovery(
diff --git a/net/base/sdch_filter_unittest.cc b/net/base/sdch_filter_unittest.cc
index 6f3ee62..f79dc8e 100644
--- a/net/base/sdch_filter_unittest.cc
+++ b/net/base/sdch_filter_unittest.cc
@@ -147,7 +147,7 @@ static std::string NewSdchDictionary(const std::string& domain) {
 
 //------------------------------------------------------------------------------
 
-TEST_F(SdchFilterTest, BasicBadDictionary) {
+TEST_F(SdchFilterTest, EmptyInputOk) {
   std::vector<Filter::FilterType> filter_types;
   filter_types.push_back(Filter::FILTER_TYPE_SDCH);
   const int kInputBufferSize(30);
@@ -165,8 +165,183 @@ TEST_F(SdchFilterTest, BasicBadDictionary) {
 
   EXPECT_EQ(0, output_bytes_or_buffer_size);
   EXPECT_EQ(Filter::FILTER_NEED_MORE_DATA, status);
+}
+
+TEST_F(SdchFilterTest, PassThroughWhenTentative) {
+  std::vector<Filter::FilterType> filter_types;
+  // Selective a tentative filter (which can fall back to pass through).
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE);
+  const int kInputBufferSize(30);
+  char output_buffer[20];
+  MockFilterContext filter_context(kInputBufferSize);
+  // Response code needs to be 200 to allow a pass through.
+  filter_context.SetResponseCode(200);
+  std::string url_string("http://ignore.com");
+  filter_context.SetURL(GURL(url_string));
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context));
+
+  // Supply enough data to force a pass-through mode, which means we have
+  // provided more than 9 characters that can't be a dictionary hash.
+  std::string non_sdch_content("This is not SDCH");
+
+  char* input_buffer = filter->stream_buffer()->data();
+  int input_buffer_size = filter->stream_buffer_size();
+  EXPECT_EQ(kInputBufferSize, input_buffer_size);
+
+  EXPECT_LT(static_cast<int>(non_sdch_content.size()),
+            input_buffer_size);
+  memcpy(input_buffer, non_sdch_content.data(),
+         non_sdch_content.size());
+  filter->FlushStreamBuffer(non_sdch_content.size());
+
+  // Try to read output.
+  int output_bytes_or_buffer_size = sizeof(output_buffer);
+  Filter::FilterStatus status = filter->ReadData(output_buffer,
+                                                 &output_bytes_or_buffer_size);
+
+  EXPECT_TRUE(non_sdch_content.size() == output_bytes_or_buffer_size);
+  ASSERT_TRUE(sizeof(output_buffer) > output_bytes_or_buffer_size);
+  output_buffer[output_bytes_or_buffer_size] = '\0';
+  EXPECT_TRUE(non_sdch_content == output_buffer);
+  EXPECT_EQ(Filter::FILTER_NEED_MORE_DATA, status);
+}
+
+TEST_F(SdchFilterTest, RefreshBadReturnCode) {
+  std::vector<Filter::FilterType> filter_types;
+  // Selective a tentative filter (which can fall back to pass through).
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE);
+  const int kInputBufferSize(30);
+  char output_buffer[20];
+  MockFilterContext filter_context(kInputBufferSize);
+  // Response code needs to be 200 to allow a pass through.
+  filter_context.SetResponseCode(403);
+  // Meta refresh will only appear for html content
+  filter_context.SetMimeType("text/html");
+  std::string url_string("http://ignore.com");
+  filter_context.SetURL(GURL(url_string));
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context));
+
+  // Supply enough data to force a pass-through mode, which means we have
+  // provided more than 9 characters that can't be a dictionary hash.
+  std::string non_sdch_content("This is not SDCH");
+
+  char* input_buffer = filter->stream_buffer()->data();
+  int input_buffer_size = filter->stream_buffer_size();
+  EXPECT_EQ(kInputBufferSize, input_buffer_size);
+
+  EXPECT_LT(static_cast<int>(non_sdch_content.size()),
+            input_buffer_size);
+  memcpy(input_buffer, non_sdch_content.data(),
+         non_sdch_content.size());
+  filter->FlushStreamBuffer(non_sdch_content.size());
+
+  // Try to read output.
+  int output_bytes_or_buffer_size = sizeof(output_buffer);
+  Filter::FilterStatus status = filter->ReadData(output_buffer,
+                                                 &output_bytes_or_buffer_size);
+
+  // We should have read a long and complicated meta-refresh request.
+  EXPECT_TRUE(sizeof(output_buffer) == output_bytes_or_buffer_size);
+  // Check at least the prefix of the return.
+  EXPECT_EQ(0, strncmp(output_buffer,
+      "<head><META HTTP-EQUIV=\"Refresh\" CONTENT=\"0\"></head>",
+      sizeof(output_buffer)));
+  EXPECT_EQ(Filter::FILTER_OK, status);
+}
+
+TEST_F(SdchFilterTest, ErrorOnBadReturnCode) {
+  std::vector<Filter::FilterType> filter_types;
+  // Selective a tentative filter (which can fall back to pass through).
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE);
+  const int kInputBufferSize(30);
+  char output_buffer[20];
+  MockFilterContext filter_context(kInputBufferSize);
+  // Response code needs to be 200 to allow a pass through.
+  filter_context.SetResponseCode(403);
+  // Meta refresh will only appear for html content, so set to something else
+  // to induce an error (we can't meta refresh).
+  filter_context.SetMimeType("anything");
+  std::string url_string("http://ignore.com");
+  filter_context.SetURL(GURL(url_string));
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context));
+
+  // Supply enough data to force a pass-through mode, which means we have
+  // provided more than 9 characters that can't be a dictionary hash.
+  std::string non_sdch_content("This is not SDCH");
+
+  char* input_buffer = filter->stream_buffer()->data();
+  int input_buffer_size = filter->stream_buffer_size();
+  EXPECT_EQ(kInputBufferSize, input_buffer_size);
+
+  EXPECT_LT(static_cast<int>(non_sdch_content.size()),
+            input_buffer_size);
+  memcpy(input_buffer, non_sdch_content.data(),
+         non_sdch_content.size());
+  filter->FlushStreamBuffer(non_sdch_content.size());
+
+  // Try to read output.
+  int output_bytes_or_buffer_size = sizeof(output_buffer);
+  Filter::FilterStatus status = filter->ReadData(output_buffer,
+                                                 &output_bytes_or_buffer_size);
+
+  EXPECT_EQ(0, output_bytes_or_buffer_size);
+  EXPECT_EQ(Filter::FILTER_ERROR, status);
+}
+
+TEST_F(SdchFilterTest, ErrorOnBadReturnCodeWithHtml) {
+  std::vector<Filter::FilterType> filter_types;
+  // Selective a tentative filter (which can fall back to pass through).
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE);
+  const int kInputBufferSize(30);
+  char output_buffer[20];
+  MockFilterContext filter_context(kInputBufferSize);
+  // Response code needs to be 200 to allow a pass through.
+  filter_context.SetResponseCode(403);
+  // Meta refresh will only appear for html content
+  filter_context.SetMimeType("text/html");
+  std::string url_string("http://ignore.com");
+  filter_context.SetURL(GURL(url_string));
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context));
+
+  // Supply enough data to force a pass-through mode, which means we have
+  // provided more than 9 characters that can't be a dictionary hash.
+  std::string non_sdch_content("This is not SDCH");
+
+  char* input_buffer = filter->stream_buffer()->data();
+  int input_buffer_size = filter->stream_buffer_size();
+  EXPECT_EQ(kInputBufferSize, input_buffer_size);
+
+  EXPECT_LT(static_cast<int>(non_sdch_content.size()),
+            input_buffer_size);
+  memcpy(input_buffer, non_sdch_content.data(),
+         non_sdch_content.size());
+  filter->FlushStreamBuffer(non_sdch_content.size());
+
+  // Try to read output.
+  int output_bytes_or_buffer_size = sizeof(output_buffer);
+  Filter::FilterStatus status = filter->ReadData(output_buffer,
+                                                 &output_bytes_or_buffer_size);
+
+  // We should have read a long and complicated meta-refresh request.
+  EXPECT_EQ(sizeof(output_buffer), output_bytes_or_buffer_size);
+  // Check at least the prefix of the return.
+  EXPECT_EQ(0, strncmp(output_buffer,
+      "<head><META HTTP-EQUIV=\"Refresh\" CONTENT=\"0\"></head>",
+      sizeof(output_buffer)));
+  EXPECT_EQ(Filter::FILTER_OK, status);
+}
+
+TEST_F(SdchFilterTest, BasicBadDictionary) {
+  std::vector<Filter::FilterType> filter_types;
+  filter_types.push_back(Filter::FILTER_TYPE_SDCH);
+  const int kInputBufferSize(30);
+  char output_buffer[20];
+  MockFilterContext filter_context(kInputBufferSize);
+  std::string url_string("http://ignore.com");
+  filter_context.SetURL(GURL(url_string));
+  scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context));
 
-  // Supply bogus data (which doesnt't yet specify a full dictionary hash).
+  // Supply bogus data (which doesn't yet specify a full dictionary hash).
   // Dictionary hash is 8 characters followed by a null.
   std::string dictionary_hash_prefix("123");
 
@@ -181,8 +356,9 @@ TEST_F(SdchFilterTest, BasicBadDictionary) {
   filter->FlushStreamBuffer(dictionary_hash_prefix.size());
 
   // With less than a dictionary specifier, try to read output.
-  output_bytes_or_buffer_size = sizeof(output_buffer);
-  status = filter->ReadData(output_buffer, &output_bytes_or_buffer_size);
+  int output_bytes_or_buffer_size = sizeof(output_buffer);
+  Filter::FilterStatus status = filter->ReadData(output_buffer,
+                                                 &output_bytes_or_buffer_size);
 
   EXPECT_EQ(0, output_bytes_or_buffer_size);
   EXPECT_EQ(Filter::FILTER_NEED_MORE_DATA, status);
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 90a06a7..c0a2080 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -182,7 +182,7 @@ bool URLRequestHttpJob::GetResponseCookies(
   return true;
 }
 
-int URLRequestHttpJob::GetResponseCode() {
+int URLRequestHttpJob::GetResponseCode() const {
   DCHECK(transaction_.get());
 
   if (!response_info_)
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index dbbdfab..19ecdf86 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -42,7 +42,7 @@ class URLRequestHttpJob : public URLRequestJob {
   virtual bool GetCharset(std::string* charset);
   virtual void GetResponseInfo(net::HttpResponseInfo* info);
   virtual bool GetResponseCookies(std::vector<std::string>* cookies);
-  virtual int GetResponseCode();
+  virtual int GetResponseCode() const;
   virtual bool GetContentEncodings(
       std::vector<Filter::FilterType>* encoding_type);
   virtual bool IsSdchResponse() const;
diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h
index f99ffdf..af12149 100644
--- a/net/url_request/url_request_job.h
+++ b/net/url_request/url_request_job.h
@@ -105,7 +105,7 @@ class URLRequestJob : public base::RefCountedThreadSafe<URLRequestJob>,
   }
 
   // Returns the HTTP response code for the request.
-  virtual int GetResponseCode() { return -1; }
+  virtual int GetResponseCode() const { return -1; }
 
   // Called to fetch the encoding types for this request. Only makes sense for
   // some types of requests. Returns true on success. Calling this on a request
@@ -195,10 +195,10 @@ class URLRequestJob : public base::RefCountedThreadSafe<URLRequestJob>,
   // FilterContext methods:
   // These methods are not applicable to all connections.
   virtual bool GetMimeType(std::string* mime_type) const { return false; }
-  virtual int64 GetByteReadCount() const;
   virtual bool GetURL(GURL* gurl) const;
   virtual base::Time GetRequestTime() const;
   virtual bool IsCachedContent() const;
+  virtual int64 GetByteReadCount() const;
   virtual int GetInputStreamBufferSize() const { return kFilterBufSize; }
 
  protected:
diff --git a/net/url_request/url_request_new_ftp_job.cc b/net/url_request/url_request_new_ftp_job.cc
index 3101783..be2f2c6 100644
--- a/net/url_request/url_request_new_ftp_job.cc
+++ b/net/url_request/url_request_new_ftp_job.cc
@@ -56,7 +56,7 @@ void URLRequestNewFtpJob::GetResponseInfo() {
   NOTIMPLEMENTED();
 }
 
-int URLRequestNewFtpJob::GetResponseCode() {
+int URLRequestNewFtpJob::GetResponseCode() const {
   NOTIMPLEMENTED();
   return -1;
 }
diff --git a/net/url_request/url_request_new_ftp_job.h b/net/url_request/url_request_new_ftp_job.h
index 79b165c..2d3f186 100644
--- a/net/url_request/url_request_new_ftp_job.h
+++ b/net/url_request/url_request_new_ftp_job.h
@@ -31,7 +31,7 @@ class URLRequestNewFtpJob : public URLRequestJob {
   virtual void Kill();
   virtual uint64 GetUploadProgress() const;
   virtual void GetResponseInfo();
-  virtual int GetResponseCode();
+  virtual int GetResponseCode() const;
   virtual bool GetMoreData();
   virtual bool ReadRawData(net::IOBuffer* buf, int buf_size, int *bytes_read);