diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 806ecdd17801..1c5088a28302 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -102,6 +102,31 @@ class HeaderString { */ bool find(const char* str) const { return strstr(c_str(), str); } + /** + * HeaderString is in token list form, each token separated by commas or whitespace, + * see https://www.w3.org/Protocols/rfc2616/rfc2616-sec2.html#sec2.1 for more information, + * header field value's case sensitivity depends on each header. + * @return whether contains token in case insensitive manner. + */ + bool caseInsensitiveContains(const char* token) const { + // Avoid dead loop if token argument is empty. + const int n = strlen(token); + if (n == 0) { + return false; + } + + // Find token substring, skip if it's partial of other token. + const char* tokens = c_str(); + for (const char* p = tokens; (p = strcasestr(p, token)); p += n) { + if ((p == tokens || *(p - 1) == ' ' || *(p - 1) == ',') && + (*(p + n) == '\0' || *(p + n) == ' ' || *(p + n) == ',')) { + return true; + } + } + + return false; + } + /** * Set the value of the string by copying data into it. This overwrites any existing string. */ diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 89dadb738231..2c6210a4b0b0 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -21,19 +21,30 @@ void ConnectionManagerUtility::mutateRequestHeaders( ConnectionManagerConfig& config, const Router::Config& route_config, Runtime::RandomGenerator& random, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info) { - // Clean proxy headers. - request_headers.removeEnvoyInternalRequest(); - request_headers.removeKeepAlive(); - request_headers.removeProxyConnection(); - request_headers.removeTransferEncoding(); - // If this is a WebSocket Upgrade request, do not remove the Connection and Upgrade headers, // as we forward them verbatim to the upstream hosts. - if (protocol != Protocol::Http11 || !Utility::isWebSocketUpgradeRequest(request_headers)) { + if (protocol == Protocol::Http11 && Utility::isWebSocketUpgradeRequest(request_headers)) { + // The current WebSocket implementation re-uses the HTTP1 codec to send upgrade headers to + // the upstream host. This adds the "transfer-encoding: chunked" request header if the stream + // has not ended and content-length does not exist. In HTTP1.1, if transfer-encoding and + // content-length both do not exist this means there is no request body. After transfer-encoding + // is stripped here, the upstream request becomes invalid. We can fix it by explicitly adding a + // "content-length: 0" request header here. + const bool no_body = (!request_headers.TransferEncoding() && !request_headers.ContentLength()); + if (no_body) { + request_headers.insertContentLength().value(uint64_t(0)); + } + } else { request_headers.removeConnection(); request_headers.removeUpgrade(); } + // Clean proxy headers. + request_headers.removeEnvoyInternalRequest(); + request_headers.removeKeepAlive(); + request_headers.removeProxyConnection(); + request_headers.removeTransferEncoding(); + // If we are "using remote address" this means that we create/append to XFF with our immediate // peer. Cases where we don't "use remote address" include trusted double proxy where we expect // our peer to have already properly set XFF, etc. diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index f6112d1ae5ed..9976aa0895da 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -184,10 +184,11 @@ bool Utility::isInternalRequest(const HeaderMap& headers) { } bool Utility::isWebSocketUpgradeRequest(const HeaderMap& headers) { + // In firefox the "Connection" request header value is "keep-alive, Upgrade", + // we should check if it contains the "Upgrade" token. return (headers.Connection() && headers.Upgrade() && - (0 == StringUtil::caseInsensitiveCompare( - headers.Connection()->value().c_str(), - Http::Headers::get().ConnectionValues.Upgrade.c_str())) && + headers.Connection()->value().caseInsensitiveContains( + Http::Headers::get().ConnectionValues.Upgrade.c_str()) && (0 == StringUtil::caseInsensitiveCompare( headers.Upgrade()->value().c_str(), Http::Headers::get().UpgradeValues.WebSocket.c_str()))); diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 34c445f6d1be..751fd9a38f3e 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -258,6 +258,31 @@ TEST(HeaderStringTest, All) { EXPECT_EQ(11U, string.size()); EXPECT_EQ(HeaderString::Type::Reference, string.type()); } + + // caseInsensitiveContains + { + const std::string static_string("keep-alive, Upgrade, close"); + HeaderString string(static_string); + EXPECT_TRUE(string.caseInsensitiveContains("keep-alive")); + EXPECT_TRUE(string.caseInsensitiveContains("Keep-alive")); + EXPECT_TRUE(string.caseInsensitiveContains("Upgrade")); + EXPECT_TRUE(string.caseInsensitiveContains("upgrade")); + EXPECT_TRUE(string.caseInsensitiveContains("close")); + EXPECT_TRUE(string.caseInsensitiveContains("Close")); + EXPECT_FALSE(string.caseInsensitiveContains("")); + EXPECT_FALSE(string.caseInsensitiveContains("keep")); + EXPECT_FALSE(string.caseInsensitiveContains("alive")); + EXPECT_FALSE(string.caseInsensitiveContains("grade")); + + const std::string small("close"); + string.setCopy(small.c_str(), small.size()); + EXPECT_FALSE(string.caseInsensitiveContains("keep-alive")); + + const std::string empty(""); + string.setCopy(empty.c_str(), empty.size()); + EXPECT_FALSE(string.caseInsensitiveContains("keep-alive")); + EXPECT_FALSE(string.caseInsensitiveContains("")); + } } TEST(HeaderMapImplTest, InlineInsert) { diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index b523d84de2be..42261b8fbfe2 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -364,12 +364,13 @@ FakeRawConnectionPtr FakeUpstream::waitForRawConnection() { return connection; } -void FakeRawConnection::waitForData(uint64_t num_bytes) { +std::string FakeRawConnection::waitForData(uint64_t num_bytes) { std::unique_lock lock(lock_); while (data_.size() != num_bytes) { ENVOY_LOG(debug, "waiting for {} bytes of data", num_bytes); connection_event_.wait(lock); } + return data_; } void FakeRawConnection::write(const std::string& data) { diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index ee85810c0b27..999c26043ac5 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -252,7 +252,7 @@ class FakeRawConnection : Logger::Loggable, public FakeConn connection_.addReadFilter(Network::ReadFilterSharedPtr{new ReadFilter(*this)}); } - void waitForData(uint64_t num_bytes); + std::string waitForData(uint64_t num_bytes); void write(const std::string& data); private: diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 9d19724044d3..2f4a547d9c25 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -193,31 +193,36 @@ TEST_P(IntegrationTest, WebSocketConnectionDownstreamDisconnect) { IntegrationTcpClientPtr tcp_client; FakeRawConnectionPtr fake_upstream_connection; const std::string upgrade_req_str = "GET /websocket/test HTTP/1.1\r\nHost: host\r\nConnection: " - "Upgrade\r\nUpgrade: websocket\r\n\r\n"; + "keep-alive, Upgrade\r\nUpgrade: websocket\r\n\r\n"; const std::string upgrade_resp_str = "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n"; tcp_client = makeTcpConnection(lookupPort("http")); // Send websocket upgrade request // The request path gets rewritten from /websocket/test to /websocket. - // The size of headers received by the destination is 225 bytes. + // The size of headers received by the destination is 228 bytes. tcp_client->write(upgrade_req_str); fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); - fake_upstream_connection->waitForData(225); + const std::string data = fake_upstream_connection->waitForData(228); + // In HTTP1, the transfer-length is defined by use of the "chunked" transfer-coding, even if + // content-length header is present. No body websocket upgrade request send to upstream has + // content-length header and has no transfer-encoding header. + EXPECT_NE(data.find("content-length:"), std::string::npos); + EXPECT_EQ(data.find("transfer-encoding:"), std::string::npos); // Accept websocket upgrade request fake_upstream_connection->write(upgrade_resp_str); tcp_client->waitForData(upgrade_resp_str); // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); - // datalen = 225 + strlen(hello) - fake_upstream_connection->waitForData(230); + // datalen = 228 + strlen(hello) + fake_upstream_connection->waitForData(233); fake_upstream_connection->write("world"); tcp_client->waitForData(upgrade_resp_str + "world"); tcp_client->write("bye!"); // downstream disconnect tcp_client->close(); - // datalen = 225 + strlen(hello) + strlen(bye!) - fake_upstream_connection->waitForData(234); + // datalen = 228 + strlen(hello) + strlen(bye!) + fake_upstream_connection->waitForData(237); fake_upstream_connection->waitForDisconnect(); } @@ -230,7 +235,7 @@ TEST_P(IntegrationTest, WebSocketConnectionUpstreamDisconnect) { IntegrationTcpClientPtr tcp_client; FakeRawConnectionPtr fake_upstream_connection; const std::string upgrade_req_str = "GET /websocket/test HTTP/1.1\r\nHost: host\r\nConnection: " - "Upgrade\r\nUpgrade: websocket\r\n\r\n"; + "keep-alive, Upgrade\r\nUpgrade: websocket\r\n\r\n"; const std::string upgrade_resp_str = "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n"; tcp_client = makeTcpConnection(lookupPort("http")); @@ -238,15 +243,20 @@ TEST_P(IntegrationTest, WebSocketConnectionUpstreamDisconnect) { tcp_client->write(upgrade_req_str); fake_upstream_connection = fake_upstreams_[0]->waitForRawConnection(); // The request path gets rewritten from /websocket/test to /websocket. - // The size of headers received by the destination is 225 bytes. - fake_upstream_connection->waitForData(225); + // The size of headers received by the destination is 228 bytes. + const std::string data = fake_upstream_connection->waitForData(228); + // In HTTP1, the transfer-length is defined by use of the "chunked" transfer-coding, even if + // content-length header is present. No body websocket upgrade request send to upstream has + // content-length header and has no transfer-encoding header. + EXPECT_NE(data.find("content-length:"), std::string::npos); + EXPECT_EQ(data.find("transfer-encoding:"), std::string::npos); // Accept websocket upgrade request fake_upstream_connection->write(upgrade_resp_str); tcp_client->waitForData(upgrade_resp_str); // Standard TCP proxy semantics post upgrade tcp_client->write("hello"); - // datalen = 225 + strlen(hello) - fake_upstream_connection->waitForData(230); + // datalen = 228 + strlen(hello) + fake_upstream_connection->waitForData(233); fake_upstream_connection->write("world"); // upstream disconnect fake_upstream_connection->close();