From bbae90fd45cd15ead43c5b91656bc838405b309f Mon Sep 17 00:00:00 2001 From: "vkuzkokov@chromium.org" Date: Sat, 10 May 2014 19:20:34 +0000 Subject: [PATCH] HttpServer: Handling of multiple header fields with the same name and multiple values of "Connection". BUG=370437 Review URL: https://codereview.chromium.org/274813002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269619 0039d316-1c4b-4281-b951-d872f2087c98 --- net/server/http_server.cc | 21 +++++--- net/server/http_server_request_info.cc | 17 ++++++ net/server/http_server_request_info.h | 6 +++ net/server/http_server_unittest.cc | 94 ++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) diff --git a/net/server/http_server.cc b/net/server/http_server.cc index df77c3672daa..f746f066e421 100644 --- a/net/server/http_server.cc +++ b/net/server/http_server.cc @@ -142,8 +142,7 @@ void HttpServer::DidRead(StreamListenSocket* socket, // Sets peer address if exists. socket->GetPeerAddress(&request.peer); - std::string connection_header = request.GetHeaderValue("connection"); - if (connection_header == "Upgrade") { + if (request.HasHeaderValue("connection", "upgrade")) { connection->web_socket_.reset(WebSocket::CreateWebSocket(connection, request, &pos)); @@ -205,7 +204,7 @@ HttpServer::~HttpServer() { // Input character types. enum header_parse_inputs { - INPUT_SPACE, + INPUT_LWS, INPUT_CR, INPUT_LF, INPUT_COLON, @@ -244,7 +243,8 @@ int parser_state[MAX_STATES][MAX_INPUTS] = { int charToInput(char ch) { switch(ch) { case ' ': - return INPUT_SPACE; + case '\t': + return INPUT_LWS; case '\r': return INPUT_CR; case '\n': @@ -270,6 +270,7 @@ bool HttpServer::ParseHeaders(HttpConnection* connection, int next_state = parser_state[state][input]; bool transition = (next_state != state); + HttpServerRequestInfo::HeadersMap::iterator it; if (transition) { // Do any actions based on state transitions. switch (state) { @@ -292,9 +293,15 @@ bool HttpServer::ParseHeaders(HttpConnection* connection, break; case ST_VALUE: base::TrimWhitespaceASCII(buffer, base::TRIM_LEADING, &header_value); - // TODO(mbelshe): Deal better with duplicate headers - DCHECK(info->headers.find(header_name) == info->headers.end()); - info->headers[header_name] = header_value; + it = info->headers.find(header_name); + // See last paragraph ("Multiple message-header fields...") + // of www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 + if (it == info->headers.end()) { + info->headers[header_name] = header_value; + } else { + it->second.append(","); + it->second.append(header_value); + } buffer.clear(); break; case ST_SEPARATOR: diff --git a/net/server/http_server_request_info.cc b/net/server/http_server_request_info.cc index 67965f29aa34..8b65bee50c1a 100644 --- a/net/server/http_server_request_info.cc +++ b/net/server/http_server_request_info.cc @@ -22,4 +22,21 @@ std::string HttpServerRequestInfo::GetHeaderValue( return std::string(); } +bool HttpServerRequestInfo::HasHeaderValue( + const std::string& header_name, + const std::string& header_value) const { + DCHECK_EQ(StringToLowerASCII(header_value), header_value); + std::string complete_value = GetHeaderValue(header_name); + StringToLowerASCII(&complete_value); + std::vector value_items; + Tokenize(complete_value, ",", &value_items); + for (std::vector::iterator it = value_items.begin(); + it != value_items.end(); ++it) { + base::TrimString(*it, " \t", &*it); + if (*it == header_value) + return true; + } + return false; +} + } // namespace net diff --git a/net/server/http_server_request_info.h b/net/server/http_server_request_info.h index 183da1c07f3d..1b02655b7421 100644 --- a/net/server/http_server_request_info.h +++ b/net/server/http_server_request_info.h @@ -25,6 +25,12 @@ class HttpServerRequestInfo { // lower case. std::string GetHeaderValue(const std::string& header_name) const; + // Checks for item in comma-separated header value for given header name. + // Both |header_name| and |header_value| should be lower case. + bool HasHeaderValue( + const std::string& header_name, + const std::string& header_value) const; + // Request peer address. IPEndPoint peer; diff --git a/net/server/http_server_unittest.cc b/net/server/http_server_unittest.cc index de8690f138e0..207c454acfbc 100644 --- a/net/server/http_server_unittest.cc +++ b/net/server/http_server_unittest.cc @@ -209,6 +209,22 @@ class HttpServerTest : public testing::Test, size_t quit_after_request_count_; }; +class WebSocketTest : public HttpServerTest { + virtual void OnHttpRequest(int connection_id, + const HttpServerRequestInfo& info) OVERRIDE { + NOTREACHED(); + } + + virtual void OnWebSocketRequest(int connection_id, + const HttpServerRequestInfo& info) OVERRIDE { + HttpServerTest::OnHttpRequest(connection_id, info); + } + + virtual void OnWebSocketMessage(int connection_id, + const std::string& data) OVERRIDE { + } +}; + TEST_F(HttpServerTest, Request) { TestHttpClient client; ASSERT_EQ(OK, client.ConnectAndWait(server_address_)); @@ -253,6 +269,71 @@ TEST_F(HttpServerTest, RequestWithHeaders) { } } +TEST_F(HttpServerTest, RequestWithDuplicateHeaders) { + TestHttpClient client; + ASSERT_EQ(OK, client.ConnectAndWait(server_address_)); + const char* kHeaders[][3] = { + {"FirstHeader", ": ", "1"}, + {"DuplicateHeader", ": ", "2"}, + {"MiddleHeader", ": ", "3"}, + {"DuplicateHeader", ": ", "4"}, + {"LastHeader", ": ", "5"}, + }; + std::string headers; + for (size_t i = 0; i < arraysize(kHeaders); ++i) { + headers += + std::string(kHeaders[i][0]) + kHeaders[i][1] + kHeaders[i][2] + "\r\n"; + } + + client.Send("GET /test HTTP/1.1\r\n" + headers + "\r\n"); + ASSERT_TRUE(RunUntilRequestsReceived(1)); + ASSERT_EQ("", GetRequest(0).data); + + for (size_t i = 0; i < arraysize(kHeaders); ++i) { + std::string field = StringToLowerASCII(std::string(kHeaders[i][0])); + std::string value = (field == "duplicateheader") ? "2,4" : kHeaders[i][2]; + ASSERT_EQ(1u, GetRequest(0).headers.count(field)) << field; + ASSERT_EQ(value, GetRequest(0).headers[field]) << kHeaders[i][0]; + } +} + +TEST_F(HttpServerTest, HasHeaderValueTest) { + TestHttpClient client; + ASSERT_EQ(OK, client.ConnectAndWait(server_address_)); + const char* kHeaders[] = { + "Header: Abcd", + "HeaderWithNoWhitespace:E", + "HeaderWithWhitespace : \t f \t ", + "DuplicateHeader: g", + "HeaderWithComma: h, i ,j", + "DuplicateHeader: k", + "EmptyHeader:", + "EmptyHeaderWithWhitespace: \t ", + "HeaderWithNonASCII: \xf7", + }; + std::string headers; + for (size_t i = 0; i < arraysize(kHeaders); ++i) { + headers += std::string(kHeaders[i]) + "\r\n"; + } + + client.Send("GET /test HTTP/1.1\r\n" + headers + "\r\n"); + ASSERT_TRUE(RunUntilRequestsReceived(1)); + ASSERT_EQ("", GetRequest(0).data); + + ASSERT_TRUE(GetRequest(0).HasHeaderValue("header", "abcd")); + ASSERT_FALSE(GetRequest(0).HasHeaderValue("header", "bc")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithnowhitespace", "e")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithwhitespace", "f")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("duplicateheader", "g")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithcomma", "h")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithcomma", "i")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithcomma", "j")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("duplicateheader", "k")); + ASSERT_FALSE(GetRequest(0).HasHeaderValue("emptyheader", "x")); + ASSERT_FALSE(GetRequest(0).HasHeaderValue("emptyheaderwithwhitespace", "x")); + ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithnonascii", "\xf7")); +} + TEST_F(HttpServerTest, RequestWithBody) { TestHttpClient client; ASSERT_EQ(OK, client.ConnectAndWait(server_address_)); @@ -270,6 +351,19 @@ TEST_F(HttpServerTest, RequestWithBody) { ASSERT_EQ('c', *body.rbegin()); } +TEST_F(WebSocketTest, RequestWebSocket) { + TestHttpClient client; + ASSERT_EQ(OK, client.ConnectAndWait(server_address_)); + client.Send( + "GET /test HTTP/1.1\r\n" + "Upgrade: WebSocket\r\n" + "Connection: SomethingElse, Upgrade\r\n" + "Sec-WebSocket-Version: 8\r\n" + "Sec-WebSocket-Key: key\r\n" + "\r\n"); + ASSERT_TRUE(RunUntilRequestsReceived(1)); +} + TEST_F(HttpServerTest, RequestWithTooLargeBody) { class TestURLFetcherDelegate : public URLFetcherDelegate { public: -- 2.11.4.GIT