refactor fix to be more robust and encapsulated

Author: ayavilevich

src/AsyncJson.cpp

@@ -5,6 +5,11 @@
 
 #if ASYNC_JSON_SUPPORT == 1
 
+typedef struct {
+  size_t length; // the size we can write into "content", not including null termination
+  uint8_t content[1]; // this will be of size "content-length" + 1 byte to guarantee that the content is null terminated. null termination is needed for ArduinoJson 5
+} AsyncJsonResponseBuffer;
+
 #if ARDUINOJSON_VERSION_MAJOR == 5
 AsyncJsonResponse::AsyncJsonResponse(bool isArray) : _isValid{false} {
   _code = 200;
@@ -119,40 +124,42 @@ void AsyncCallbackJsonWebHandler::handleRequest(AsyncWebServerRequest *request)
       return;
     }
     // this is not a GET
-    // check if body is too large, if it is, don't parse
+    // check if json body is too large, if it is, don't deserialize
     if (request->contentLength() > _maxContentLength)
     {
       request->send(413);
       return;
     }
 
     // try to parse body as JSON
-    if (request->_tempObject != NULL)
+    if (request->_tempObject != NULL) // see if we succeeded allocating a buffer earlier
     {
-      size_t dataSize = min(request->contentLength(), request->_tempSize); // smaller value of contentLength or the size of the buffer. normally those should match.
+      AsyncJsonResponseBuffer * buffer = (AsyncJsonResponseBuffer*)request->_tempObject;
 #if ARDUINOJSON_VERSION_MAJOR == 5
       DynamicJsonBuffer jsonBuffer;
-      uint8_t * p = (uint8_t *)(request->_tempObject);
-      p[dataSize] = '\0'; // null terminate, assume we allocated one extra char
+      buffer->content[buffer->length] = '\0'; // null terminate, assume we allocated one extra char
       // parse can only get null terminated strings as parameters
-      JsonVariant json = jsonBuffer.parse(p);
+      JsonVariant json = jsonBuffer.parse(buffer->content);
       if (json.success()) {
 #elif ARDUINOJSON_VERSION_MAJOR == 6
-      DynamicJsonDocument jsonBuffer(this->maxJsonBufferSize);
-      DeserializationError error = deserializeJson(jsonBuffer, (uint8_t *)(request->_tempObject), dataSize);
+      DynamicJsonDocument jsonBuffer(this->maxJsonBufferSize); // content with length > this->maxJsonBufferSize might not get deserialized
+      DeserializationError error = deserializeJson(jsonBuffer, buffer->content, buffer->length);
       if (!error) {
         JsonVariant json = jsonBuffer.as<JsonVariant>();
 #else
       JsonDocument jsonBuffer;
       // deserializeJson expects a null terminated string or a pointer plus length
-      DeserializationError error = deserializeJson(jsonBuffer, (uint8_t *)(request->_tempObject), dataSize);
+      DeserializationError error = deserializeJson(jsonBuffer, buffer->content, buffer->length);
       if (!error) {
         JsonVariant json = jsonBuffer.as<JsonVariant>();
 #endif
 
         _onRequest(request, json);
         return;
       }
+      // free buffer, we are done with it, so release memory ASAP
+      free(request->_tempObject);
+      request->_tempObject = NULL;
     }
     // there is no body, no buffer or we had an error parsing the body
     request->send(400);
@@ -163,30 +170,46 @@ void AsyncCallbackJsonWebHandler::handleRequest(AsyncWebServerRequest *request)
 
 void AsyncCallbackJsonWebHandler::handleBody(AsyncWebServerRequest *request, uint8_t *data, size_t len, size_t index, size_t total) {
   if (_onRequest) {
-    if (total > 0 && request->_tempObject == NULL && total < _maxContentLength) { // if request content length is valid size and we have no content buffer yet
-      request->_tempObject = malloc(total + 1); // allocate one additional byte so we can null terminate this buffer (needed for ArduinoJson 5)
+    if (index == 0) // on first piece
+    {
+      // check nobody has already allocated the buffer
+      if (request->_tempObject != NULL)
+      {
+#ifdef ESP32
+        log_e("Temp object already in use");
+#endif
+        return; // do nothing else here, handleRequest will return a HTTP error
+      }
+      // check total size is valid
+      if (total >= _maxContentLength)
+      {
+        return; // do nothing else here, handleRequest will return a HTTP error
+      }
+      // allocate buffer
+      request->_tempObject = calloc(1, sizeof(AsyncJsonResponseBuffer) + total); // normally _tempObject will be "free"ed  by the destructor of the request, but can release earlier if desired.
       if (request->_tempObject == NULL) { // if allocation failed
 #ifdef ESP32
         log_e("Failed to allocate");
 #endif
-        request->abort();
-        return;
+        return; // do nothing else here, handleRequest will return a HTTP error
       }
-      request->_tempSize = total; // store the size of allocation we made into _tempSize
+      ((AsyncJsonResponseBuffer*)request->_tempObject)->length = total; // store the size of allocation we made into _tempObject
     }
+
+    // add data to the buffer if the buffer exists
     if (request->_tempObject != NULL) {
+      AsyncJsonResponseBuffer * buffer = (AsyncJsonResponseBuffer*)request->_tempObject;
       // check if the buffer is the right size so we don't write out of bounds
-      if (request->_tempSize >= total)
+      if (buffer->length >= total && buffer->length >= index + len)
       {
-        memcpy((uint8_t *)(request->_tempObject) + index, data, len);
+        memcpy(buffer->content + index, data, len);
       }
       else
       {
 #ifdef ESP32
         log_e("Bad size of temp buffer");
 #endif
-        request->abort();
-        return;
+        return; // do nothing else here
       }
     }
   }

src/ESPAsyncWebServer.h

@@ -261,7 +261,6 @@ class AsyncWebServerRequest {
 public:
   File _tempFile;
   void *_tempObject;
-  size_t _tempSize;
 
   AsyncWebServerRequest(AsyncWebServer *, AsyncClient *);
   ~AsyncWebServerRequest();

src/WebRequest.cpp

@@ -25,7 +25,7 @@ AsyncWebServerRequest::AsyncWebServerRequest(AsyncWebServer *s, AsyncClient *c)
   : _client(c), _server(s), _handler(NULL), _response(NULL), _temp(), _parseState(PARSE_REQ_START), _version(0), _method(HTTP_ANY), _url(), _host(),
     _contentType(), _boundary(), _authorization(), _reqconntype(RCT_HTTP), _authMethod(AsyncAuthType::AUTH_NONE), _isMultipart(false), _isPlainPost(false),
     _expectingContinue(false), _contentLength(0), _parsedLength(0), _multiParseState(0), _boundaryPosition(0), _itemStartIndex(0), _itemSize(0), _itemName(),
-    _itemFilename(), _itemType(), _itemValue(), _itemBuffer(0), _itemBufferIndex(0), _itemIsFile(false), _tempObject(NULL), _tempSize(0) {
+    _itemFilename(), _itemType(), _itemValue(), _itemBuffer(0), _itemBufferIndex(0), _itemIsFile(false), _tempObject(NULL) {
   c->onError(
     [](void *r, AsyncClient *c, int8_t error) {
       (void)c;