Match the Connection "Upgrade" token exactly in WebSocket handshakes (#2542)

* Match the Connection "Upgrade" token exactly in WebSocket handshakes

The server and the client both looked for "upgrade" as a substring of the
Connection field value, so "notupgrade", "upgrade-not" and "xupgrade" all
passed as the standalone token the handshake requires. RFC 6455 4.2.1 asks
for an ASCII case-insensitive token match, and a value split across several
Connection lines was missed entirely because only the first line was read.

Parse the field as the comma-separated token list it is, across every line,
and reuse the same helper for the server request check and the client
response check.

Reported by gb1dev.

* Tidy up the Connection token helper

Move has_header_token() out of the WebSocket-only detail block and next to
the other header field helpers, forward-declaring it beside split(). Use the
existing split_find(), which drops the manual found flag and stops at the
first matching token.

Drive the client-side test from an ordinary Server route answering 101 with
a bad Connection value, rather than the hand-rolled listening socket copied
from the test above it.

* Keep the Connection token helper out of the split build's ABI

The split build strips inline from everything below the border line, so a
helper defined there becomes an exported symbol of the shared library and
abidiff reports it as an added function. The tests also could not see
is_websocket_upgrade() or websocket_accept_key(), since neither is declared
in the part of the header that survives the split.

Define has_header_token() as a static inline above the border, next to the
split() declarations its two call sites already sit below, and declare the
two WebSocket helpers the way ws::impl::read_websocket_frame() already is.
The shared library's exported symbols are now identical to master's.
This commit is contained in:
yhirose
2026-08-18 06:48:12 -04:00
committed by GitHub
parent 2004668509
commit ffe2a1c1e9
2 changed files with 123 additions and 10 deletions
+34 -10
View File
@@ -3500,6 +3500,36 @@ void split(const char *b, const char *e, char d,
void split(const char *b, const char *e, char d, size_t m,
std::function<void(const char *, const char *)> fn);
bool split_find(const char *b, const char *e, char d,
std::function<bool(const char *, const char *)> fn);
// Defined here rather than beside the other header-field helpers because both
// call sites sit in the implementation section below. Kept static so that an
// internal helper does not become an exported symbol of the shared library.
static inline bool has_header_token(const Headers &headers,
const std::string &key,
const std::string &token) {
// RFC 9110 7.6.1: a comma-separated token list field such as Connection may
// carry several tokens, and RFC 9110 5.3 lets that list be split across
// several lines. Match complete tokens rather than searching the raw value,
// so that a value such as "notupgrade" is not read as the token "upgrade".
auto rng = headers.equal_range(key);
for (auto it = rng.first; it != rng.second; ++it) {
const auto &value = it->second;
if (split_find(value.data(), value.data() + value.size(), ',',
[&](const char *b, const char *e) {
return case_ignore::equal(std::string(b, e), token);
})) {
return true;
}
}
return false;
}
std::string websocket_accept_key(const std::string &client_key);
bool is_websocket_upgrade(const Request &req);
bool process_client_socket(
socket_t sock, time_t read_timeout_sec, time_t read_timeout_usec,
time_t write_timeout_sec, time_t write_timeout_usec,
@@ -5289,11 +5319,8 @@ inline bool is_websocket_upgrade(const Request &req) {
auto upgrade_val = case_ignore::to_lower(upgrade_it->second);
if (upgrade_val != "websocket") { return false; }
// Check Connection header contains "Upgrade"
auto connection_it = req.headers.find("Connection");
if (connection_it == req.headers.end()) { return false; }
auto connection_val = case_ignore::to_lower(connection_it->second);
if (connection_val.find("upgrade") == std::string::npos) { return false; }
// Check Connection: Upgrade
if (!has_header_token(req.headers, "Connection", "upgrade")) { return false; }
// Check Sec-WebSocket-Key is a valid base64-encoded 16-byte value (24 chars)
// RFC 6455 Section 4.2.1
@@ -7885,11 +7912,8 @@ inline bool read_websocket_upgrade_response(Stream &strm,
return false;
}
// Verify Connection header contains "Upgrade" (case-insensitive)
auto connection_it = headers.find("Connection");
if (connection_it == headers.end() ||
case_ignore::to_lower(connection_it->second).find("upgrade") ==
std::string::npos) {
// Verify Connection: Upgrade
if (!has_header_token(headers, "Connection", "upgrade")) {
upgrade.error = Error::WebSocketHandshake;
return false;
}
+89
View File
@@ -20983,6 +20983,95 @@ TEST(WebSocketTest, InvalidHeaderInHandshakeWritesNothing) {
EXPECT_EQ(0, received);
}
TEST(WebSocketTest, ConnectionHeaderNeedsCompleteUpgradeToken) {
// RFC 6455 4.2.1: Connection is a token list, so a value that merely
// contains "upgrade" as a substring is a different token and must not
// start a WebSocket handshake.
auto make_request = [](const std::vector<std::string> &connection_values) {
Request req;
req.method = "GET";
req.headers.emplace("Upgrade", "websocket");
for (const auto &value : connection_values) {
req.headers.emplace("Connection", value);
}
req.headers.emplace("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==");
req.headers.emplace("Sec-WebSocket-Version", "13");
return req;
};
EXPECT_TRUE(detail::is_websocket_upgrade(make_request({"Upgrade"})));
EXPECT_TRUE(detail::is_websocket_upgrade(make_request({"upgrade"})));
EXPECT_TRUE(
detail::is_websocket_upgrade(make_request({"keep-alive, Upgrade"})));
EXPECT_TRUE(
detail::is_websocket_upgrade(make_request({"Upgrade , keep-alive"})));
EXPECT_TRUE(
detail::is_websocket_upgrade(make_request({"keep-alive", "Upgrade"})));
EXPECT_FALSE(detail::is_websocket_upgrade(make_request({"notupgrade"})));
EXPECT_FALSE(detail::is_websocket_upgrade(make_request({"upgrade-not"})));
EXPECT_FALSE(detail::is_websocket_upgrade(make_request({"xupgrade"})));
EXPECT_FALSE(
detail::is_websocket_upgrade(make_request({"keep-alive, notupgrade"})));
EXPECT_FALSE(detail::is_websocket_upgrade(make_request({"close"})));
EXPECT_FALSE(detail::is_websocket_upgrade(make_request({})));
}
TEST(WebSocketTest, ServerRejectsHandshakeWithoutUpgradeToken) {
Server svr;
svr.WebSocket("/ws", [](const Request &, ws::WebSocket &) {});
auto port = svr.bind_to_any_port("localhost");
std::thread t([&]() { svr.listen_after_bind(); });
auto se = detail::scope_exit([&] {
svr.stop();
t.join();
});
svr.wait_until_ready();
Headers headers = {{"Upgrade", "websocket"},
{"Connection", "notupgrade"},
{"Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ=="},
{"Sec-WebSocket-Version", "13"}};
Client cli("localhost", port);
auto res = cli.Get("/ws", headers);
// The route exists only as a WebSocket route, so a request that fails the
// handshake check falls through to ordinary routing.
ASSERT_TRUE(res);
EXPECT_EQ(StatusCode::NotFound_404, res->status);
}
TEST(WebSocketTest, ClientRejectsResponseWithoutUpgradeToken) {
// The peer answers 101 with a correct Sec-WebSocket-Accept, so the
// Connection value is the only thing left for the client to reject.
Server svr;
svr.Get("/ws", [](const Request &req, Response &res) {
res.status = StatusCode::SwitchingProtocol_101;
res.set_header("Upgrade", "websocket");
res.set_header("Connection", "notupgrade");
res.set_header("Sec-WebSocket-Accept",
detail::websocket_accept_key(
req.get_header_value("Sec-WebSocket-Key")));
});
auto port = svr.bind_to_any_port("localhost");
std::thread t([&]() { svr.listen_after_bind(); });
auto se = detail::scope_exit([&] {
svr.stop();
t.join();
});
svr.wait_until_ready();
ws::WebSocketClient client("ws://localhost:" + std::to_string(port) + "/ws");
auto res = client.connect();
EXPECT_FALSE(res);
EXPECT_EQ(Error::WebSocketHandshake, res.error());
EXPECT_FALSE(client.is_open());
}
TEST(WebSocketTest, HostHeaderOverUnixSocket) {
// The socket path doubles as the URL host, so it must not contain '/'.
const char *shard = getenv("GTEST_SHARD_INDEX");