Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(491)

Unified Diff: net/http/http_stream_parser.cc

Issue 1266713007: Net: Stop treating partial HTTP headers as a valid response. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Response to comments Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/http/http_network_transaction_unittest.cc ('k') | net/http/http_stream_parser_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_stream_parser.cc
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 3c3b88d76d93dfed25087181b31c58f9aed250c7..d27a4a2dd51004bbb11bc61424affd2214baf2b0 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -29,7 +29,7 @@ namespace {
enum HttpHeaderParserEvent {
HEADER_PARSER_INVOKED = 0,
HEADER_HTTP_09_RESPONSE = 1,
- HEADER_ALLOWED_TRUNCATED_HEADERS = 2,
+ // Obsolete: HEADER_ALLOWED_TRUNCATED_HEADERS = 2,
HEADER_SKIPPED_WS_PREFIX = 3,
HEADER_SKIPPED_NON_WS_PREFIX = 4,
NUM_HEADER_EVENTS
@@ -768,55 +768,46 @@ int HttpStreamParser::HandleReadHeaderResult(int result) {
if (result == 0)
result = ERR_CONNECTION_CLOSED;
- if (result < 0 && result != ERR_CONNECTION_CLOSED) {
- io_state_ = STATE_DONE;
- return result;
- }
- // If we've used the connection before, then we know it is not a HTTP/0.9
- // response and return ERR_CONNECTION_CLOSED.
- if (result == ERR_CONNECTION_CLOSED && read_buf_->offset() == 0 &&
- connection_->is_reused()) {
- io_state_ = STATE_DONE;
- return result;
- }
-
- // Record our best estimate of the 'response time' as the time when we read
- // the first bytes of the response headers.
- if (read_buf_->offset() == 0 && result != ERR_CONNECTION_CLOSED)
- response_->response_time = base::Time::Now();
-
if (result == ERR_CONNECTION_CLOSED) {
- // The connection closed before we detected the end of the headers.
if (read_buf_->offset() == 0) {
- // The connection was closed before any data was sent. Likely an error
- // rather than empty HTTP/0.9 response.
io_state_ = STATE_DONE;
- return ERR_EMPTY_RESPONSE;
- } else if (request_->url.SchemeIsCryptographic()) {
- // The connection was closed in the middle of the headers. For HTTPS we
- // don't parse partial headers. Return a different error code so that we
- // know that we shouldn't attempt to retry the request.
- io_state_ = STATE_DONE;
- return ERR_RESPONSE_HEADERS_TRUNCATED;
+ // If the connection has not been reused, it may have been a 0-length
+ // HTTP/0.9 responses, but it was most likely an error, so just return
+ // ERR_EMPTY_RESPONSE instead. If the connection was reused, just pass
+ // on the original connection close error, as rather than being an
+ // empty HTTP/0.9 response it's much more likely the server closed the
+ // socket before it received the request.
+ if (!connection_->is_reused())
+ return ERR_EMPTY_RESPONSE;
+ return result;
}
- // Parse things as well as we can and let the caller decide what to do.
- int end_offset;
+
+ // The connection closed before the end of the headers. For HTTPS,
+ // accepting the partial headers headers is a potential security
+ // vulnerability. For HTTP, it just doesn't seem like a good idea.
if (response_header_start_offset_ >= 0) {
- // The response looks to be a truncated set of HTTP headers.
- io_state_ = STATE_READ_BODY_COMPLETE;
- end_offset = read_buf_->offset();
- RecordHeaderParserEvent(HEADER_ALLOWED_TRUNCATED_HEADERS);
- } else {
- // The response is apparently using HTTP/0.9. Treat the entire response
- // the body.
- end_offset = 0;
+ io_state_ = STATE_DONE;
+ return ERR_RESPONSE_HEADERS_TRUNCATED;
}
- int rv = ParseResponseHeaders(end_offset);
+
+ // The response is apparently using HTTP/0.9. Treat the entire response as
+ // the body.
+ int rv = ParseResponseHeaders(0);
if (rv < 0)
return rv;
return result;
}
+ if (result < 0) {
+ io_state_ = STATE_DONE;
+ return result;
+ }
+
+ // Record our best estimate of the 'response time' as the time when we read
+ // the first bytes of the response headers.
+ if (read_buf_->offset() == 0)
+ response_->response_time = base::Time::Now();
+
read_buf_->set_offset(read_buf_->offset() + result);
DCHECK_LE(read_buf_->offset(), read_buf_->capacity());
DCHECK_GE(result, 0);
« no previous file with comments | « net/http/http_network_transaction_unittest.cc ('k') | net/http/http_stream_parser_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698