Cleanup proposal fix for

#183

Author: mathieucarbou

examples/Json/Json.ino

@@ -63,14 +63,27 @@ void setup() {
     JsonObject root = doc.to<JsonObject>();
     root["foo"] = "bar";
     serializeJson(root, *response);
+    Serial.println();
     request->send(response);
   });
 
   // curl -v -X POST -H 'Content-Type: application/json' -d '{"name":"You"}' http://192.168.4.1/json2
   // curl -v -X PUT -H 'Content-Type: application/json' -d '{"name":"You"}' http://192.168.4.1/json2
+  //
+  // edge cases:
+  //
+  // curl -v -X POST -H "Content-Type: application/json" -d "1234" -H "Content-Length: 5" http://192.168.4.1/json2 => rx timeout
+  // curl -v -X POST -H "Content-Type: application/json" -d "1234" -H "Content-Length: 2" http://192.168.4.1/json2 => 12
+  // curl -v -X POST -H "Content-Type: application/json" -d "1234" -H "Content-Length: 4" http://192.168.4.1/json2 => 1234
+  // curl -v -X POST -H "Content-Type: application/json" -d "1234" -H "Content-Length: 10" http://192.168.4.1/json2 => rx timeout
+  // curl -v -X POST -H "Content-Type: application/json" -d "12345678" -H "Content-Length: 8" http://192.168.4.1/json2 => 12345678
+  // curl -v -X POST -H "Content-Type: application/json" -d "123456789" -H "Content-Length: 8" http://192.168.4.1/json2 => 12345678
+  // curl -v -X POST -H "Content-Type: application/json" -d "123456789" -H "Content-Length: 9" http://192.168.4.1/json2 => 413: Content length exceeds maximum allowed
+  handler->setMaxContentLength(8);
   handler->setMethod(HTTP_POST | HTTP_PUT);
   handler->onRequest([](AsyncWebServerRequest *request, JsonVariant &json) {
     serializeJson(json, Serial);
+    Serial.println();
     AsyncJsonResponse *response = new AsyncJsonResponse();
     JsonObject root = response->getRoot().to<JsonObject>();
     root["hello"] = json.as<JsonObject>()["name"];

src/AsyncJson.cpp

@@ -5,12 +5,6 @@
 
 #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,95 +113,77 @@ bool AsyncCallbackJsonWebHandler::canHandle(AsyncWebServerRequest *request) cons
 
 void AsyncCallbackJsonWebHandler::handleRequest(AsyncWebServerRequest *request) {
   if (_onRequest) {
+    // GET request:
     if (request->method() == HTTP_GET) {
       JsonVariant json;
       _onRequest(request, json);
       return;
     }
-    // this is not a GET
-    // check if json body is too large, if it is, don't deserialize
+
+    // POST / PUT / ... requests:
+    // check if JSON body is too large, if it is, don't deserialize
     if (request->contentLength() > _maxContentLength) {
+#ifdef ESP32
+      log_e("Content length exceeds maximum allowed");
+#endif
       request->send(413);
       return;
     }
 
-    // try to parse body as JSON
-    if (request->_tempObject != NULL)  // see if we succeeded allocating a buffer earlier
-    {
-      AsyncJsonResponseBuffer *buffer = (AsyncJsonResponseBuffer *)request->_tempObject;
+    if (request->_tempObject == NULL) {
+      // there is no body
+      request->send(400);
+      return;
+    }
+
 #if ARDUINOJSON_VERSION_MAJOR == 5
-      DynamicJsonBuffer jsonBuffer;
-      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(buffer->content);
-      if (json.success()) {
+    DynamicJsonBuffer jsonBuffer;
+    JsonVariant json = jsonBuffer.parse((const char *)request->_tempObject);
+    if (json.success()) {
 #elif ARDUINOJSON_VERSION_MAJOR == 6
-      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>();
+    DynamicJsonDocument jsonBuffer(this->maxJsonBufferSize);
+    DeserializationError error = deserializeJson(jsonBuffer, (const char *)request->_tempObject);
+    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, buffer->content, buffer->length);
-      if (!error) {
-        JsonVariant json = jsonBuffer.as<JsonVariant>();
+    JsonDocument jsonBuffer;
+    DeserializationError error = deserializeJson(jsonBuffer, (const char *)request->_tempObject);
+    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;
+      _onRequest(request, json);
+    } else {
+      // error parsing the body
+      request->send(400);
     }
-    // there is no body, no buffer or we had an error parsing the body
-    request->send(400);
-  } else {  // if no _onRequest
-    request->send(500);
   }
 }
 
 void AsyncCallbackJsonWebHandler::handleBody(AsyncWebServerRequest *request, uint8_t *data, size_t len, size_t index, size_t total) {
   if (_onRequest) {
-    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
+    // ignore callback if size is larger than maxContentLength
+    if (total > _maxContentLength) {
+      return;
+    }
+
+    if (index == 0) {
+      // this check allows request->_tempObject to be initialized from a middleware
+      if (request->_tempObject == NULL) {
+        request->_tempObject = calloc(total + 1, sizeof(uint8_t));  // null-terminated string
+        if (request->_tempObject == NULL) {
 #ifdef ESP32
-        log_e("Failed to allocate");
+          log_e("Failed to allocate");
 #endif
-        return;  // do nothing else here, handleRequest will return a HTTP error
+          request->abort();
+          return;
+        }
       }
-      ((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 (buffer->length >= total && buffer->length >= index + len) {
-        memcpy(buffer->content + index, data, len);
-      } else {
-#ifdef ESP32
-        log_e("Bad size of temp buffer");
-#endif
-        return;  // do nothing else here
-      }
+      uint8_t *buffer = (uint8_t *)request->_tempObject;
+      memcpy(buffer + index, data, len);
     }
   }
 }