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

Unified Diff: net/http/http_stream_parser.cc

Issue 1884943003: HttpStreamParser: Don't reuse sockets which receive unparsed data. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: oops Created 4 years, 7 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_stream_parser.h ('k') | no next file » | 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 c718761ec7856dd132f14cfb211b977d58b7ed38..c6b87e26a21984dd0450e597e44be31906904a6f 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -214,6 +214,7 @@ HttpStreamParser::HttpStreamParser(ClientSocketHandle* connection,
sent_bytes_(0),
response_(nullptr),
response_body_length_(-1),
+ response_is_keep_alive_(false),
response_body_read_(0),
user_read_buf_(nullptr),
user_read_buf_len_(0),
@@ -869,6 +870,7 @@ int HttpStreamParser::HandleReadHeaderResult(int result) {
}
} 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.
@@ -892,11 +894,17 @@ int HttpStreamParser::HandleReadHeaderResult(int result) {
response_body_length_ = -1;
// Now waiting for the second set of headers to be read.
} else {
+ // Only set keep-alive based on final set of headers.
+ response_is_keep_alive_ = response_->headers->IsKeepAlive();
+
io_state_ = STATE_DONE;
}
return OK;
}
+ // Only set keep-alive based on final set of headers.
+ response_is_keep_alive_ = response_->headers->IsKeepAlive();
+
// Note where the headers stop.
read_buf_unused_offset_ = end_of_header_offset;
// Now waiting for the body to be read.
@@ -1087,8 +1095,20 @@ void HttpStreamParser::SetConnectionReused() {
bool HttpStreamParser::CanReuseConnection() const {
if (!CanFindEndOfResponse())
return false;
- if (!response_->headers || !response_->headers->IsKeepAlive())
+
+ if (!response_is_keep_alive_)
+ return false;
+
+ // Check if extra data was received after reading the entire response body. If
+ // extra data was received, reusing the socket is not a great idea. This does
+ // have the down side of papering over certain server bugs, but seems to be
+ // the best option here.
+ //
+ // TODO(mmenke): Consider logging this - hard to decipher socket reuse
+ // behavior makes NetLogs harder to read.
+ if (IsResponseBodyComplete() && IsMoreDataBuffered())
return false;
+
return connection_->socket() && connection_->socket()->IsConnected();
}
« no previous file with comments | « net/http/http_stream_parser.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698