Skip to content

fix(ws): Fully handle websocket frame tearing (closes #453) #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
willmmiles merged 12 commits into from
Jul 25, 2026
Merged

fix(ws): Fully handle websocket frame tearing (closes #453) #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f416d76
Unify AsyncWebSocketControl into AsyncWebSocketMessage
willmmiles Jul 18, 2026
09e9bcf
Make WS frame sends resumable and track actual bytes committed
willmmiles Jul 18, 2026
dac0a61
Lift WS send state onto AsyncWebSocketClient; enforce one outstanding…
willmmiles Jul 18, 2026
ef8fa6e
Dequeue WS messages on add() completion instead of waiting for ack
willmmiles Jul 20, 2026
c5dd1e5
feat(ws): Pass pong data to callback
willmmiles Jul 25, 2026
fc5296d
feat(example/WebSocket) Measure ping RTT
willmmiles Jul 25, 2026
66745b7
fix(ws): Add mask for 0-length frames
willmmiles Jul 24, 2026
61a22ec
fix(ws): Fix possible UB with empty message
willmmiles Jul 24, 2026
c39bec7
fix(ws): Window exhaustion
willmmiles Jul 24, 2026
30cd53b
fix(ws): Ensure non-mutation of shared data buffers
willmmiles Jul 25, 2026
04de99b
refactor(ws): static vs anonymous namespace
willmmiles Jul 25, 2026
a3b0e4c
feat(ws): Add masked data broadcast test
willmmiles Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 77 additions & 7 deletions examples/arduino/WebSocket/WebSocket.ino
View file
Open in desktop

@willmmiles willmmiles Jul 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit fc5296d: I found this useful during testing, but it had an interesting interaction with the fragmentation code. When the large packet is sent, it's fragmented. The fragmentation logic takes the available TCP window and subtracts 8 bytes, enough for the largest possible header (ie. including a mask). But we don't send a mask: this leaves 4 bytes in the TCP window, which is enough to fit the first 0-byte ping from pingAll().

Adding the timestamp no longer fits in that window, so it adds a minimum of 2 RTTs to collect the ping response. (We have to get at least one ack back before the remainder of the control packet can be sent.) This results in some fairly large round trip delays.

I found it useful but we do not need to accept this commit upstream.

@mathieucarbou mathieucarbou Jul 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s fine let’s leave it.
We cannot optimize more anywhere since the ping data part is user controled so if larger it could even be 3 rtts right ?

@willmmiles willmmiles Jul 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit a3b0e4c: This is also somewhat dubious -- while it's a useful test for a key property (the shared data is not modified by the library), the specific test approach is a standards violation. I've separated this in to an isolated commit so we can drop it if we don't want to keep it.

@mathieucarbou mathieucarbou Jul 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, let’s leave it. Anything that can help us test is good I think. Maybe one day we can think about separating examples from tests and make tests a little bit clever so that we could eventually partly automate them.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@

#include <ESPAsyncWebServer.h>

#include <algorithm>
#include <memory>
#include <vector>

static const char *htmlContent PROGMEM = R"(
<!DOCTYPE html>
<html>
Expand Down Expand Up @@ -63,12 +67,55 @@ static const size_t htmlContentLength = strlen_P(htmlContent);
static AsyncWebServer server(80);
static AsyncWebSocket ws("/ws");

//
// Masked-broadcast test: sends one AsyncWebSocketSharedBuffer to every connected client, each
// with its own random mask key. Validates that masking a shared buffer for one client can't
// affect what any other client (sharing that same buffer) sends or receives.
//
// Triggered by sending the text command "masktest" (optionally "masktest <len>") over an
// already-open WS connection (see the WS_EVT_DATA handler below). The payload is deterministic
// (a seq/len header + a repeating 0-9 filler)The decoded payload must be byte-identical for all
// clients despite each seeing different bytes on the wire.
//
// RFC 6455 5.1 forbids the server from masking frames it sends, and requires a compliant client
// to close the connection on receipt of one. This test deliberately violates that rule, so it
// requires a client that tolerates reading masked frames from the server (e.g. websocat4).
//
static void fillMaskTestPayload(std::vector<uint8_t> &out, size_t len) {
char header[48];
const int written = snprintf(header, sizeof(header), "MASKTEST len=%u\n", (unsigned)len);
// snprintf's return value can exceed sizeof(header) if truncated; clamp to what's actually in the buffer
const size_t headerLen = std::min((size_t)std::max(written, 0), sizeof(header));
out.resize(len);
const size_t n = std::min(headerLen, len);
memcpy(out.data(), header, n);
for (size_t i = n; i < len; i++) {
out[i] = '0' + ((i - n) % 10);
}
}

// Sends `len` bytes of deterministic content to every connected client as an individually
// masked TEXT frame, reusing one shared buffer across all of them. Returns the number of
// clients the frame was successfully enqueued for.
static size_t sendMaskedBroadcast(size_t len) {
std::shared_ptr<std::vector<uint8_t>> buffer = std::make_shared<std::vector<uint8_t>>();
fillMaskTestPayload(*buffer, len);
size_t sent = 0;
for (auto &client : ws.getClients()) {
if (client.status() == WS_CONNECTED && client.message(buffer, WS_TEXT, /*mask=*/true)) {
sent++;
}
}
Serial.printf("[masktest] len=%u sent to %u client(s), buffer use_count=%ld\n", (unsigned)len, (unsigned)sent, (long)buffer.use_count());
return sent;
}

void setup() {
Serial.begin(115200);

#if ASYNCWEBSERVER_WIFI_SUPPORTED
WiFi.mode(WIFI_AP);
WiFi.softAP("esp-captive");
WiFi.softAP("esp-captive", nullptr, 11);
#endif

// serves root html page
Expand All @@ -87,6 +134,11 @@ void setup() {
// Generates 2001 characters (\n at the end) to cause a fragmentation (over TCP MSS):
// > openssl rand -hex 1000 | websocat ws://192.168.4.1/ws
//
// Triggers the masked-broadcast test (see sendMaskedBroadcast() above).
// This requires a client that tolerates reading masked frames from the server (e.g. websocat4),
// since it's a deliberate RFC 6455 5.1 violation when used this way.
// > echo "masktest 4000" | websocat4 --ws-ignore-invalid-masks ws://192.168.4.1/ws
//
ws.onEvent([](AsyncWebSocket *server, AsyncWebSocketClient *client, AwsEventType type, void *arg, uint8_t *data, size_t len) {
(void)len;

Expand All @@ -103,8 +155,14 @@ void setup() {
Serial.println("ws error");

} else if (type == WS_EVT_PONG) {
Serial.println("ws pong");

if (len == sizeof(uint32_t)) {
uint32_t pongTime;
memcpy(&pongTime, data, sizeof(pongTime));
uint32_t pingDuration = micros() - pongTime;
Serial.printf("ws[%" PRIu32 "] pong [%" PRIu32 "] duration: %" PRIu32 " us\n", client->id(), pongTime, pingDuration);
} else {
Serial.printf("ws[%" PRIu32 "] pong [%u]\n", client->id(), len);
}
} else if (type == WS_EVT_DATA) {
AwsFrameInfo *info = (AwsFrameInfo *)arg;
Serial.printf(
Expand All @@ -117,8 +175,18 @@ void setup() {
if (info->message_opcode == WS_TEXT) {
Serial.printf("ws text: %s\n", (char *)data);
client->ping();
// Also send a message in the message queue when we get one
ws.textAll("Message received: " + String((char *)data));

if (strncmp((char *)data, "masktest", 8) == 0) {
size_t maskLen = 2000;
unsigned long parsed = strtoul((char *)data + 8, nullptr, 10);
if (parsed > 0) {
maskLen = std::min((size_t)parsed, (size_t)65000); // stay within a single (non-fragmented) WS frame
}
sendMaskedBroadcast(maskLen);
} else {
// Also send a message in the message queue when we get one
ws.textAll("Message received: " + String((char *)data));
}
}

} else {
Expand Down Expand Up @@ -205,8 +273,10 @@ void loop() {
ws.textAll(random);

// ping twice (2 control frames)
ws.pingAll();
ws.pingAll();
uint32_t pingTime = micros();
ws.pingAll((const uint8_t *)&pingTime, sizeof(pingTime));
pingTime = micros();
ws.pingAll((const uint8_t *)&pingTime, sizeof(pingTime));

#ifdef ESP32
Serial.printf("Uptime: %3lu s, Free heap: %" PRIu32 ", Min free heap: %" PRIu32 "\n", millis() / 1000, ESP.getFreeHeap(), ESP.getMinFreeHeap());
Expand Down
Loading
Loading