Allow URLRequest::AppendChunkToUpload to take 0-byte final chunks.

net/http/http_stream_parser_unittest.cc

Index: net/http/http_stream_parser_unittest.cc
diff --git a/net/http/http_stream_parser_unittest.cc b/net/http/http_stream_parser_unittest.cc
index 01d4e120ab1b583bae90aef127db07654e16224b..aa45b71373560682bec527e367e25156e7b9a7e8 100644
--- a/net/http/http_stream_parser_unittest.cc
+++ b/net/http/http_stream_parser_unittest.cc
@@ -322,6 +322,316 @@ TEST(HttpStreamParser, AsyncChunkAndAsyncSocket) {
   ASSERT_EQ(kBodySize, rv);
 }
 
+// Test to ensure the HttpStreamParser state machine does not get confused
+// when the final "chunk" has 0 bytes, and is received from the UploadStream
+// only after sending other chunk.
+TEST(HttpStreamParser, AsyncChunkWithEmptyFinalChunk) {
+  const char kChunk[] = "Chunk";
+
+  MockWrite writes[] = {
+      MockWrite(ASYNC, 0,
+                "GET /one.html HTTP/1.1\r\n"
+                "Host: localhost\r\n"
+                "Transfer-Encoding: chunked\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+      MockWrite(ASYNC, 1, "5\r\nChunk\r\n"),
+      MockWrite(ASYNC, 2, "0\r\n\r\n"),
+  };
+
+  // The size of the response body, as reflected in the Content-Length of the
+  // MockRead below.
+  const int kBodySize = 8;
+
+  MockRead reads[] = {
+      MockRead(ASYNC, 3, "HTTP/1.1 200 OK\r\n"),
+      MockRead(ASYNC, 4, "Content-Length: 8\r\n\r\n"),
+      MockRead(ASYNC, 5, "one.html"),
+      MockRead(SYNCHRONOUS, 0, 6),  // EOF
+  };
+
+  ChunkedUploadDataStream upload_stream(0);
+  upload_stream.AppendData(kChunk, arraysize(kChunk) - 1, false);
+  ASSERT_EQ(OK, upload_stream.Init(TestCompletionCallback().callback()));
+
+  DeterministicSocketData data(reads, arraysize(reads), writes,
+                               arraysize(writes));
+  data.set_connect_data(MockConnect(SYNCHRONOUS, OK));
+
+  scoped_ptr<DeterministicMockTCPClientSocket> transport(
+      new DeterministicMockTCPClientSocket(NULL, &data));
+  data.set_delegate(transport->AsWeakPtr());
+
+  TestCompletionCallback callback;
+  int rv = transport->Connect(callback.callback());
+  rv = callback.GetResult(rv);
+  ASSERT_EQ(OK, rv);
+
+  scoped_ptr<ClientSocketHandle> socket_handle(new ClientSocketHandle);
+  socket_handle->SetSocket(transport.Pass());
+
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("http://localhost");
+  request_info.load_flags = LOAD_NORMAL;
+  request_info.upload_data_stream = &upload_stream;

[mef, 2015/02/11 21:32:10]

is it possible to set upload_data_stream to stream which doesn't have any data
yet?

[mmenke, 2015/02/11 22:37:22]

Yes...SyncEmptyChunkedUpload is the only test in the file that does that.  We
really don't have many tests for the HttpStreamParser, sadly.  I've added a
couple basic sync/async tests.

Also noticed a silly bug in the logic to connect the mock socket in these tests,
which I've fixed.

+
+  scoped_refptr<GrowableIOBuffer> read_buffer(new GrowableIOBuffer);
+  HttpStreamParser parser(socket_handle.get(), &request_info, read_buffer.get(),
+                          BoundNetLog());
+
+  HttpRequestHeaders request_headers;
+  request_headers.SetHeader("Host", "localhost");
+  request_headers.SetHeader("Transfer-Encoding", "chunked");
+  request_headers.SetHeader("Connection", "keep-alive");
+
+  HttpResponseInfo response_info;
+  // This will attempt to Write() the initial request and headers, which will
+  // complete asynchronously.
+  rv = parser.SendRequest("GET /one.html HTTP/1.1\r\n", request_headers,
+                          &response_info, callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+
+  // Complete the initial request write and the chunk with data.
+  data.RunFor(2);
+  ASSERT_FALSE(callback.have_result());
+
+  // Now append the terminal 0-byte "chunk".
+  upload_stream.AppendData(nullptr, 0, true);
+  ASSERT_FALSE(callback.have_result());
+
+  // Finalize writing the trailer.
+  data.RunFor(1);
+  ASSERT_TRUE(callback.have_result());
+
+  // Warning: This will hang if the callback doesn't already have a result,
+  // due to the deterministic socket provider. Do not remove the above
+  // ASSERT_TRUE, which will avoid this hang.
+  rv = callback.WaitForResult();
+  ASSERT_EQ(OK, rv);
+
+  // Attempt to read the response status and the response headers.
+  rv = parser.ReadResponseHeaders(callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+  data.RunFor(2);
+
+  ASSERT_TRUE(callback.have_result());
+  rv = callback.WaitForResult();
+  ASSERT_GT(rv, 0);
+
+  // Finally, attempt to read the response body.
+  scoped_refptr<IOBuffer> body_buffer(new IOBuffer(kBodySize));
+  rv = parser.ReadResponseBody(body_buffer.get(), kBodySize,
+                               callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+  data.RunFor(1);
+
+  ASSERT_TRUE(callback.have_result());
+  rv = callback.WaitForResult();
+  ASSERT_EQ(kBodySize, rv);
+}
+
+// Test to ensure the HttpStreamParser state machine does not get confused
+// when there's only one "chunk" with 0 bytes, and is received from the
+// UploadStream only after sending the request headers successfully.
+TEST(HttpStreamParser, AsyncEmptyChunkedUpload) {
+  MockWrite writes[] = {
+      MockWrite(ASYNC, 0,
+                "GET /one.html HTTP/1.1\r\n"
+                "Host: localhost\r\n"
+                "Transfer-Encoding: chunked\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+      MockWrite(ASYNC, 1, "0\r\n\r\n"),
+  };
+
+  // The size of the response body, as reflected in the Content-Length of the
+  // MockRead below.
+  const int kBodySize = 8;
+
+  MockRead reads[] = {
+      MockRead(ASYNC, 2, "HTTP/1.1 200 OK\r\n"),
+      MockRead(ASYNC, 3, "Content-Length: 8\r\n\r\n"),
+      MockRead(ASYNC, 4, "one.html"),
+      MockRead(SYNCHRONOUS, 0, 5),  // EOF
+  };
+
+  ChunkedUploadDataStream upload_stream(0);
+  ASSERT_EQ(OK, upload_stream.Init(TestCompletionCallback().callback()));
+
+  DeterministicSocketData data(reads, arraysize(reads), writes,
+                               arraysize(writes));
+  data.set_connect_data(MockConnect(SYNCHRONOUS, OK));
+
+  scoped_ptr<DeterministicMockTCPClientSocket> transport(
+      new DeterministicMockTCPClientSocket(NULL, &data));
+  data.set_delegate(transport->AsWeakPtr());
+
+  TestCompletionCallback callback;
+  int rv = transport->Connect(callback.callback());
+  rv = callback.GetResult(rv);
+  ASSERT_EQ(OK, rv);
+
+  scoped_ptr<ClientSocketHandle> socket_handle(new ClientSocketHandle);
+  socket_handle->SetSocket(transport.Pass());
+
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("http://localhost");
+  request_info.load_flags = LOAD_NORMAL;
+  request_info.upload_data_stream = &upload_stream;
+
+  scoped_refptr<GrowableIOBuffer> read_buffer(new GrowableIOBuffer);
+  HttpStreamParser parser(socket_handle.get(), &request_info, read_buffer.get(),
+                          BoundNetLog());
+
+  HttpRequestHeaders request_headers;
+  request_headers.SetHeader("Host", "localhost");
+  request_headers.SetHeader("Transfer-Encoding", "chunked");
+  request_headers.SetHeader("Connection", "keep-alive");
+
+  HttpResponseInfo response_info;
+  // This will attempt to Write() the initial request and headers, which will
+  // complete asynchronously.
+  rv = parser.SendRequest("GET /one.html HTTP/1.1\r\n", request_headers,
+                          &response_info, callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+
+  // Complete writing the request headers.
+  data.RunFor(1);
+  ASSERT_FALSE(callback.have_result());
+
+  // Now append the terminal 0-byte "chunk".
+  upload_stream.AppendData(nullptr, 0, true);
+  ASSERT_FALSE(callback.have_result());
+
+  // Finalize writing the trailer.
+  data.RunFor(1);
+  ASSERT_TRUE(callback.have_result());
+
+  // Warning: This will hang if the callback doesn't already have a result,
+  // due to the deterministic socket provider. Do not remove the above
+  // ASSERT_TRUE, which will avoid this hang.
+  rv = callback.WaitForResult();
+  ASSERT_EQ(OK, rv);
+
+  // Attempt to read the response status and the response headers.
+  rv = parser.ReadResponseHeaders(callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+  data.RunFor(2);
+
+  ASSERT_TRUE(callback.have_result());
+  rv = callback.WaitForResult();
+  ASSERT_GT(rv, 0);
+
+  // Finally, attempt to read the response body.
+  scoped_refptr<IOBuffer> body_buffer(new IOBuffer(kBodySize));
+  rv = parser.ReadResponseBody(body_buffer.get(), kBodySize,
+                               callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+  data.RunFor(1);
+
+  ASSERT_TRUE(callback.have_result());
+  rv = callback.WaitForResult();
+  ASSERT_EQ(kBodySize, rv);
+}
+
+// Test to ensure the HttpStreamParser state machine does not get confused
+// when there's only one "chunk" with 0 bytes, which was already appended before
+// the request was started.
+TEST(HttpStreamParser, SyncEmptyChunkedUpload) {
+  MockWrite writes[] = {
+      MockWrite(ASYNC, 0,
+                "GET /one.html HTTP/1.1\r\n"
+                "Host: localhost\r\n"
+                "Transfer-Encoding: chunked\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+      MockWrite(ASYNC, 1, "0\r\n\r\n"),
+  };
+
+  // The size of the response body, as reflected in the Content-Length of the
+  // MockRead below.
+  const int kBodySize = 8;
+
+  MockRead reads[] = {
+      MockRead(ASYNC, 2, "HTTP/1.1 200 OK\r\n"),
+      MockRead(ASYNC, 3, "Content-Length: 8\r\n\r\n"),
+      MockRead(ASYNC, 4, "one.html"),
+      MockRead(SYNCHRONOUS, 0, 5),  // EOF
+  };
+
+  ChunkedUploadDataStream upload_stream(0);
+  ASSERT_EQ(OK, upload_stream.Init(TestCompletionCallback().callback()));
+  // Append final empty chunk.
+  upload_stream.AppendData(nullptr, 0, true);
+
+  DeterministicSocketData data(reads, arraysize(reads), writes,
+                               arraysize(writes));
+  data.set_connect_data(MockConnect(SYNCHRONOUS, OK));
+
+  scoped_ptr<DeterministicMockTCPClientSocket> transport(
+      new DeterministicMockTCPClientSocket(NULL, &data));
+  data.set_delegate(transport->AsWeakPtr());
+
+  TestCompletionCallback callback;
+  int rv = transport->Connect(callback.callback());
+  rv = callback.GetResult(rv);
+  ASSERT_EQ(OK, rv);
+
+  scoped_ptr<ClientSocketHandle> socket_handle(new ClientSocketHandle);
+  socket_handle->SetSocket(transport.Pass());
+
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("http://localhost");
+  request_info.load_flags = LOAD_NORMAL;
+  request_info.upload_data_stream = &upload_stream;
+
+  scoped_refptr<GrowableIOBuffer> read_buffer(new GrowableIOBuffer);
+  HttpStreamParser parser(socket_handle.get(), &request_info, read_buffer.get(),
+                          BoundNetLog());
+
+  HttpRequestHeaders request_headers;
+  request_headers.SetHeader("Host", "localhost");
+  request_headers.SetHeader("Transfer-Encoding", "chunked");
+  request_headers.SetHeader("Connection", "keep-alive");
+
+  HttpResponseInfo response_info;
+  // This will attempt to Write() the initial request and headers, which will
+  // complete asynchronously.
+  rv = parser.SendRequest("GET /one.html HTTP/1.1\r\n", request_headers,
+                          &response_info, callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+
+  // Complete writing the request headers and body.
+  data.RunFor(2);
+  ASSERT_TRUE(callback.have_result());
+
+  // Warning: This will hang if the callback doesn't already have a result,
+  // due to the deterministic socket provider. Do not remove the above
+  // ASSERT_TRUE, which will avoid this hang.
+  rv = callback.WaitForResult();
+  ASSERT_EQ(OK, rv);
+
+  // Attempt to read the response status and the response headers.
+  rv = parser.ReadResponseHeaders(callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+  data.RunFor(2);
+
+  ASSERT_TRUE(callback.have_result());
+  rv = callback.WaitForResult();
+  ASSERT_GT(rv, 0);
+
+  // Finally, attempt to read the response body.
+  scoped_refptr<IOBuffer> body_buffer(new IOBuffer(kBodySize));
+  rv = parser.ReadResponseBody(body_buffer.get(), kBodySize,
+                               callback.callback());
+  ASSERT_EQ(ERR_IO_PENDING, rv);
+  data.RunFor(1);
+
+  ASSERT_TRUE(callback.have_result());
+  rv = callback.WaitForResult();
+  ASSERT_EQ(kBodySize, rv);
+}
+
 TEST(HttpStreamParser, TruncatedHeaders) {
   MockRead truncated_status_reads[] = {
     MockRead(SYNCHRONOUS, 1, "HTTP/1.1 20"),