mirror of
https://github.com/rstrouse/ESPSomfy-RTS.git
synced 2026-05-12 21:32:13 +02:00
Merge pull request #25 from cjkas/scz/split-response
Memory-bounded streaming builder for /controller. Produces the JSON p…
This commit is contained in:
commit
15e7dd1ee2
4 changed files with 270 additions and 31 deletions
59
docs/KNOWN_ISSUES.md
Normal file
59
docs/KNOWN_ISSUES.md
Normal file
|
|
@ -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.
|
||||||
|
|
@ -35,6 +35,17 @@ void JsonSockEvent::_safecat(const char *val, bool escape) {
|
||||||
else strcat(this->buff, val);
|
else strcat(this->buff, val);
|
||||||
if(escape) strcat(this->buff, "\"");
|
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) {
|
void AsyncJsonResp::beginResponse(AsyncWebServerRequest *request, char *buff, size_t buffSize) {
|
||||||
this->_request = request;
|
this->_request = request;
|
||||||
this->buff = buff;
|
this->buff = buff;
|
||||||
|
|
|
||||||
|
|
@ -52,6 +52,11 @@ class JsonFormatter {
|
||||||
void addElem(const char *name, const char *val);
|
void addElem(const char *name, const char *val);
|
||||||
void addElem(const char* name, uint64_t lval);
|
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 {
|
class AsyncJsonResp : public JsonFormatter {
|
||||||
protected:
|
protected:
|
||||||
void _safecat(const char *val, bool escape = false) override;
|
void _safecat(const char *val, bool escape = false) override;
|
||||||
|
|
|
||||||
226
src/Web.cpp
226
src/Web.cpp
|
|
@ -17,6 +17,7 @@
|
||||||
#include <AsyncTCP.h>
|
#include <AsyncTCP.h>
|
||||||
#include <ESPAsyncWebServer.h>
|
#include <ESPAsyncWebServer.h>
|
||||||
#include <AsyncJson.h>
|
#include <AsyncJson.h>
|
||||||
|
#include <memory>
|
||||||
|
|
||||||
extern ConfigSettings settings;
|
extern ConfigSettings settings;
|
||||||
extern SSDPClass SSDP;
|
extern SSDPClass SSDP;
|
||||||
|
|
@ -418,6 +419,194 @@ static void serializeGitRelease(GitRelease *rel, JsonFormatter &json) {
|
||||||
json.endObject();
|
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 --
|
// -- Async handler implementations --
|
||||||
void Web::handleDiscovery(AsyncWebServerRequest *request) {
|
void Web::handleDiscovery(AsyncWebServerRequest *request) {
|
||||||
if(request->method() == HTTP_POST || request->method() == HTTP_GET) {
|
if(request->method() == HTTP_POST || request->method() == HTTP_GET) {
|
||||||
|
|
@ -501,37 +690,12 @@ void Web::handleController(AsyncWebServerRequest *request) {
|
||||||
if(!this->isAuthenticated(request)) return;
|
if(!this->isAuthenticated(request)) return;
|
||||||
if(request->method() == HTTP_POST || request->method() == HTTP_GET) {
|
if(request->method() == HTTP_POST || request->method() == HTTP_GET) {
|
||||||
settings.printAvailHeap();
|
settings.printAvailHeap();
|
||||||
AsyncJsonResp resp;
|
auto state = std::make_shared<ControllerChunker>();
|
||||||
resp.beginResponse(request, g_async_content, sizeof(g_async_content));
|
AsyncWebServerResponse *response = request->beginChunkedResponse(_encoding_json,
|
||||||
resp.beginObject();
|
[state](uint8_t *buffer, size_t maxLen, size_t index) -> size_t {
|
||||||
resp.addElem("maxRooms", (uint8_t)SOMFY_MAX_ROOMS);
|
return state->generate(buffer, maxLen);
|
||||||
resp.addElem("maxShades", (uint8_t)SOMFY_MAX_SHADES);
|
});
|
||||||
resp.addElem("maxGroups", (uint8_t)SOMFY_MAX_GROUPS);
|
request->send(response);
|
||||||
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();
|
|
||||||
}
|
}
|
||||||
else request->send(404, _encoding_text, _response_404);
|
else request->send(404, _encoding_text, _response_404);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue