Skip to content

Commit 340c93f

Browse files
mathieucarbouCopilotwillmmilesclaude
authored
Fix request processing failures caused by failed allocations (#501)
* Fix request processing failures caused by failed allocations Abort the request instead of crashing, corrupting the wire format or silently mis-parsing it when a heap allocation fails. Arduino String leaves a failed string empty, and its c_str() is null on ESP32. Sites that copied header or body data and then passed c_str() to C string functions could crash (issue #500), and sites that stored the copy in request state silently lost or corrupted data. Header parsing: - Add AsyncWebHeader::tryParse() reporting ParseStatus::NoMemory so a failed name/value copy fails the request instead of dropping the header (a lost Content-Length desynchronizes the body parser). - Scan Accept and Transfer-Encoding in place: the lowercased copies are gone, so issue #500 cannot happen and chunked framing is no longer silently missed. - Abort on failed copies into _host, _contentType, _authorization and on failed header-line/body accumulation in the plain and multipart parsers. - Write the 100-continue reply from the literal: the String copy exceeded the small-buffer size and an OOM one silently stalled the client. Responses: - Check every concat() in _assembleHead(); callers now close the connection instead of writing a truncated head (basic, abstract, WebSocket and SSE responses). - AsyncWebSocketResponse flags RESPONSE_FAILED when it cannot build Sec-WebSocket-Accept, and _sourceValid() reflects it, so the upgrade is refused rather than completing with a garbage accept header. Allocation helpers: - Use std::nothrow for response objects created by beginResponse*(), AsyncStaticWebHandler and AsyncEventSource so failures are observable instead of aborting the device; send()/_send() handle a null response. - Replace plain new with malloc in generateBasicHash() and the WebSocket printf helpers, where the null checks were dead code. Static file handler: - Stop and fall through to 404 when the path cannot be built, rather than serving the default or a wrong file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * _assembleHead return false if allocation failed * Switch to base64_encode_block * Introduc request->failWithRawResponse(); to fail a request with a custom raw response * failWithRawResponse => sendRawResponse * Improve AsyncWebHeader parsing status usign SSO buffer * Fix usage of asyncsrv::T_HTTP_503_HEAD in sendRawResponse calls * AsyncWebHeader(String name, String value) : _name(std::move(name)), _value(std::move(value)) {} * abort() => sendRawResponse() * Remove strlen() * Fail the parse in sendRawResponse() and abort() Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Reply to header parse failures from _onData Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Don't run the handler after a chunked encoding error Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop multipart parsing by return value after a raw response Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Reply to plain body parse failures from _onData Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop parsing when a body or upload handler ends the request Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Don't search for the default file after a raw response Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop dispatch when canHandle() ends the request Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Don't leak a response sent after the request has ended Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Return the raw response from the multipart parser _parseLine returns REQUEST_ENDED too when canHandle() ends the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop PARSE_REQ_FAIL in favour of PARSE_REQ_END Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix error return on request parse failure --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Will Miles <will@willmiles.net> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 75dafb4 commit 340c93f

12 files changed

Lines changed: 532 additions & 242 deletions

‎src/AsyncEventSource.cpp‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,13 @@ bool AsyncEventSource::canHandle(AsyncWebServerRequest *request) const {
386386
}
387387

388388
void AsyncEventSource::handleRequest(AsyncWebServerRequest *request) {
389-
request->send(new AsyncEventSourceResponse(this));
389+
AsyncWebServerResponse *response = new (std::nothrow) AsyncEventSourceResponse(this);
390+
if (!response) {
391+
async_ws_log_e("Failed to allocate");
392+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
393+
return;
394+
}
395+
request->send(response);
390396
}
391397

392398
// list iteration protected by caller's lock
@@ -415,7 +421,14 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(AsyncEventSource *server) : _
415421

416422
void AsyncEventSourceResponse::_respond(AsyncWebServerRequest *request) {
417423
String out;
418-
_assembleHead(out, request->version());
424+
if (!_assembleHead(out, request->version())) {
425+
// Head assembly ran out of memory: fail the stream instead of sending a
426+
// corrupted head.
427+
async_ws_log_e("Failed to allocate");
428+
_state = RESPONSE_FAILED;
429+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
430+
return;
431+
}
419432
uint32_t lastId = 0;
420433
if (request->hasHeader(T_Last_Event_ID)) {
421434
lastId = strtoul(request->getHeader(T_Last_Event_ID)->value().c_str(), nullptr, 10);

‎src/AsyncJson.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ void AsyncCallbackJsonWebHandler::handleBody(AsyncWebServerRequest *request, uin
208208
// way to handle this would be to use a String instead of
209209
// a fixed-length buffer, but for now we just reject.
210210
async_ws_log_w("AsyncJson cannot handle chunked requests without X-Expected-Entity-Length");
211-
request->abort();
211+
request->sendRawResponse(asyncsrv::T_HTTP_400_HEAD);
212212
return;
213213
}
214214

@@ -217,7 +217,7 @@ void AsyncCallbackJsonWebHandler::handleBody(AsyncWebServerRequest *request, uin
217217
request->_tempObject = calloc(total + 1, sizeof(uint8_t)); // null-terminated string
218218
if (request->_tempObject == NULL) {
219219
async_ws_log_e("Failed to allocate");
220-
request->abort();
220+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
221221
return;
222222
}
223223
}

‎src/AsyncWebHeader.cpp‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33

44
#include <ESPAsyncWebServer.h>
55

6+
#include <utility>
7+
8+
AsyncWebHeader::ParseError AsyncWebHeader::parseError() const {
9+
return _name.length() ? ParseError::NONE : (_value.length() ? ParseError::ALLOC_FAIL : ParseError::INVALID);
10+
}
11+
612
const AsyncWebHeader AsyncWebHeader::parse(const char *data) {
713
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers
814
// In HTTP/1.X, a header is a case-insensitive name followed by a colon, then optional whitespace which will be ignored, and finally by its value
@@ -29,7 +35,21 @@ const AsyncWebHeader AsyncWebHeader::parse(const char *data) {
2935
startOfValue++;
3036
}
3137
String name;
32-
name.reserve(colon - data);
33-
name.concat(data, colon - data);
34-
return AsyncWebHeader(name, String(startOfValue));
38+
if (!name.reserve(colon - data) || !name.concat(data, colon - data)) {
39+
async_ws_log_e("Failed to allocate while parsing header");
40+
// 'M' means NoMemory - no allocation => set in SSO buffer directly
41+
return AsyncWebHeader(String(), String('M'));
42+
}
43+
String value(startOfValue);
44+
// A copy of a non-empty value can never be empty: Arduino String leaves
45+
// length() == 0 only when the heap buffer could not be allocated (its
46+
// c_str() is then null on ESP32, which downstream header consumers such as
47+
// atoi() and strncasecmp() would dereference). Report it so the caller can
48+
// fail the request.
49+
if (*startOfValue && !value.length()) {
50+
async_ws_log_e("Failed to allocate while parsing header");
51+
// 'M' means NoMemory - no allocation => set in SSO buffer directly
52+
return AsyncWebHeader(String(), String('M'));
53+
}
54+
return AsyncWebHeader(std::move(name), std::move(value));
3555
}

‎src/AsyncWebSocket.cpp‎

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -859,19 +859,17 @@ size_t AsyncWebSocketClient::printf(const char *format, ...) {
859859
return 0;
860860
}
861861

862-
char *buffer = new char[len + 1];
863-
862+
std::unique_ptr<char[]> buffer(new (std::nothrow) char[len + 1]);
864863
if (!buffer) {
864+
async_ws_log_e("Failed to allocate");
865865
return 0;
866866
}
867867

868868
va_start(arg, format);
869-
len = vsnprintf(buffer, len + 1, format, arg);
869+
len = vsnprintf(buffer.get(), len + 1, format, arg);
870870
va_end(arg);
871871

872-
bool enqueued = text(buffer, len);
873-
delete[] buffer;
874-
return enqueued ? len : 0;
872+
return text(buffer.get(), len) ? len : 0;
875873
}
876874

877875
#ifdef ESP8266
@@ -885,19 +883,17 @@ size_t AsyncWebSocketClient::printf_P(PGM_P formatP, ...) {
885883
return 0;
886884
}
887885

888-
char *buffer = new char[len + 1];
889-
886+
std::unique_ptr<char[]> buffer(new (std::nothrow) char[len + 1]);
890887
if (!buffer) {
888+
async_ws_log_e("Failed to allocate");
891889
return 0;
892890
}
893891

894892
va_start(arg, formatP);
895-
len = vsnprintf_P(buffer, len + 1, formatP, arg);
893+
len = vsnprintf_P(buffer.get(), len + 1, formatP, arg);
896894
va_end(arg);
897895

898-
bool enqueued = text(buffer, len);
899-
delete[] buffer;
900-
return enqueued ? len : 0;
896+
return text(buffer.get(), len) ? len : 0;
901897
}
902898
#endif
903899

@@ -1369,19 +1365,17 @@ size_t AsyncWebSocket::printfAll(const char *format, ...) {
13691365
return 0;
13701366
}
13711367

1372-
char *buffer = new char[len + 1];
1373-
1368+
std::unique_ptr<char[]> buffer(new (std::nothrow) char[len + 1]);
13741369
if (!buffer) {
1370+
async_ws_log_e("Failed to allocate");
13751371
return 0;
13761372
}
13771373

13781374
va_start(arg, format);
1379-
len = vsnprintf(buffer, len + 1, format, arg);
1375+
len = vsnprintf(buffer.get(), len + 1, format, arg);
13801376
va_end(arg);
13811377

1382-
AsyncWebSocket::SendStatus status = textAll(buffer, len);
1383-
delete[] buffer;
1384-
return status == DISCARDED ? 0 : len;
1378+
return textAll(buffer.get(), len) == DISCARDED ? 0 : len;
13851379
}
13861380

13871381
#ifdef ESP8266
@@ -1408,19 +1402,17 @@ size_t AsyncWebSocket::printfAll_P(PGM_P formatP, ...) {
14081402
return 0;
14091403
}
14101404

1411-
char *buffer = new char[len + 1];
1412-
1405+
std::unique_ptr<char[]> buffer(new (std::nothrow) char[len + 1]);
14131406
if (!buffer) {
1407+
async_ws_log_e("Failed to allocate");
14141408
return 0;
14151409
}
14161410

14171411
va_start(arg, formatP);
1418-
len = vsnprintf_P(buffer, len + 1, formatP, arg);
1412+
len = vsnprintf_P(buffer.get(), len + 1, formatP, arg);
14191413
va_end(arg);
14201414

1421-
AsyncWebSocket::SendStatus status = textAll(buffer, len);
1422-
delete[] buffer;
1423-
return status == DISCARDED ? 0 : len;
1415+
return textAll(buffer.get(), len) == DISCARDED ? 0 : len;
14241416
}
14251417
#endif
14261418

@@ -1464,15 +1456,28 @@ void AsyncWebSocket::handleRequest(AsyncWebServerRequest *request) {
14641456
const AsyncWebHeader *version = request->getHeader(WS_STR_VERSION);
14651457
if (version->value().toInt() != 13) {
14661458
AsyncWebServerResponse *response = request->beginResponse(400);
1459+
if (!response) {
1460+
async_ws_log_e("Failed to allocate");
1461+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
1462+
return;
1463+
}
14671464
response->addHeader(WS_STR_VERSION, T_13);
14681465
request->send(response);
14691466
return;
14701467
}
14711468
const AsyncWebHeader *key = request->getHeader(WS_STR_KEY);
1472-
AsyncWebServerResponse *response = new AsyncWebSocketResponse(key->value(), this);
1469+
AsyncWebServerResponse *response = new (std::nothrow) AsyncWebSocketResponse(key->value(), this);
14731470
if (response == NULL) {
14741471
async_ws_log_e("Failed to allocate");
1475-
request->abort();
1472+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
1473+
return;
1474+
}
1475+
if (!response->_sourceValid()) {
1476+
// The response could not build the Sec-WebSocket-Accept header (out of
1477+
// memory): fail the upgrade instead of sending an invalid handshake.
1478+
async_ws_log_e("Failed to allocate");
1479+
delete response;
1480+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
14761481
return;
14771482
}
14781483
if (request->hasHeader(WS_STR_PROTOCOL)) {
@@ -1509,6 +1514,7 @@ AsyncWebSocketResponse::AsyncWebSocketResponse(const String &key, AsyncWebSocket
15091514
String k;
15101515
if (!k.reserve(key.length() + WS_STR_UUID_LEN)) {
15111516
async_ws_log_e("Failed to allocate");
1517+
_state = RESPONSE_FAILED;
15121518
return;
15131519
}
15141520
k.concat(key);
@@ -1539,7 +1545,18 @@ AsyncWebSocketResponse::AsyncWebSocketResponse(const String &key, AsyncWebSocket
15391545

15401546
void AsyncWebSocketResponse::_respond(AsyncWebServerRequest *request) {
15411547
if (_state == RESPONSE_FAILED) {
1542-
request->client()->close();
1548+
// Sec-WebSocket-Accept could not be computed in the constructor: refuse
1549+
// the upgrade instead of doing a bare close.
1550+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
1551+
return;
1552+
}
1553+
String out;
1554+
if (!_assembleHead(out, request->version())) {
1555+
// Head assembly ran out of memory: fail the upgrade instead of sending a
1556+
// corrupted handshake.
1557+
async_ws_log_e("Failed to allocate");
1558+
_state = RESPONSE_FAILED;
1559+
request->sendRawResponse(asyncsrv::T_HTTP_503_HEAD);
15431560
return;
15441561
}
15451562
// unbind client's onAck callback from AsyncWebServerRequest's, we will destroy it on next callback and steal the client,
@@ -1554,8 +1571,6 @@ void AsyncWebSocketResponse::_respond(AsyncWebServerRequest *request) {
15541571
},
15551572
this
15561573
);
1557-
String out;
1558-
_assembleHead(out, request->version());
15591574
request->client()->write(out.c_str(), _headLength);
15601575
_state = RESPONSE_WAIT_ACK;
15611576
}

‎src/AsyncWebSocket.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,9 @@ class AsyncWebSocketResponse : public AsyncWebServerResponse {
468468
return 0;
469469
};
470470
bool _sourceValid() const override {
471-
return true;
471+
// _state is RESPONSE_FAILED when the Sec-WebSocket-Accept computation
472+
// could not allocate (see the constructor).
473+
return _state != RESPONSE_FAILED;
472474
}
473475
};
474476

‎src/ESPAsyncWebServer.h‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -461,11 +461,18 @@ class AsyncWebHeader {
461461
String _value;
462462

463463
public:
464+
// Result of AsyncWebHeader::parse()
465+
enum class ParseError {
466+
NONE, // header parsed, returned header is valid
467+
INVALID, // malformed header line, returned header is empty
468+
ALLOC_FAIL, // allocation failure: request must be failed, not skipped
469+
};
470+
464471
AsyncWebHeader() {}
465472
AsyncWebHeader(const AsyncWebHeader &) = default;
466473
AsyncWebHeader(AsyncWebHeader &&) = default;
467474
AsyncWebHeader(const char *name, const char *value) : _name(name), _value(value) {}
468-
AsyncWebHeader(const String &name, const String &value) : _name(name), _value(value) {}
475+
AsyncWebHeader(String name, String value) : _name(std::move(name)), _value(std::move(value)) {}
469476

470477
[[deprecated("Use AsyncWebHeader::parse(data) instead")]]
471478
AsyncWebHeader(const String &data)
@@ -480,14 +487,26 @@ class AsyncWebHeader {
480487
const String &value() const {
481488
return _value;
482489
}
490+
ParseError parseError() const;
483491

484492
String toString() const;
485493

486494
// returns true if the header is valid
487495
operator bool() const {
488-
return _name.length();
496+
return parseError() == ParseError::NONE;
489497
}
490498

499+
/**
500+
* @brief Parse a header line.
501+
*
502+
* A NoMemory result must abort the request (see issue #500): a String whose
503+
* internal allocation failed has a null c_str() on ESP32 and an empty one on
504+
* ESP8266, and downstream header consumers would either dereference it or
505+
* silently misparse the request.
506+
*
507+
* @param data raw header line "Name: value" (no CR/LF)
508+
* @return the parse status together with the parsed header
509+
*/
491510
static const AsyncWebHeader parse(const String &data) {
492511
return parse(data.c_str());
493512
}
@@ -584,7 +603,7 @@ class AsyncWebServerRequest {
584603
size_t _chunkSize; // Size of the current chunk
585604
uint8_t _chunkedParseState;
586605
uint8_t _chunkedLastChar;
587-
bool _parseChunkedBytes(uint8_t *data, size_t len);
606+
void _parseChunkedBytes(uint8_t *data, size_t len);
588607

589608
void _onPoll();
590609
void _onAck(size_t len, uint32_t time);
@@ -594,14 +613,15 @@ class AsyncWebServerRequest {
594613
void _onData(void *buf, size_t len);
595614

596615
bool _parseReqHead();
597-
bool _parseReqHeader();
598-
void _parseLine();
599-
void _parsePlainPostChar(uint8_t data);
600-
void _parseMultipartPostByte(uint8_t data, bool last);
616+
// return the raw response to stop the request with, or nullptr to continue
617+
const char *_parseReqHeader();
618+
const char *_parseLine();
619+
const char *_parsePlainPostChar(uint8_t data);
620+
const char *_parseMultipartPostByte(uint8_t data, bool last);
601621
void _addGetParams(const String &params);
602622

603623
void _handleUploadStart();
604-
void _handleUploadByte(uint8_t data, bool last);
624+
bool _handleUploadByte(uint8_t data, bool last);
605625
void _handleUploadEnd();
606626

607627
void _send();
@@ -896,6 +916,14 @@ class AsyncWebServerRequest {
896916
return _paused;
897917
}
898918

919+
/**
920+
* @brief Best-effort fixed error head for the client before closing the connection (out of memory).
921+
* Marks the request as sent, writes a pre-assembled literal head (503 Service Unavailable and
922+
* "Connection: close" by default, so it cannot itself fail an allocation), then closes the connection.
923+
* Must only be used while nothing has been queued on the TCP connection yet.
924+
*/
925+
void sendRawResponse(const char *response);
926+
899927
/**
900928
* @brief Aborts the request and close the client (RST).
901929
* Mark the request as sent.
@@ -1738,7 +1766,7 @@ class AsyncWebServerResponse {
17381766
_assembleHead(buffer, version);
17391767
return buffer;
17401768
}
1741-
void _assembleHead(String &buffer, uint8_t version);
1769+
bool _assembleHead(String &buffer, uint8_t version);
17421770

17431771
virtual bool _started() const;
17441772
virtual bool _finished() const;
@@ -2046,7 +2074,7 @@ class AsyncWebServer : public AsyncMiddlewareChain {
20462074

20472075
void reset(); // remove all writers and handlers, with onNotFound/onFileUpload/onRequestBody
20482076

2049-
void _attachHandler(AsyncWebServerRequest *request);
2077+
bool _attachHandler(AsyncWebServerRequest *request);
20502078
void _rewriteRequest(AsyncWebServerRequest *request);
20512079
};
20522080

0 commit comments

Comments
 (0)