From 41d8bced183a1a786e80b1e9ecfee37b53bb7c45 Mon Sep 17 00:00:00 2001 From: agessaman Date: Tue, 11 Aug 2026 18:39:58 -0700 Subject: [PATCH] docs(mqtt): correct what the WS buffer padding actually fixes The comment claimed the (WS_BUFFER_SIZE + 1) padding makes an oversized upgrade response "fail cleanly (Upgrade header not found)". Reading release/v4.4 transport_ws.c against release/v5.3 shows it does not. v4.4's response loop is } while (NULL == strstr(ws->buffer, "\r\n\r\n") && header_len < WS_BUFFER_SIZE); so it also exits when the buffer fills without the terminator, and the code then looks for "Sec-WebSocket-Accept:" and returns 0 if it is present. That header appears early in a response, so an oversized header block yields a BOGUS SUCCESS rather than a clean failure: the unread remainder stays queued on the socket and is delivered as the first post-upgrade read, where the deframer parses HTTP bytes as a WebSocket frame header. Observed on hardware 2026-08-12 on a Heltec V4: a Cloudflare Page Shield CSP report-uri header pushed the 101 response past the buffer, and the tail of that header ("csp-reporting.cloudflare.com/cdn-cgi/script_monitor/report?") reached the MQTT layer as payload, surfacing as "Invalid MSG_TYPE response: 3" (0x35 = '5', high nibble 3). The padding's real and only value is preventing the one-byte overflow of the heap canary, which is still worth having. Narrow the comment to that claim and record where the parser fix has to come from: IDF 5.2+ requires the "\r\n\r\n" delimiter, memmoves the bytes following it, and fails cleanly when the buffer fills. It cannot be patched here, since transport_ws.c ships precompiled in libtcp_transport.a on Arduino 2.x. No functional change. (cherry picked from commit 9894e65e8ad961704d430a12a065baffda353f50) --- src/helpers/ESP32WsTransportFix.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/helpers/ESP32WsTransportFix.cpp b/src/helpers/ESP32WsTransportFix.cpp index 2e14bb33e..a69e0b088 100644 --- a/src/helpers/ESP32WsTransportFix.cpp +++ b/src/helpers/ESP32WsTransportFix.cpp @@ -33,13 +33,24 @@ // Fix: [esp32_base] adds `-Wl,--wrap=esp_transport_ws_init`, so every // creation of a WS transport (esp-mqtt does one per wss slot) is routed // through __wrap_esp_transport_ws_init below, which replaces the freshly -// allocated 1024-byte buffer with a (WS_BUFFER_SIZE + 1)-byte one. The -// out-of-bounds index WS_BUFFER_SIZE then lands on our extra byte and the -// handshake fails cleanly ("Upgrade" header not found) instead of corrupting -// the heap. Upstream fixed this in ESP-IDF 5.x, so this file compiles to a +// allocated 1024-byte buffer with a (WS_BUFFER_SIZE + 1)-byte one, so the +// out-of-bounds index WS_BUFFER_SIZE lands on our extra byte instead of the +// heap canary. Upstream fixed this in ESP-IDF 5.x, so this file compiles to a // pass-through there and can be deleted (together with the --wrap flag) when // the fork moves to Arduino core 3.x. // +// Scope: this stops the heap corruption and NOTHING else. It does not make an +// oversized response fail the handshake — v4.4's read loop also exits on +// `header_len < WS_BUFFER_SIZE` going false, and then still accepts the +// connection if "Sec-WebSocket-Accept:" was inside those first bytes (it comes +// early, so it usually is). ws_connect() therefore returns success on a partial +// header block and the unread remainder arrives as the first "payload" read, +// where the deframer parses HTTP bytes as a frame header. Observed on hardware +// 2026-08-12: a large Cloudflare CSP header produced exactly that, surfacing as +// `Invalid MSG_TYPE response: 3`. Only IDF 5.2+ fixes it (it requires the +// "\r\n\r\n" delimiter, preserves the bytes after it, and fails cleanly when the +// buffer fills); it cannot be patched here because transport_ws.c is precompiled. +// // transport_ws_t below is copied verbatim from ESP-IDF release/v4.4 // transport_ws.c (the struct is file-private, so it is not in any shipped // header). Source fidelity was verified against the shipped binary: addr2line