Reland of HttpChunkedDecoder: Support chunks longer than 2^31-1 bytes....

Reland of HttpChunkedDecoder: Support chunks longer than 2^31-1 bytes. (patchset #1 id:1 of https://codereview.chromium.org/2183433003/ )

Reason for revert:
The CL was blamed for compile failures it didn't cause, re-landing.

Original issue's description:
> Revert of HttpChunkedDecoder: Support chunks longer than 2^31-1 bytes. (patchset #4 id:60001 of https://codereview.chromium.org/2170133004/ )
>
> Reason for revert:
> Speculative revert for bug 631246.  Have no better ideas.
>
> Apologies if this wasn't the cause.
>
> Original issue's description:
> > HttpChunkedDecoder: Support chunks longer than 2^31-1 bytes.
> >
> > We were using HexStringToInt to parse chunk size, which returns a
> > 32-bit int.  This CL switches to using HexStringToInt64, which uses
> > 64-bit ints, so we can now support chunks up to 2^63-1 bytes.
> >
> > That should be enough for anybody. [Cue dramatic music]
> >
> > BUG=630680
> >
> > Committed: https://crrev.com/8ea249f8b70f154f9995ed538fc853fe9cf46503
> > Cr-Commit-Position: refs/heads/master@{#407549}
>
> TBR=eroman@chromium.org,mmenke@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=630680
>
> Committed: https://crrev.com/2321e8a45603ca595296b67e4d064c070f2b3591
> Cr-Commit-Position: refs/heads/master@{#407651}

TBR=eroman@chromium.org,mpearson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=630680

Review-Url: https://codereview.chromium.org/2180063002
Cr-Commit-Position: refs/heads/master@{#407700}

net/http/http_chunked_decoder.cc

@@ -69,9 +69,12 @@ HttpChunkedDecoder::HttpChunkedDecoder()
 int HttpChunkedDecoder::FilterBuf(char* buf, int buf_len) {
   int result = 0;
 
-  while (buf_len) {
-    if (chunk_remaining_) {
-      int num = std::min(chunk_remaining_, buf_len);
+  while (buf_len > 0) {
+    if (chunk_remaining_ > 0) {
+      // Since |chunk_remaining_| is positive and |buf_len| an int, the minimum
+      // of the two must be an int.
+      int num = static_cast<int>(
+          std::min(chunk_remaining_, static_cast<int64_t>(buf_len)));
 
       buf_len -= num;
       chunk_remaining_ -= num;
@@ -79,8 +82,8 @@ int HttpChunkedDecoder::FilterBuf(char* buf, int buf_len) {
       result += num;
       buf += num;
 
-      // After each chunk's data there should be a CRLF
-      if (!chunk_remaining_)
+      // After each chunk's data there should be a CRLF.
+      if (chunk_remaining_ == 0)
         chunk_terminator_remaining_ = true;
       continue;
     } else if (reached_eof_) {
@@ -93,7 +96,7 @@ int HttpChunkedDecoder::FilterBuf(char* buf, int buf_len) {
       return bytes_consumed; // Error
 
     buf_len -= bytes_consumed;
-    if (buf_len)
+    if (buf_len > 0)
       memmove(buf, buf + bytes_consumed, buf_len);
   }
 
@@ -121,17 +124,17 @@ int HttpChunkedDecoder::ScanForChunkRemaining(const char* buf, int buf_len) {
     }
 
     if (reached_last_chunk_) {
-      if (buf_len)
+      if (buf_len > 0)
         DVLOG(1) << "ignoring http trailer";
       else
         reached_eof_ = true;
     } else if (chunk_terminator_remaining_) {
-      if (buf_len) {
+      if (buf_len > 0) {
         DLOG(ERROR) << "chunk data not terminated properly";
         return ERR_INVALID_CHUNKED_ENCODING;
       }
       chunk_terminator_remaining_ = false;
-    } else if (buf_len) {
+    } else if (buf_len > 0) {
       // Ignore any chunk-extensions.
       size_t index_of_semicolon = base::StringPiece(buf, buf_len).find(';');
       if (index_of_semicolon != base::StringPiece::npos)
@@ -189,14 +192,16 @@ int HttpChunkedDecoder::ScanForChunkRemaining(const char* buf, int buf_len) {
 // known sites.
 //
 //         Us: ^\X+[ ]*$
-bool HttpChunkedDecoder::ParseChunkSize(const char* start, int len, int* out) {
+bool HttpChunkedDecoder::ParseChunkSize(const char* start,
+                                        int len,
+                                        int64_t* out) {
   DCHECK_GE(len, 0);
 
   // Strip trailing spaces
-  while (len && start[len - 1] == ' ')
+  while (len > 0 && start[len - 1] == ' ')
     len--;
 
-  // Be more restrictive than HexStringToInt;
+  // Be more restrictive than HexStringToInt64;
   // don't allow inputs with leading "-", "+", "0x", "0X"
   base::StringPiece chunk_size(start, len);
   if (chunk_size.find_first_not_of("0123456789abcdefABCDEF")
@@ -204,8 +209,8 @@ bool HttpChunkedDecoder::ParseChunkSize(const char* start, int len, int* out) {
     return false;
   }
 
-  int parsed_number;
-  bool ok = base::HexStringToInt(chunk_size, &parsed_number);
+  int64_t parsed_number;
+  bool ok = base::HexStringToInt64(chunk_size, &parsed_number);
   if (ok && parsed_number >= 0) {
     *out = parsed_number;
     return true;

net/http/http_chunked_decoder.h

@@ -46,6 +46,7 @@
 #define NET_HTTP_HTTP_CHUNKED_DECODER_H_
 
 #include <stddef.h>
+#include <stdint.h>
 
 #include <string>
 
@@ -107,10 +108,10 @@ class NET_EXPORT_PRIVATE HttpChunkedDecoder {
   // Converts string |start| of length |len| to a numeric value.
   // |start| is a string of type "chunk-size" (hex string).
   // If the conversion succeeds, returns true and places the result in |out|.
-  static bool ParseChunkSize(const char* start, int len, int* out);
+  static bool ParseChunkSize(const char* start, int len, int64_t* out);
 
   // Indicates the number of bytes remaining for the current chunk.
-  int chunk_remaining_;
+  int64_t chunk_remaining_;
 
   // A small buffer used to store a partial chunk marker.
   std::string line_buf_;

net/http/http_chunked_decoder_unittest.cc

@@ -5,7 +5,11 @@
 #include "net/http/http_chunked_decoder.h"
 
 #include <memory>
+#include <string>
+#include <vector>
 
+#include "base/format_macros.h"
+#include "base/strings/stringprintf.h"
 #include "net/base/net_errors.h"
 #include "net/test/gtest_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -291,10 +295,70 @@ TEST(HttpChunkedDecoderTest, InvalidConsecutiveCRLFs) {
   RunTestUntilFailure(inputs, arraysize(inputs), 1);
 }
 
-TEST(HttpChunkedDecoderTest, ExcessiveChunkLen) {
-  const char* const inputs[] = {
-    "c0000000\r\nhello\r\n"
+TEST(HttpChunkedDecoderTest, ReallyBigChunks) {
+  // Number of bytes sent through the chunked decoder per loop iteration. To
+  // minimize runtime, should be the square root of the chunk lengths, below.
+  const int64_t kWrittenBytesPerIteration = 0x10000;
+
+  // Length of chunks to test. Must be multiples of kWrittenBytesPerIteration.
+  int64_t kChunkLengths[] = {
+      // Overflows when cast to a signed int32.
+      0x0c0000000,
+      // Overflows when cast to an unsigned int32.
+      0x100000000,
   };
+
+  for (int64_t chunk_length : kChunkLengths) {
+    HttpChunkedDecoder decoder;
+    EXPECT_FALSE(decoder.reached_eof());
+
+    // Feed just the header to the decode.
+    std::string chunk_header =
+        base::StringPrintf("%" PRIx64 "\r\n", chunk_length);
+    std::vector<char> data(chunk_header.begin(), chunk_header.end());
+    EXPECT_EQ(OK, decoder.FilterBuf(data.data(), data.size()));
+    EXPECT_FALSE(decoder.reached_eof());
+
+    // Set |data| to be kWrittenBytesPerIteration long, and have a repeating
+    // pattern.
+    data.clear();
+    data.reserve(kWrittenBytesPerIteration);
+    for (size_t i = 0; i < kWrittenBytesPerIteration; i++) {
+      data.push_back(static_cast<char>(i));
+    }
+
+    // Repeatedly feed the data to the chunked decoder. Since the data doesn't
+    // include any chunk lengths, the decode will never have to move the data,
+    // and should run fairly quickly.
+    for (int64_t total_written = 0; total_written < chunk_length;
+         total_written += kWrittenBytesPerIteration) {
+      EXPECT_EQ(kWrittenBytesPerIteration,
+                decoder.FilterBuf(data.data(), kWrittenBytesPerIteration));
+      EXPECT_FALSE(decoder.reached_eof());
+    }
+
+    // Chunk terminator and the final chunk.
+    char final_chunk[] = "\r\n0\r\n\r\n";
+    EXPECT_EQ(OK, decoder.FilterBuf(final_chunk, arraysize(final_chunk)));
+    EXPECT_TRUE(decoder.reached_eof());
+
+    // Since |data| never included any chunk headers, it should not have been
+    // modified.
+    for (size_t i = 0; i < kWrittenBytesPerIteration; i++) {
+      EXPECT_EQ(static_cast<char>(i), data[i]);
+    }
+  }
+}
+
+TEST(HttpChunkedDecoderTest, ExcessiveChunkLen) {
+  // Smallest number that can't be represented as a signed int64.
+  const char* const inputs[] = {"8000000000000000\r\nhello\r\n"};
+  RunTestUntilFailure(inputs, arraysize(inputs), 0);
+}
+
+TEST(HttpChunkedDecoderTest, ExcessiveChunkLen2) {
+  // Smallest number that can't be represented as an unsigned int64.
+  const char* const inputs[] = {"10000000000000000\r\nhello\r\n"};
   RunTestUntilFailure(inputs, arraysize(inputs), 0);
 }