Skip to content

Commit a443fcd

Browse files
committed
fix(ws): Exit immediately on failure
In cases where we close the connection, ensure we immediately exit to avoid accessing any member variables after the object has been destroyed. H/t @mathieucarbou
1 parent 4e4fbee commit a443fcd

2 files changed

Lines changed: 16 additions & 6 deletions

File tree

src/AsyncWebSocket.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ void AsyncWebSocketClient::close(uint16_t code, const char *message) {
535535
if (c) {
536536
c->abort();
537537
}
538+
return;
538539
}
539540
}
540541
_queueControl(WS_DISCONNECT);
@@ -680,7 +681,9 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
680681
"[%s][%" PRIu32 "] DATA processing next fragment of %s frame %" PRIu32 ", index: %" PRIu64 ", len: %" PRIu32 "", _server->url(), _clientId,
681682
(_pinfo.message_opcode == WS_TEXT) ? "text" : "binary", _pinfo.num, _pinfo.index, (uint32_t)datalen
682683
);
683-
_handleDataEvent(data, datalen, datalen == plen); // datalen == plen means that we are processing the last part of the current TCP packet
684+
if (!_handleDataEvent(data, datalen, datalen == plen)) { // datalen == plen means that we are processing the last part of the current TCP packet
685+
return; // stop processing on failure
686+
}
684687
}
685688

686689
// track index for next fragment
@@ -704,6 +707,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
704707
if (_client) {
705708
_client->close();
706709
}
710+
return; // our object is now destroyed, so we must return immediately to avoid accessing any member
707711
} else {
708712
_status = WS_DISCONNECTING;
709713
if (_client) {
@@ -729,7 +733,9 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
729733
(_pinfo.message_opcode == WS_TEXT) ? "text" : "binary", _pinfo.num, _pinfo.index, (uint32_t)datalen
730734
);
731735

732-
_handleDataEvent(data, datalen, datalen == plen); // datalen == plen means that we are processing the last part of the current TCP packet
736+
if (!_handleDataEvent(data, datalen, datalen == plen)) { // datalen == plen means that we are processing the last part of the current TCP packet
737+
return; // stop processing on failure
738+
}
733739

734740
if (_pinfo.final) {
735741
_pinfo.num = 0;
@@ -759,7 +765,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
759765
}
760766
}
761767

762-
void AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len, bool endOfPaquet) {
768+
bool AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len, bool endOfPaquet) {
763769
// ------------------------------------------------------------
764770
// Issue 384: https://github.com/ESP32Async/ESPAsyncWebServer/issues/384
765771
// Discussion: https://github.com/ESP32Async/ESPAsyncWebServer/pull/383#discussion_r2760425739
@@ -790,9 +796,11 @@ void AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len, bool endO
790796
_server->_handleEvent(this, WS_EVT_DATA, (void *)&_pinfo, copy.get(), len);
791797
} else {
792798
async_ws_log_e("Failed to allocate");
793-
if (_client) {
794-
_client->abort();
799+
AsyncClient *c = _client;
800+
if (c) {
801+
c->abort();
795802
}
803+
return false; // failure!
796804
}
797805
} else {
798806
uint8_t backup = data[len];
@@ -803,6 +811,7 @@ void AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len, bool endO
803811
} else {
804812
_server->_handleEvent(this, WS_EVT_DATA, (void *)&_pinfo, data, len);
805813
}
814+
return true;
806815
}
807816

808817
size_t AsyncWebSocketClient::printf(const char *format, ...) {

src/AsyncWebSocket.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ class AsyncWebSocketClient {
235235
void _clearQueue();
236236

237237
// this function is called when a text message is received, in order to copy the buffer and place a null terminator at the end of the buffer for easier handling of text messages.
238-
void _handleDataEvent(uint8_t *data, size_t len, bool endOfPaquet);
238+
// Returns true on success, false on failure (e.g. memory allocation failure)
239+
bool _handleDataEvent(uint8_t *data, size_t len, bool endOfPaquet);
239240

240241
public:
241242
void *_tempObject;

0 commit comments

Comments
 (0)