fix(http): prevent use-after-free from synchronous abort/close() - v2

src/AsyncEventSource.cpp

@@ -186,8 +186,11 @@ AsyncEventSourceClient::AsyncEventSourceClient(AsyncWebServerRequest *request, A
 
   _server->_addClient(this);
   _client->setNoDelay(true);
-  // delete AsyncWebServerRequest object (and bound response) since we have the ownership on client connection now
-  delete request;
+  // Release the AsyncWebServerRequest (and bound response) since we have the ownership on client connection now.
+  // Use release() instead of `delete request` because _this is now an owning
+  // shared_ptr (set in create()).  A manual delete would cause a double-free
+  // when the shared_ptr later drops its last reference.
+  request->release();
 }
 
 AsyncEventSourceClient::~AsyncEventSourceClient() {

src/AsyncWebSocket.cpp

@@ -1001,7 +1001,10 @@ AsyncWebSocketClient *AsyncWebSocket::_newClient(AsyncWebServerRequest *request)
   // we've just detached AsyncTCP client from AsyncWebServerRequest
   _handleEvent(&_clients.back(), WS_EVT_CONNECT, request, NULL, 0);
   // after user code completed CONNECT event callback we can delete req/response objects
-  delete request;
+  // Use release() instead of `delete request` because _this is now an owning
+  // shared_ptr (set in create()).  A manual delete would cause a double-free
+  // when the shared_ptr later drops its last reference.
+  request->release();
   return &_clients.back();
 }
 

src/ESPAsyncWebServer.h

@@ -508,13 +508,34 @@ class AsyncWebServerRequest {
 
   static bool _getEtag(File gzFile, char *eTag);
 
+  // Constructor is private to ensure factory is used to create shared_ptrs
+  AsyncWebServerRequest(AsyncWebServer *, AsyncClient *);
+
 public:
   File _tempFile;
   void *_tempObject;
 
-  AsyncWebServerRequest(AsyncWebServer *, AsyncClient *);
+  // Factory function
+  static std::shared_ptr<AsyncWebServerRequest> create(AsyncWebServer *server, AsyncClient *client) {
+    AsyncWebServerRequest *req = new (std::nothrow) AsyncWebServerRequest(server, client);
+    if (req) {
+      req->_this = std::shared_ptr<AsyncWebServerRequest>(req);  // store shared pointer to this request
+      return req->_this;
+    }
+    return {};  // empty shared_ptr
+  }
   ~AsyncWebServerRequest();
 
+  // Release the self-referential shared_ptr that owns this request.
+  // Used by WebSocket / EventSource handoff paths (which used to call `delete request`
+  // directly) to trigger destruction via the shared_ptr instead of a manual delete,
+  // avoiding a double-free now that _this is an owning shared_ptr.
+  // If a local shared_ptr hold (e.g. in _onData/_onPoll/_onAck) is on the stack,
+  // the request stays alive until that hold goes out of scope.
+  void release() {
+    _this.reset();
+  }
+
   AsyncClient *client() {
     return _client;
   }
@@ -1763,7 +1784,6 @@ class AsyncWebServer : public AsyncMiddlewareChain {
 
   void reset();  // remove all writers and handlers, with onNotFound/onFileUpload/onRequestBody
 
-  void _handleDisconnect(AsyncWebServerRequest *request);
   void _attachHandler(AsyncWebServerRequest *request);
   void _rewriteRequest(AsyncWebServerRequest *request);
 };

src/WebRequest.cpp

@@ -17,8 +17,6 @@ static inline bool isParamChar(char c) {
   return ((c) && ((c) != '{') && ((c) != '[') && ((c) != '&') && ((c) != '='));
 }
 
-static void doNotDelete(AsyncWebServerRequest *) {}
-
 using namespace asyncsrv;
 
 enum {
@@ -108,8 +106,6 @@ AsyncWebServerRequest::~AsyncWebServerRequest() {
     _response = nullptr;
   }
 
-  _this.reset();
-
   if (_tempObject != NULL) {
     free(_tempObject);
   }
@@ -124,6 +120,8 @@ AsyncWebServerRequest::~AsyncWebServerRequest() {
 }
 
 void AsyncWebServerRequest::_onData(void *buf, size_t len) {
+  std::shared_ptr<AsyncWebServerRequest> self = _this;  // Ensure we stay in scope over the function
+
   // SSL/TLS handshake detection
 #ifndef ASYNC_TCP_SSL_ENABLED
   if (_parseState == PARSE_REQ_START && len && ((uint8_t *)buf)[0] == 0x16) {  // 0x16 indicates a Handshake message (SSL/TLS).
@@ -240,13 +238,15 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
 }
 
 void AsyncWebServerRequest::_onPoll() {
+  std::shared_ptr<AsyncWebServerRequest> self = _this;  // Ensure we stay in scope over the function
   // os_printf("p\n");
   if (_response && _client && _client->canSend()) {
     _response->_ack(this, 0, 0);
   }
 }
 
 void AsyncWebServerRequest::_onAck(size_t len, uint32_t time) {
+  std::shared_ptr<AsyncWebServerRequest> self = _this;  // Ensure we stay in scope over the function
   // os_printf("a:%u:%u\n", len, time);
   if (!_response) {
     return;
@@ -269,6 +269,7 @@ void AsyncWebServerRequest::_onError(int8_t error) {
 }
 
 void AsyncWebServerRequest::_onTimeout(uint32_t time) {
+  std::shared_ptr<AsyncWebServerRequest> self = _this;  // Ensure we stay in scope over the function
   (void)time;
   // os_printf("TIMEOUT: %u, state: %s\n", time, _client->stateToString());
   _client->close();
@@ -280,10 +281,16 @@ void AsyncWebServerRequest::onDisconnect(ArDisconnectHandler fn) {
 
 void AsyncWebServerRequest::_onDisconnect() {
   async_ws_log_v("onDisconnect() cb for request: %s", _url.c_str());
+  // Take a local copy of _this before resetting the member: if _this is the
+  // last owning shared_ptr, resetting it would destruct *this while this
+  // member function is still executing.  The local copy keeps us alive until
+  // the function returns, then the shared_ptr destructor runs safely.
+  std::shared_ptr<AsyncWebServerRequest> self = _this;
   if (_onDisconnectfn) {
     _onDisconnectfn();
   }
-  _server->_handleDisconnect(this);
+  _this.reset();  // release shared pointer to this request, allowing for object destruction
+  // 'self' goes out of scope here — if it was the last owner, the request is deleted
 }
 
 void AsyncWebServerRequest::_addGetParams(const String &params) {
@@ -1054,9 +1061,6 @@ AsyncWebServerRequestPtr AsyncWebServerRequest::pause() {
     return _this;
   }
   client()->setRxTimeout(0);
-  // this shared ptr will hold the request pointer until it gets destroyed following a disconnect.
-  // this is just used as a holder providing weak observers, so the deleter is a no-op.
-  _this = std::shared_ptr<AsyncWebServerRequest>(this, doNotDelete);
   _paused = true;
   return _this;
 }
@@ -1065,7 +1069,6 @@ void AsyncWebServerRequest::abort() {
   if (!_sent) {
     _sent = true;
     _paused = false;
-    _this.reset();
     async_ws_log_v("Abort request: %s", _url.c_str());
     _client->abort();
   }
@@ -1327,7 +1330,9 @@ void AsyncWebServerRequest::requestAuthentication(AsyncAuthType method, const ch
         r->addHeader(T_WWW_AUTH, header.c_str());
       } else {
         async_ws_log_e("Failed to allocate");
+        delete r;
         abort();
+        return;
       }
 
       break;
@@ -1351,7 +1356,9 @@ void AsyncWebServerRequest::requestAuthentication(AsyncAuthType method, const ch
           r->addHeader(T_WWW_AUTH, header.c_str());
         } else {
           async_ws_log_e("Failed to allocate");
+          delete r;
           abort();
+          return;
         }
       }
       break;

src/WebServer.cpp

@@ -4,6 +4,7 @@
 #include "ESPAsyncWebServer.h"
 #include "WebHandlerImpl.h"
 
+#include <memory>
 #include <string>
 #include <utility>
 
@@ -47,7 +48,7 @@ AsyncWebServer::AsyncWebServer(uint16_t port) : _server(port) {
         return;
       }
       c->setRxTimeout(3);
-      AsyncWebServerRequest *r = new AsyncWebServerRequest((AsyncWebServer *)s, c);
+      std::shared_ptr<AsyncWebServerRequest> r = AsyncWebServerRequest::create(static_cast<AsyncWebServer *>(s), c);
       if (r == NULL) {
         c->abort();
         delete c;
@@ -127,10 +128,6 @@ void AsyncWebServer::beginSecure(const char *cert, const char *key, const char *
 }
 #endif
 
-void AsyncWebServer::_handleDisconnect(AsyncWebServerRequest *request) {
-  delete request;
-}
-
 void AsyncWebServer::_rewriteRequest(AsyncWebServerRequest *request) {
   // the last rewrite that matches the request will be used
   // we do not break the loop to allow for multiple rewrites to be applied and only the last one to be used (allows overriding)