Commit 5db2f4ea authored by ricea's avatar ricea Committed by Commit bot

Make HRH::IsKeepAlive() look past the first header

Up until now, HttpResponseHeaders::IsKeepAlive() only looked at the
first value of the Connection header. Since for a WebSocket response a
header like

Connection: Upgrade, close

is valid we should check other values past the first value.

BUG=371759
TEST=net_unittests

Review URL: https://codereview.chromium.org/640213002

Cr-Commit-Position: refs/heads/master@{#300087}
parent e1adb7ed
......@@ -1202,28 +1202,40 @@ bool HttpResponseHeaders::GetTimeValuedHeader(const std::string& name,
return Time::FromUTCString(value.c_str(), result);
}
// We accept the first value of "close" or "keep-alive" in a Connection or
// Proxy-Connection header, in that order. Obeying "keep-alive" in HTTP/1.1 or
// "close" in 1.0 is not strictly standards-compliant, but we'd like to
// avoid looking at the Proxy-Connection header whenever it is reasonable to do
// so.
// TODO(ricea): Measure real-world usage of the "Proxy-Connection" header,
// with a view to reducing support for it in order to make our Connection header
// handling more RFC 7230 compliant.
bool HttpResponseHeaders::IsKeepAlive() const {
if (http_version_ < HttpVersion(1, 0))
return false;
// NOTE: It is perhaps risky to assume that a Proxy-Connection header is
// meaningful when we don't know that this response was from a proxy, but
// Mozilla also does this, so we'll do the same.
std::string connection_val;
if (!EnumerateHeader(NULL, "connection", &connection_val))
EnumerateHeader(NULL, "proxy-connection", &connection_val);
static const char* kConnectionHeaders[] = {"connection", "proxy-connection"};
struct KeepAliveToken {
const char* token;
bool keep_alive;
};
static const KeepAliveToken kKeepAliveTokens[] = {{"keep-alive", true},
{"close", false}};
bool keep_alive;
if (http_version_ < HttpVersion(1, 0))
return false;
if (http_version_ == HttpVersion(1, 0)) {
// HTTP/1.0 responses default to NOT keep-alive
keep_alive = LowerCaseEqualsASCII(connection_val, "keep-alive");
} else {
// HTTP/1.1 responses default to keep-alive
keep_alive = !LowerCaseEqualsASCII(connection_val, "close");
for (const char* header : kConnectionHeaders) {
void* iterator = nullptr;
std::string token;
while (EnumerateHeader(&iterator, header, &token)) {
for (const KeepAliveToken& keep_alive_token : kKeepAliveTokens) {
if (LowerCaseEqualsASCII(token, keep_alive_token.token))
return keep_alive_token.keep_alive;
}
}
}
return keep_alive;
return http_version_ != HttpVersion(1, 0);
}
bool HttpResponseHeaders::HasStrongValidators() const {
......
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include <algorithm>
#include <iostream>
#include <limits>
#include "base/basictypes.h"
......@@ -1640,6 +1641,14 @@ struct KeepAliveTestData {
bool expected_keep_alive;
};
// Enable GTest to print KeepAliveTestData in an intelligible way if the test
// fails.
void PrintTo(const KeepAliveTestData& keep_alive_test_data,
std::ostream* os) {
*os << "{\"" << keep_alive_test_data.headers << "\", " << std::boolalpha
<< keep_alive_test_data.expected_keep_alive << "}";
}
class IsKeepAliveTest
: public HttpResponseHeadersTest,
public ::testing::WithParamInterface<KeepAliveTestData> {
......@@ -1714,6 +1723,82 @@ const KeepAliveTestData keepalive_tests[] = {
"proxy-connection: keep-alive\n",
true
},
{ "HTTP/1.1 200 OK\n"
"Connection: Upgrade, close\n",
false
},
{ "HTTP/1.1 200 OK\n"
"Connection: Upgrade, keep-alive\n",
true
},
{ "HTTP/1.1 200 OK\n"
"Connection: Upgrade\n"
"Connection: close\n",
false
},
{ "HTTP/1.1 200 OK\n"
"Connection: Upgrade\n"
"Connection: keep-alive\n",
true
},
{ "HTTP/1.1 200 OK\n"
"Connection: close, Upgrade\n",
false
},
{ "HTTP/1.1 200 OK\n"
"Connection: keep-alive, Upgrade\n",
true
},
{ "HTTP/1.1 200 OK\n"
"Connection: Upgrade\n"
"Proxy-Connection: close\n",
false
},
{ "HTTP/1.1 200 OK\n"
"Connection: Upgrade\n"
"Proxy-Connection: keep-alive\n",
true
},
// In situations where the response headers conflict with themselves, use the
// first one for backwards-compatibility.
{ "HTTP/1.1 200 OK\n"
"Connection: close\n"
"Connection: keep-alive\n",
false
},
{ "HTTP/1.1 200 OK\n"
"Connection: keep-alive\n"
"Connection: close\n",
true
},
{ "HTTP/1.0 200 OK\n"
"Connection: close\n"
"Connection: keep-alive\n",
false
},
{ "HTTP/1.0 200 OK\n"
"Connection: keep-alive\n"
"Connection: close\n",
true
},
// Ignore the Proxy-Connection header if at all possible.
{ "HTTP/1.0 200 OK\n"
"Proxy-Connection: keep-alive\n"
"Connection: close\n",
false
},
{ "HTTP/1.1 200 OK\n"
"Proxy-Connection: close\n"
"Connection: keep-alive\n",
true
},
// Older versions of Chrome would have ignored Proxy-Connection in this case,
// but it doesn't seem safe.
{ "HTTP/1.1 200 OK\n"
"Proxy-Connection: close\n"
"Connection: Transfer-Encoding\n",
false
},
};
INSTANTIATE_TEST_CASE_P(HttpResponseHeaders,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment