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


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

4 files changed, 28 insertions, 17 deletions

diff --git a/net/base/io_buffer.cc b/net/base/io_buffer.cc
index 4aa469a..9ad0570 100644
--- a/net/base/io_buffer.cc
+++ b/net/base/io_buffer.cc
@@ -20,11 +20,9 @@ void DrainableIOBuffer::SetOffset(int bytes) {
 }
 
 void GrowableIOBuffer::SetCapacity(int capacity) {
-  CHECK(capacity >= 0);
+  DCHECK(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 f12fd1f..a17e6e9 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -3876,4 +3876,15 @@ 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 d0b9dc9..329db5a 100644
--- a/net/http/http_stream.h
+++ b/net/http/http_stream.h
@@ -50,13 +50,14 @@ class HttpStream {
   // Provides access to HttpResponseInfo (owned by HttpStream).
   virtual HttpResponseInfo* GetResponseInfo() const = 0;
 
-  // 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.
+  // 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.
   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 f392cef..d02c691 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -84,6 +84,7 @@ 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;
@@ -372,15 +373,15 @@ int HttpStreamParser::DoReadBodyComplete(int result) {
     if (chunked_decoder_.get()) {
       save_amount = chunked_decoder_->bytes_after_eof();
     } else if (response_body_length_ >= 0) {
-      save_amount = static_cast<int>(response_body_read_ -
-                                     response_body_length_);
-      if (save_amount < 0)
-        save_amount = 0;
-
-      if (result > 0)
-        result -= save_amount;
+      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;
+      }
     }
 
+    CHECK(save_amount + additional_save_amount <= kMaxHeaderBufSize);
     if (read_buf_->capacity() < save_amount + additional_save_amount) {
       read_buf_->SetCapacity(save_amount + additional_save_amount);
     }