diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md new file mode 100644 index 00000000..8bb915e0 --- /dev/null +++ b/docs/KNOWN_ISSUES.md @@ -0,0 +1,59 @@ +# Known Issues + +## Concurrent mutation during chunked `/controller` response + +**Status:** open +**Introduced:** 2026-04-19, alongside the chunked-response conversion of `/controller` (see [`ControllerChunker` in src/Web.cpp](../src/Web.cpp) and the original crash report about `cbuf::resize` aborting `async_tcp`). + +### Summary + +The chunked streaming of `/controller` lazily reads `somfy.rooms`, `somfy.shades`, `somfy.groups`, and `somfy.repeaters` across many `async_tcp` callback invocations as the client's TCP send window opens. If another task mutates those arrays mid-stream, the rendered JSON can be internally inconsistent. + +### Why this is strictly worse than the pre-chunked code + +The old `handleController` ran the serializers synchronously inside a single request-handler invocation, so the full JSON was built in memory *before* any bytes were sent. Window of exposure: a few milliseconds. + +The new `ControllerChunker` reads state as the response drains. On a slow link or under backpressure, that window is hundreds of ms to seconds. + +### Concrete failure modes + +1. A shade is deleted mid-stream (e.g. via `/deleteShade`) — it may appear in the `shades` array but be missing from a later group's `linkedShades`, or vice versa. +2. `lastRollingCode` / position fields change between the `shades` pass and a later group's `linkedShades` pass — the client sees two values for the same shade in one document. +3. A group's `linkedShades` list is mutated while the chunker iterates inside `S_GROUPS` — an entry is skipped or emitted twice. + +### Fix options (pick one later) + +- **(a) Document and accept.** In practice users rarely mutate shades while the config UI is loading. Zero code change. +- **(b) FreeRTOS mutex around `somfy` reads/writes.** Acquire for the full duration of the chunked response and in every mutating path (RF RX, MQTT, web handlers). Cleanest but wide-reaching. +- **(c) Up-front snapshot.** At handler start, copy the subset of `somfy` state the response will serialize into the `ControllerChunker`. Defeats part of the memory benefit — a full snapshot of shades + groups is close in size to the old growing cbuf. Could be reduced by snapshotting only minimal fields (IDs, names, rolling codes) and reading the rest live. + +### Related + +- Same exposure exists in any other endpoint converted to chunked responses next (`/discovery`, `/shades`). Resolve this issue before expanding the pattern. + +## Silent truncation of large websocket events + +**Status:** open +**Location:** [`JsonSockEvent` in src/WResp.cpp](../src/WResp.cpp), buffer defined at [src/Sockets.cpp:45-46](../src/Sockets.cpp#L45-L46) as `g_response[MAX_SOCK_RESPONSE]` = 2048 bytes. + +### Summary + +Socket events are built into a fixed 2 KB static buffer. On overflow, [`JsonSockEvent::_safecat`](../src/WResp.cpp) logs an error and returns without appending — the event is sent truncated, producing malformed socket.io text that the client drops silently. + +Unlike the `/controller` HTTP crash, this path does **not** abort — there is no growing cbuf and no `new[]` on the send path. Per-client frame allocations inside `AsyncWebSocket` are bounded by the 2 KB buffer size and have their own overflow guard (queue drop / client disconnect). + +### Concrete failure modes + +1. A single event serializing a fully-populated shade (~1.3–1.5 KB for a shade with all `SOMFY_MAX_LINKED_REMOTES` = 7 populated) gets close to the 2 KB limit. Any additional fields or long names push it over and the JSON is silently cut mid-value. +2. Any event that loops over a collection (e.g. frequency-scan results, batch emits in `Somfy.cpp` around lines 1870–1975) can exceed 2 KB depending on size, with no indication to the client beyond the ESP-side `ESP_LOGE` line. + +### Fix options (pick one later) + +- **(a) Fail loud.** Keep truncation but emit a sentinel/error frame so the client knows the event was lost, instead of sending a malformed one. +- **(b) Split large events across frames.** Use the socket.io ack/chunk pattern to send an event in multiple frames when it wouldn't fit. Requires matching client-side reassembly. +- **(c) Raise `MAX_SOCK_RESPONSE`.** Cheapest, but just pushes the limit — doesn't eliminate the failure mode. + +### Related + +- Not the same code path as the `/controller` crash. Solve independently. +- Worth grepping for `JsonSockEvent` usages that iterate collections (see references in `Somfy.cpp`, `ESPNetwork.cpp`, `GitOTA.cpp`) to identify the most at-risk events. diff --git a/src/WResp.cpp b/src/WResp.cpp index c00c1c50..c40a028e 100644 --- a/src/WResp.cpp +++ b/src/WResp.cpp @@ -35,6 +35,17 @@ void JsonSockEvent::_safecat(const char *val, bool escape) { else strcat(this->buff, val); if(escape) strcat(this->buff, "\""); } +void BufferedJsonFormatter::setBuffer(char *b, size_t sz) { + this->buff = b; + this->buffSize = sz; + this->buff[0] = 0; + this->_nocomma = true; + this->_objects = 0; + this->_arrays = 0; +} +size_t BufferedJsonFormatter::length() const { + return strlen(this->buff); +} void AsyncJsonResp::beginResponse(AsyncWebServerRequest *request, char *buff, size_t buffSize) { this->_request = request; this->buff = buff; diff --git a/src/WResp.h b/src/WResp.h index 7356189a..17e6c8ce 100644 --- a/src/WResp.h +++ b/src/WResp.h @@ -52,6 +52,11 @@ class JsonFormatter { void addElem(const char *name, const char *val); void addElem(const char* name, uint64_t lval); }; +class BufferedJsonFormatter : public JsonFormatter { + public: + void setBuffer(char *b, size_t sz); + size_t length() const; +}; class AsyncJsonResp : public JsonFormatter { protected: void _safecat(const char *val, bool escape = false) override; diff --git a/src/Web.cpp b/src/Web.cpp index 71c063c6..27a6aced 100644 --- a/src/Web.cpp +++ b/src/Web.cpp @@ -17,6 +17,7 @@ #include #include #include +#include extern ConfigSettings settings; extern SSDPClass SSDP; @@ -418,6 +419,194 @@ static void serializeGitRelease(GitRelease *rel, JsonFormatter &json) { json.endObject(); } +// Memory-bounded streaming builder for /controller. Produces the JSON payload +// one small unit at a time into a fixed 3KB buffer, so a chunked HTTP response +// can drain it as the TCP send window allows — no growing cbuf in the async +// response stream. +class ControllerChunker { +public: + enum : uint8_t { S_HEADER, S_ROOMS, S_SHADES, S_GROUPS, S_REPEATERS, S_DONE }; + static constexpr uint16_t LS_OPEN = 0xFFFF; + + BufferedJsonFormatter fmt; + char unit[3072]; + size_t unitLen = 0; + size_t consumed = 0; + uint8_t section = S_HEADER; + uint16_t idx = 0; + uint16_t lsIdx = LS_OPEN; + bool firstInArray = true; + bool firstInLSArray = true; + + size_t generate(uint8_t *out, size_t maxLen) { + size_t written = 0; + while(written < maxLen) { + if(consumed < unitLen) { + size_t n = unitLen - consumed; + if(n > maxLen - written) n = maxLen - written; + memcpy(out + written, unit + consumed, n); + consumed += n; + written += n; + } else { + produceNext(); + if(unitLen == 0) return written; + } + } + return written; + } + +private: + void resetFmt(size_t offset = 0) { + unit[offset] = 0; + fmt.setBuffer(unit + offset, sizeof(unit) - offset); + } + + void produceNext() { + consumed = 0; + unitLen = 0; + switch(section) { + case S_HEADER: { + resetFmt(); + fmt.beginObject(); + fmt.addElem("maxRooms", (uint8_t)SOMFY_MAX_ROOMS); + fmt.addElem("maxShades", (uint8_t)SOMFY_MAX_SHADES); + fmt.addElem("maxGroups", (uint8_t)SOMFY_MAX_GROUPS); + fmt.addElem("maxGroupedShades", (uint8_t)SOMFY_MAX_GROUPED_SHADES); + fmt.addElem("maxLinkedRemotes", (uint8_t)SOMFY_MAX_LINKED_REMOTES); + fmt.addElem("startingAddress", (uint32_t)somfy.startingAddress); + fmt.beginObject("transceiver"); + fmt.beginObject("config"); + serializeTransceiverConfig(fmt); + fmt.endObject(); + fmt.endObject(); + fmt.beginObject("version"); + serializeGitVersion(fmt); + fmt.endObject(); + fmt.beginArray("rooms"); + unitLen = strlen(unit); + section = S_ROOMS; + idx = 0; + firstInArray = true; + return; + } + case S_ROOMS: { + while(idx < SOMFY_MAX_ROOMS && somfy.rooms[idx].roomId == 0) idx++; + if(idx >= SOMFY_MAX_ROOMS) { + strcpy(unit, "],\"shades\":["); + unitLen = strlen(unit); + section = S_SHADES; + idx = 0; + firstInArray = true; + return; + } + size_t pos = 0; + if(!firstInArray) unit[pos++] = ','; + resetFmt(pos); + fmt.beginObject(); + serializeRoom(&somfy.rooms[idx], fmt); + fmt.endObject(); + unitLen = pos + strlen(unit + pos); + idx++; + firstInArray = false; + return; + } + case S_SHADES: { + while(idx < SOMFY_MAX_SHADES && somfy.shades[idx].getShadeId() == 255) idx++; + if(idx >= SOMFY_MAX_SHADES) { + strcpy(unit, "],\"groups\":["); + unitLen = strlen(unit); + section = S_GROUPS; + idx = 0; + lsIdx = LS_OPEN; + firstInArray = true; + return; + } + size_t pos = 0; + if(!firstInArray) unit[pos++] = ','; + resetFmt(pos); + fmt.beginObject(); + serializeShade(&somfy.shades[idx], fmt); + fmt.endObject(); + unitLen = pos + strlen(unit + pos); + idx++; + firstInArray = false; + return; + } + case S_GROUPS: { + while(idx < SOMFY_MAX_GROUPS && somfy.groups[idx].getGroupId() == 255) idx++; + if(idx >= SOMFY_MAX_GROUPS) { + strcpy(unit, "],\"repeaters\":["); + unitLen = strlen(unit); + section = S_REPEATERS; + idx = 0; + firstInArray = true; + return; + } + SomfyGroup *g = &somfy.groups[idx]; + if(lsIdx == LS_OPEN) { + size_t pos = 0; + if(!firstInArray) unit[pos++] = ','; + resetFmt(pos); + fmt.beginObject(); + serializeGroupRef(g, fmt); + fmt.beginArray("linkedShades"); + unitLen = pos + strlen(unit + pos); + lsIdx = 0; + firstInArray = false; + firstInLSArray = true; + return; + } + SomfyShade *lshade = nullptr; + while(lsIdx < SOMFY_MAX_GROUPED_SHADES) { + uint8_t sid = g->linkedShades[lsIdx]; + if(sid > 0 && sid < 255) { + lshade = somfy.getShadeById(sid); + if(lshade) break; + } + lsIdx++; + } + if(!lshade) { + strcpy(unit, "]}"); + unitLen = 2; + idx++; + lsIdx = LS_OPEN; + return; + } + size_t pos = 0; + if(!firstInLSArray) unit[pos++] = ','; + resetFmt(pos); + fmt.beginObject(); + serializeShadeRef(lshade, fmt); + fmt.endObject(); + unitLen = pos + strlen(unit + pos); + lsIdx++; + firstInLSArray = false; + return; + } + case S_REPEATERS: { + while(idx < SOMFY_MAX_REPEATERS && somfy.repeaters[idx] == 0) idx++; + if(idx >= SOMFY_MAX_REPEATERS) { + strcpy(unit, "]}"); + unitLen = 2; + section = S_DONE; + return; + } + size_t pos = 0; + if(!firstInArray) unit[pos++] = ','; + pos += snprintf(unit + pos, sizeof(unit) - pos, "%lu", (unsigned long)somfy.repeaters[idx]); + unitLen = pos; + idx++; + firstInArray = false; + return; + } + case S_DONE: + default: + unitLen = 0; + return; + } + } +}; + // -- Async handler implementations -- void Web::handleDiscovery(AsyncWebServerRequest *request) { if(request->method() == HTTP_POST || request->method() == HTTP_GET) { @@ -501,37 +690,12 @@ void Web::handleController(AsyncWebServerRequest *request) { if(!this->isAuthenticated(request)) return; if(request->method() == HTTP_POST || request->method() == HTTP_GET) { settings.printAvailHeap(); - AsyncJsonResp resp; - resp.beginResponse(request, g_async_content, sizeof(g_async_content)); - resp.beginObject(); - resp.addElem("maxRooms", (uint8_t)SOMFY_MAX_ROOMS); - resp.addElem("maxShades", (uint8_t)SOMFY_MAX_SHADES); - resp.addElem("maxGroups", (uint8_t)SOMFY_MAX_GROUPS); - resp.addElem("maxGroupedShades", (uint8_t)SOMFY_MAX_GROUPED_SHADES); - resp.addElem("maxLinkedRemotes", (uint8_t)SOMFY_MAX_LINKED_REMOTES); - resp.addElem("startingAddress", (uint32_t)somfy.startingAddress); - resp.beginObject("transceiver"); - resp.beginObject("config"); - serializeTransceiverConfig(resp); - resp.endObject(); - resp.endObject(); - resp.beginObject("version"); - serializeGitVersion(resp); - resp.endObject(); - resp.beginArray("rooms"); - serializeRooms(resp); - resp.endArray(); - resp.beginArray("shades"); - serializeShades(resp); - resp.endArray(); - resp.beginArray("groups"); - serializeGroups(resp); - resp.endArray(); - resp.beginArray("repeaters"); - serializeRepeaters(resp); - resp.endArray(); - resp.endObject(); - resp.endResponse(); + auto state = std::make_shared(); + AsyncWebServerResponse *response = request->beginChunkedResponse(_encoding_json, + [state](uint8_t *buffer, size_t maxLen, size_t index) -> size_t { + return state->generate(buffer, maxLen); + }); + request->send(response); } else request->send(404, _encoding_text, _response_404); }