don't preserve 1xx responses in parser buffer
Bug: 11648225
This is a clean cherry pick from chromium code change
https://codereview.chromium.org/25312002
Change-Id: I00b8ede6250ebab58dc02206b96393f2008f0aa1
diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc
index 2274f25..a8db569 100644
--- a/net/http/http_proxy_client_socket_pool_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_unittest.cc
@@ -520,6 +520,37 @@
EXPECT_FALSE(handle_.socket());
}
+TEST_P(HttpProxyClientSocketPoolTest, Tunnel1xxResponse) {
+ // Tests that 1xx responses are rejected for a CONNECT request.
+ if (GetParam().proxy_type == SPDY) {
+ // SPDY doesn't have 1xx responses.
+ return;
+ }
+
+ MockWrite writes[] = {
+ MockWrite(ASYNC, 0,
+ "CONNECT www.google.com:443 HTTP/1.1\r\n"
+ "Host: www.google.com\r\n"
+ "Proxy-Connection: keep-alive\r\n\r\n"),
+ };
+ MockRead reads[] = {
+ MockRead(ASYNC, 1, "HTTP/1.1 100 Continue\r\n\r\n"),
+ MockRead(ASYNC, 2, "HTTP/1.1 200 Connection Established\r\n\r\n"),
+ };
+
+ Initialize(reads, arraysize(reads), writes, arraysize(writes),
+ NULL, 0, NULL, 0);
+
+ int rv = handle_.Init("a", CreateTunnelParams(), LOW, callback_.callback(),
+ &pool_, BoundNetLog());
+ EXPECT_EQ(ERR_IO_PENDING, rv);
+ EXPECT_FALSE(handle_.is_initialized());
+ EXPECT_FALSE(handle_.socket());
+
+ data_->RunFor(2);
+ EXPECT_EQ(ERR_TUNNEL_CONNECTION_FAILED, callback_.WaitForResult());
+}
+
TEST_P(HttpProxyClientSocketPoolTest, TunnelSetupError) {
MockWrite writes[] = {
MockWrite(ASYNC, 0,
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index e53aafd..853414a 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -79,7 +79,7 @@
// Example:
//
// scoped_refptr<SeekableIOBuffer> buf = new SeekableIOBuffer(1024);
-// // capacity() == 1024. size() == BytesRemaining == BytesConsumed() == 0.
+// // capacity() == 1024. size() == BytesRemaining() == BytesConsumed() == 0.
// // data() points to the beginning of the buffer.
//
// // Read() takes an IOBuffer.
@@ -94,7 +94,7 @@
// buf->DidConsume(bytes_written);
// }
// // BytesRemaining() == 0. BytesConsumed() == size().
-// // data() points to the end of the comsumed bytes (exclusive).
+// // data() points to the end of the consumed bytes (exclusive).
//
// // If you want to reuse the buffer, be sure to clear the buffer.
// buf->Clear();
@@ -161,7 +161,7 @@
}
char* real_data_;
- int capacity_;
+ const int capacity_;
int size_;
int used_;
};
@@ -293,6 +293,7 @@
DCHECK(io_state_ == STATE_REQUEST_SENT || io_state_ == STATE_DONE);
DCHECK(callback_.is_null());
DCHECK(!callback.is_null());
+ DCHECK_EQ(0, read_buf_unused_offset_);
// This function can be called with io_state_ == STATE_DONE if the
// connection is closed after seeing just a 1xx response code.
@@ -304,8 +305,8 @@
if (read_buf_->offset() > 0) {
// Simulate the state where the data was just read from the socket.
- result = read_buf_->offset() - read_buf_unused_offset_;
- read_buf_->set_offset(read_buf_unused_offset_);
+ result = read_buf_->offset();
+ read_buf_->set_offset(0);
}
if (result > 0)
io_state_ = STATE_READ_HEADERS_COMPLETE;
@@ -517,6 +518,8 @@
}
int HttpStreamParser::DoReadHeadersComplete(int result) {
+ DCHECK_EQ(0, read_buf_unused_offset_);
+
if (result == 0)
result = ERR_CONNECTION_CLOSED;
@@ -580,41 +583,43 @@
if (end_of_header_offset == -1) {
io_state_ = STATE_READ_HEADERS;
// Prevent growing the headers buffer indefinitely.
- if (read_buf_->offset() - read_buf_unused_offset_ >= kMaxHeaderBufSize) {
+ if (read_buf_->offset() >= kMaxHeaderBufSize) {
io_state_ = STATE_DONE;
return ERR_RESPONSE_HEADERS_TOO_BIG;
}
} else {
+ CalculateResponseBodySize();
+ // If the body is zero length, the caller may not call ReadResponseBody,
+ // which is where any extra data is copied to read_buf_, so we move the
+ // data here.
+ if (response_body_length_ == 0) {
+ int extra_bytes = read_buf_->offset() - end_of_header_offset;
+ if (extra_bytes) {
+ CHECK_GT(extra_bytes, 0);
+ memmove(read_buf_->StartOfBuffer(),
+ read_buf_->StartOfBuffer() + end_of_header_offset,
+ extra_bytes);
+ }
+ read_buf_->SetCapacity(extra_bytes);
+ if (response_->headers->response_code() / 100 == 1) {
+ // After processing a 1xx response, the caller will ask for the next
+ // header, so reset state to support that. We don't completely ignore a
+ // 1xx response because it cannot be returned in reply to a CONNECT
+ // request so we return OK here, which lets the caller inspect the
+ // response and reject it in the event that we're setting up a CONNECT
+ // tunnel.
+ response_header_start_offset_ = -1;
+ response_body_length_ = -1;
+ io_state_ = STATE_REQUEST_SENT;
+ } else {
+ io_state_ = STATE_DONE;
+ }
+ return OK;
+ }
+
// Note where the headers stop.
read_buf_unused_offset_ = end_of_header_offset;
-
- if (response_->headers->response_code() / 100 == 1) {
- // After processing a 1xx response, the caller will ask for the next
- // header, so reset state to support that. We don't just skip these
- // completely because 1xx codes aren't acceptable when establishing a
- // tunnel.
- io_state_ = STATE_REQUEST_SENT;
- response_header_start_offset_ = -1;
- } else {
- io_state_ = STATE_BODY_PENDING;
- CalculateResponseBodySize();
- // If the body is 0, the caller may not call ReadResponseBody, which
- // is where any extra data is copied to read_buf_, so we move the
- // data here and transition to DONE.
- if (response_body_length_ == 0) {
- io_state_ = STATE_DONE;
- int extra_bytes = read_buf_->offset() - read_buf_unused_offset_;
- if (extra_bytes) {
- CHECK_GT(extra_bytes, 0);
- memmove(read_buf_->StartOfBuffer(),
- read_buf_->StartOfBuffer() + read_buf_unused_offset_,
- extra_bytes);
- }
- read_buf_->SetCapacity(extra_bytes);
- read_buf_unused_offset_ = 0;
- return OK;
- }
- }
+ io_state_ = STATE_BODY_PENDING;
}
return result;
}
@@ -751,20 +756,19 @@
int HttpStreamParser::ParseResponseHeaders() {
int end_offset = -1;
+ DCHECK_EQ(0, read_buf_unused_offset_);
// Look for the start of the status line, if it hasn't been found yet.
if (response_header_start_offset_ < 0) {
response_header_start_offset_ = HttpUtil::LocateStartOfStatusLine(
- read_buf_->StartOfBuffer() + read_buf_unused_offset_,
- read_buf_->offset() - read_buf_unused_offset_);
+ read_buf_->StartOfBuffer(), read_buf_->offset());
}
if (response_header_start_offset_ >= 0) {
- end_offset = HttpUtil::LocateEndOfHeaders(
- read_buf_->StartOfBuffer() + read_buf_unused_offset_,
- read_buf_->offset() - read_buf_unused_offset_,
- response_header_start_offset_);
- } else if (read_buf_->offset() - read_buf_unused_offset_ >= 8) {
+ end_offset = HttpUtil::LocateEndOfHeaders(read_buf_->StartOfBuffer(),
+ read_buf_->offset(),
+ response_header_start_offset_);
+ } else if (read_buf_->offset() >= 8) {
// Enough data to decide that this is an HTTP/0.9 response.
// 8 bytes = (4 bytes of junk) + "http".length()
end_offset = 0;
@@ -776,14 +780,16 @@
int rv = DoParseResponseHeaders(end_offset);
if (rv < 0)
return rv;
- return end_offset + read_buf_unused_offset_;
+ return end_offset;
}
int HttpStreamParser::DoParseResponseHeaders(int end_offset) {
scoped_refptr<HttpResponseHeaders> headers;
+ DCHECK_EQ(0, read_buf_unused_offset_);
+
if (response_header_start_offset_ >= 0) {
headers = new HttpResponseHeaders(HttpUtil::AssembleRawHeaders(
- read_buf_->StartOfBuffer() + read_buf_unused_offset_, end_offset));
+ read_buf_->StartOfBuffer(), end_offset));
} else {
// Enough data was read -- there is no status line.
headers = new HttpResponseHeaders(std::string("HTTP/0.9 200 OK"));
@@ -830,13 +836,16 @@
// (informational), 204 (no content), and 304 (not modified) responses
// MUST NOT include a message-body. All other responses do include a
// message-body, although it MAY be of zero length.
- switch (response_->headers->response_code()) {
- // Note that 1xx was already handled earlier.
- case 204: // No Content
- case 205: // Reset Content
- case 304: // Not Modified
- response_body_length_ = 0;
- break;
+ if (response_->headers->response_code() / 100 == 1) {
+ response_body_length_ = 0;
+ } else {
+ switch (response_->headers->response_code()) {
+ case 204: // No Content
+ case 205: // Reset Content
+ case 304: // Not Modified
+ response_body_length_ = 0;
+ break;
+ }
}
if (request_->method == "HEAD")
response_body_length_ = 0;
diff --git a/net/http/http_stream_parser.h b/net/http/http_stream_parser.h
index a41e393..43e1514 100644
--- a/net/http/http_stream_parser.h
+++ b/net/http/http_stream_parser.h
@@ -178,7 +178,8 @@
scoped_refptr<GrowableIOBuffer> read_buf_;
// Offset of the first unused byte in |read_buf_|. May be nonzero due to
- // a 1xx header, or body data in the same packet as header data.
+ // body data in the same packet as header data but is zero when reading
+ // headers.
int read_buf_unused_offset_;
// The amount beyond |read_buf_unused_offset_| where the status line starts;
