diff --git a/CHANGELOG_LATEST.md b/CHANGELOG_LATEST.md index 579593296..5a45c1fd6 100644 --- a/CHANGELOG_LATEST.md +++ b/CHANGELOG_LATEST.md @@ -33,6 +33,8 @@ This release is based on the latest Espressif/Arduino core version 3. It brings - heat-pump operating state (`hpoperatingstate`: heating/off/cooling) is published for every heating circuit. On BC400/UI800 it was only filled for hc1, because hc2 never answers telegram 0x472 while the state is still broadcast in 0x2A6 offset 20 [#3097](https://github.com/emsesp/EMS-ESP32/issues/3097) - restart from the WebUI (Restart button, firmware/settings upload) only happened while the browser tab stayed open. If the tab was closed the restart was silently dropped after 5 minutes, leaving a new firmware uninstalled. EMS-ESP now restarts on its own if the WebUI hasn't picked up the pending restart within 10 seconds +- a firmware upload from a script or `curl` was accepted (`md5_ok`) but EMS-ESP didn't restart into it: the client closing the connection after the upload was treated as an aborted upload, which cancelled the pending restart +- EMS-ESP crashed and restarted when an upload was rejected, for example an empty or invalid MD5 file. The error code was kept in the request's `_tempObject`, which the web server frees as heap memory when the request ends - shunting yard show json - `system/sendmail` called from a Command or the Scheduler computed its value twice, which stripped the quotes from literal text and then failed on characters like `!`. Also shunting yard treated a `?` inside a quoted string as a ternary, so text like `"too hot?"` returned nothing - memory leak when using different timezones for ems-esp and thermostat[#3184](https://github.com/emsesp/EMS-ESP32/issues/3184) diff --git a/src/ESP32React/UploadFileService.cpp b/src/ESP32React/UploadFileService.cpp index bb4d1acb9..9f5fed1f1 100644 --- a/src/ESP32React/UploadFileService.cpp +++ b/src/ESP32React/UploadFileService.cpp @@ -155,7 +155,7 @@ void UploadFileService::handleUpload(AsyncWebServerRequest * request, const Stri void UploadFileService::uploadComplete(AsyncWebServerRequest * request) { if (request->_tempObject) { - const int code = static_cast(reinterpret_cast(request->_tempObject)); + const int code = *static_cast(request->_tempObject); if (!_response_sent && code >= 400) { request->send(request->beginResponse(code)); _response_sent = true; @@ -220,7 +220,10 @@ void UploadFileService::uploadComplete(AsyncWebServerRequest * request) { AsyncWebServerResponse * response = request->beginResponse(200); request->send(response); } - + // the upload is done: clear the flags so handleDisconnect() doesn't treat the client closing the + // connection as an aborted upload and cancel the pending restart + _is_firmware = false; + _is_filesystem = false; emsesp::EMSESP::system_.systemStatus( emsesp::SYSTEM_STATUS::SYSTEM_STATUS_PENDING_RESTART); // will be handled by the main loop. We use pending for the Web's SystemMonitor return; @@ -244,23 +247,19 @@ void UploadFileService::handleError(AsyncWebServerRequest * request, int code) { if (request->_tempObject) { return; } - - request->_tempObject = reinterpret_cast(static_cast(code)); // stop later chunks from retrying the write + // stop later chunks from retrying the write. The request destructor frees _tempObject, so it must be heap memory + auto * error = static_cast(malloc(sizeof(int))); + if (error) { + *error = code; + } + request->_tempObject = error; emsesp::EMSESP::logger().info("Upload error: %d", code); emsesp::EMSESP::system_.uart_init(); // re-enable UART _ota.abort(); - // 406: the client sent a file we won't accept. Close now so the browser fails fast. - // Other codes wait for uploadComplete() to send, so we don't destroy the request mid-stream. - if (code == 406) { - request->send(request->beginResponse(code)); - _response_sent = true; - request->client()->close(); - _is_firmware = false; - _is_filesystem = false; - _is_recovery = false; - } + // uploadComplete() sends the error code. The web server only writes a response once the whole body has + // arrived, so closing the connection here to fail fast would lose the response. // if we aborted a filesystem upload, remount LittleFS so the device keeps working if (_is_filesystem) {