This repository was archived by the owner on Jan 20, 2025. It is now read-only.
Handle edge-case in which we receive a partial WS header #953
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since in my use-case I use this library to send and receive lots of data at high rates I spotted a quite infrequent bug that does not occur with occasional sending/receiving.
The issue occurs when receiving lots of data.
Since WebSocket and TCP framing are independent it occasionally happens that the buffer passed to
AsyncWebSocketClient::_onDatais truncated. At the current state the library handles the case in which the truncation happens mid-payload.However, truncation may happen anywhere and it occasionally happens in the WS frame header too. This happens extremely rarely in practice, since most ESP applications predominantly send data - not receive it, and since if data is sent one frame at a time the frames are unlikely to be truncated, and if it happens it happens mid payload.
Sending lots of data increases the probability by a lot, since the frames will most likely be delivered one after the other in one big buffer; there is going to be a lot more headers and as a consequence more places where this bug could occur.
The fix consists in storing a partial header in the heap temporarily, then merging the remaining chunk later once it is received.
In order to do so, what used to be
uint8_t *fdata(which I renamed toheaderBuffor better readability) can now be either a pointer to an offset inside the input buffer or a heap-allocated buffer. The buffer is always allocated to 16 bytes, which I determined is the maximum, worst-case scenario size of the WS frame header.The header parsing part of the code now performs the following operations (I'd like to explain the logic since the code is complex):
!_pstate(== if we're not waiting for the remaining part of a frame truncated mid-payload)headerBufto the current position in the input bufferYou will notice I had to use a bunch of
gotos to handle the "failure - truncated header" scenario. It could have been a try-catch-finally, but as far as I know ESP8266 binaries are built with-fno-exceptions.This code, combined with my other PR #952 was tested with Valgrind and I can claim it works very well: