mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-09-30 20:52:31 +07:00
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:
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user