This looked good on trunk, but an issue was tracked down after I merged. Reverting for now.

Revert 32902 - Merge 32856  A large ContentLength header followed by a connection close could trigger an out of memory condition.  Fixed problem, added unit test, and clarified the API.  This is probably the real problem in issue 25826.

BUG=28346, 25826
TEST=HttpNetworkTransactionTest.LargeContentLengthThenReset

Review URL: http://codereview.chromium.org/418035


TBR=tony@chromium.org

git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@32907 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 17 insertions, 28 deletions

diff --git a/net/base/io_buffer.cc b/net/base/io_buffer.cc
index 9ad0570..4aa469a 100644
--- a/net/base/io_buffer.cc
+++ b/net/base/io_buffer.cc
@@ -20,9 +20,11 @@ void DrainableIOBuffer::SetOffset(int bytes) {
 }
 
 void GrowableIOBuffer::SetCapacity(int capacity) {
-  DCHECK(capacity >= 0);
+  CHECK(capacity >= 0);
   // realloc will crash if it fails.
   real_data_.reset(static_cast<char*>(realloc(real_data_.release(), capacity)));
+  // Sanity check.
+  CHECK(real_data_.get() != NULL || capacity == 0);
   capacity_ = capacity;
   if (offset_ > capacity)
     set_offset(capacity);
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index a17e6e9..f12fd1f 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -3876,15 +3876,4 @@ TEST_F(HttpNetworkTransactionTest, HTTPSViaProxyWithExtraData) {
   EXPECT_EQ(ERR_TUNNEL_CONNECTION_FAILED, rv);
 }
 
-TEST_F(HttpNetworkTransactionTest, LargeContentLengthThenClose) {
-  MockRead data_reads[] = {
-    MockRead("HTTP/1.0 200 OK\r\nContent-Length:6719476739\r\n\r\n"),
-    MockRead(false, OK),
-  };
-  SimpleGetHelperResult out = SimpleGetHelper(data_reads);
-  EXPECT_EQ(OK, out.rv);
-  EXPECT_EQ("HTTP/1.0 200 OK", out.status_line);
-  EXPECT_EQ("", out.response_data);
-}
-
 }  // namespace net
diff --git a/net/http/http_stream.h b/net/http/http_stream.h
index 329db5a..d0b9dc9 100644
--- a/net/http/http_stream.h
+++ b/net/http/http_stream.h
@@ -50,14 +50,13 @@ class HttpStream {
   // Provides access to HttpResponseInfo (owned by HttpStream).
   virtual HttpResponseInfo* GetResponseInfo() const = 0;
 
-  // Reads response body data, up to |buf_len| bytes. |buf_len| should be a
-  // reasonable size (<256KB). The number of bytes read is returned, or an
-  // error is returned upon failure.  ERR_CONNECTION_CLOSED is returned to
-  // indicate end-of-connection.  ERR_IO_PENDING is returned if the operation
-  // could not be completed synchronously, in which case the result will be
-  // passed to the callback when available. If the operation is not completed
-  // immediately, the socket acquires a reference to the provided buffer until
-  // the callback is invoked or the socket is destroyed.
+  // Reads response body data, up to |buf_len| bytes.  The number of bytes read
+  // is returned, or an error is returned upon failure.  ERR_CONNECTION_CLOSED
+  // is returned to indicate end-of-connection.  ERR_IO_PENDING is returned if
+  // the operation could not be completed synchronously, in which case the
+  // result will be passed to the callback when available. If the operation is
+  // not completed immediately, the socket acquires a reference to the provided
+  // buffer until the callback is invoked or the socket is destroyed.
   virtual int ReadResponseBody(IOBuffer* buf, int buf_len,
                                CompletionCallback* callback) = 0;
 
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index d02c691..f392cef 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -84,7 +84,6 @@ int HttpStreamParser::ReadResponseBody(IOBuffer* buf, int buf_len,
   DCHECK(io_state_ == STATE_BODY_PENDING || io_state_ == STATE_DONE);
   DCHECK(!user_callback_);
   DCHECK(callback);
-  DCHECK_LE(buf_len, kMaxHeaderBufSize);
 
   if (io_state_ == STATE_DONE)
     return OK;
@@ -373,15 +372,15 @@ int HttpStreamParser::DoReadBodyComplete(int result) {
     if (chunked_decoder_.get()) {
       save_amount = chunked_decoder_->bytes_after_eof();
     } else if (response_body_length_ >= 0) {
-      int64 extra_data_read = response_body_read_ - response_body_length_;
-      if (extra_data_read > 0) {
-        save_amount = static_cast<int>(extra_data_read);
-        if (result > 0)
-          result -= save_amount;
-      }
+      save_amount = static_cast<int>(response_body_read_ -
+                                     response_body_length_);
+      if (save_amount < 0)
+        save_amount = 0;
+
+      if (result > 0)
+        result -= save_amount;
     }
 
-    CHECK(save_amount + additional_save_amount <= kMaxHeaderBufSize);
     if (read_buf_->capacity() < save_amount + additional_save_amount) {
       read_buf_->SetCapacity(save_amount + additional_save_amount);
     }