Fix #384: Remove the buffer overflow intended by design allowing users to add a null terminator after the buffer end

[mathieucarbou]

Fix #384

Initial discussion: #383 (comment)

Wifi doc: https://github.com/ESP32Async/ESPAsyncWebServer/wiki#async-websocket-event

Doc requested by design that users add a null terminating char after the buffer ends

[mathieucarbou]

Note: I decided to always do the copy regardless of the frame type (WS_TEXT, WS_BINARY, WS_CONTINUATION) because only the first frame holds the info WS_TEXT, WS_BINARY. Others are continuation frame.

So this is more correct by the spec since when in next frames we do not have the information about the message type, but we could decide to optimize and only do the copy when we encounter a WS_TEXT frame, because a frame max length is 2^63 so this is not likely that we would encounter continuation frames on a MCU...

I am opened to both solutions.

[willmmiles]

Strictly speaking, we only need to make a copy if the message is at the end of the pbuf; for messages that don't consume the entire packet, we could pre-read or null out and restore the last byte, which would be faster.

[mathieucarbou]

Right but we don't know that right ? We only have the pointer to the buffer and the quantity that can be read.

[willmmiles]

At this point in the processing loop, we do know: if datalen == plen, we must copy to add a null terminator, otherwise it's safe to "borrow" the next byte as there's at least one byte of the next message header still in the packet buffer. (We could pass the result of that test as an argument to #388 _handleDataEvent to choose the handling algorithm.)

[mathieucarbou]

@willmmiles I will try this optimization.

when the content of data frame is handled, we have: const size_t datalen = std::min((size_t)(_pinfo.len - _pinfo.index), plen);.

datalen will basically be always equal to plen, except if the network buffer contains several fragments of frames inside, in this case it will be less.

So it would be in the cases where browsers are sending several ws messages within the same paquet without waiting for a ack...

Let me add a commit in #388 and test it.

[Copilot]

Pull request overview

Removes the previously “by design” out-of-bounds write pattern (data[len] = 0) in WebSocket receive callbacks by ensuring callbacks can still safely treat text payloads as null-terminated strings without requiring users to overflow the buffer.

Changes:

• Removed implicit data[len] = 0 behavior from AsyncWebSocketMessageHandler.
• Updated WebSocket receive path to provide a null-terminated buffer via a temporary copy instead of relying on an out-of-bounds byte.
• Updated examples to stop writing data[len] = 0 and refreshed command hints.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/AsyncWebSocket.h	Removes handler-side null-termination that could trigger an out-of-bounds write.
src/AsyncWebSocket.cpp	Replaces “backup/restore byte past end” with a temporary buffer copy that includes a terminator.
idf_component_examples/websocket/main/main.cpp	Updates example to no longer write beyond the received buffer.
examples/WebSocket/WebSocket.ino	Updates example to no longer write beyond the received buffer; refreshes usage comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[willmmiles]

Strictly speaking, we only need to make a copy if the message is at the end of the pbuf; for messages that don't consume the entire packet, we could pre-read or null out and restore the last byte, which would be faster.