From 2c99652f3546fec46876ced1e28a58fab36f3dcd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 30 Jan 2026 12:34:24 -0600 Subject: [PATCH 1/6] [http_request] Fix requests taking full timeout when response is already complete --- .../update/esp32_hosted_update.cpp | 10 ++++-- .../components/http_request/http_request.h | 36 +++++++++++++------ .../http_request/http_request_arduino.cpp | 18 ++++++++-- .../http_request/http_request_idf.cpp | 7 ++-- .../http_request/ota/ota_http_request.cpp | 4 ++- 5 files changed, 53 insertions(+), 22 deletions(-) diff --git a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp index ebcdd5f36e..404ca5edb6 100644 --- a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp +++ b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp @@ -196,11 +196,12 @@ bool Esp32HostedUpdate::fetch_manifest_() { int read_or_error = container->read(buf, sizeof(buf)); App.feed_wdt(); yield(); - auto result = http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout); + auto result = + http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == http_request::HttpReadLoopResult::RETRY) continue; if (result != http_request::HttpReadLoopResult::DATA) - break; // ERROR or TIMEOUT + break; // COMPLETE, ERROR, or TIMEOUT json_str.append(reinterpret_cast(buf), read_or_error); } container->end(); @@ -321,9 +322,12 @@ bool Esp32HostedUpdate::stream_firmware_to_coprocessor_() { App.feed_wdt(); yield(); - auto result = http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout); + auto result = + http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == http_request::HttpReadLoopResult::RETRY) continue; + if (result == http_request::HttpReadLoopResult::COMPLETE) + break; // All data received if (result != http_request::HttpReadLoopResult::DATA) { if (result == http_request::HttpReadLoopResult::TIMEOUT) { ESP_LOGE(TAG, "Timeout reading firmware data"); diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index fb39ca504c..e50b9df754 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -126,19 +126,21 @@ struct HttpReadResult { /// Result of processing a non-blocking read with timeout (for manual loops) enum class HttpReadLoopResult : uint8_t { - DATA, ///< Data was read, process it - RETRY, ///< No data yet, already delayed, caller should continue loop - ERROR, ///< Read error, caller should exit loop - TIMEOUT, ///< Timeout waiting for data, caller should exit loop + DATA, ///< Data was read, process it + COMPLETE, ///< All content has been read, caller should exit loop + RETRY, ///< No data yet, already delayed, caller should continue loop + ERROR, ///< Read error, caller should exit loop + TIMEOUT, ///< Timeout waiting for data, caller should exit loop }; /// Process a read result with timeout tracking and delay handling /// @param bytes_read_or_error Return value from read() - positive for bytes read, negative for error /// @param last_data_time Time of last successful read, updated when data received /// @param timeout_ms Maximum time to wait for data -/// @return DATA if data received, RETRY if should continue loop, ERROR/TIMEOUT if should exit -inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_t &last_data_time, - uint32_t timeout_ms) { +/// @param is_read_complete Whether all expected content has been read (from HttpContainer::is_read_complete()) +/// @return How the caller should proceed - see HttpReadLoopResult enum +inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_t &last_data_time, uint32_t timeout_ms, + bool is_read_complete) { if (bytes_read_or_error > 0) { last_data_time = millis(); return HttpReadLoopResult::DATA; @@ -146,7 +148,10 @@ inline HttpReadLoopResult http_read_loop_result(int bytes_read_or_error, uint32_ if (bytes_read_or_error < 0) { return HttpReadLoopResult::ERROR; } - // bytes_read_or_error == 0: no data available yet + // bytes_read_or_error == 0: either "no data yet" or "all content read" + if (is_read_complete) { + return HttpReadLoopResult::COMPLETE; + } if (millis() - last_data_time >= timeout_ms) { return HttpReadLoopResult::TIMEOUT; } @@ -197,6 +202,12 @@ class HttpContainer : public Parented { size_t get_bytes_read() const { return this->bytes_read_; } + /// Check if all expected content has been read (only valid for non-chunked responses) + /// For chunked responses (content_length == 0 on ESP-IDF, SIZE_MAX on Arduino), returns false + bool is_read_complete() const { + return this->content_length > 0 && this->content_length < SIZE_MAX && this->bytes_read_ >= this->content_length; + } + /** * @brief Get response headers. * @@ -231,9 +242,11 @@ inline HttpReadResult http_read_fully(HttpContainer *container, uint8_t *buffer, App.feed_wdt(); yield(); - auto result = http_read_loop_result(read_bytes_or_error, last_data_time, timeout_ms); + auto result = http_read_loop_result(read_bytes_or_error, last_data_time, timeout_ms, container->is_read_complete()); if (result == HttpReadLoopResult::RETRY) continue; + if (result == HttpReadLoopResult::COMPLETE) + break; // Server sent less data than requested, but transfer is complete if (result == HttpReadLoopResult::ERROR) return {HttpReadStatus::ERROR, read_bytes_or_error}; if (result == HttpReadLoopResult::TIMEOUT) @@ -393,11 +406,12 @@ template class HttpRequestSendAction : public Action { int read_or_error = container->read(buf + read_index, std::min(max_length - read_index, 512)); App.feed_wdt(); yield(); - auto result = http_read_loop_result(read_or_error, last_data_time, read_timeout); + auto result = + http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == HttpReadLoopResult::RETRY) continue; if (result != HttpReadLoopResult::DATA) - break; // ERROR or TIMEOUT + break; // COMPLETE, ERROR, or TIMEOUT read_index += read_or_error; } response_body.reserve(read_index); diff --git a/esphome/components/http_request/http_request_arduino.cpp b/esphome/components/http_request/http_request_arduino.cpp index 82538b2cb3..8f17622d2b 100644 --- a/esphome/components/http_request/http_request_arduino.cpp +++ b/esphome/components/http_request/http_request_arduino.cpp @@ -135,6 +135,18 @@ std::shared_ptr HttpRequestArduino::perform(const std::string &ur // When cast to size_t, -1 becomes SIZE_MAX (4294967295 on 32-bit). // The read() method handles this: bytes_read_ can never reach SIZE_MAX, so the // early return check (bytes_read_ >= content_length) will never trigger. + // + // TODO: Chunked transfer encoding is NOT properly supported on Arduino. + // The implementation in #7884 was incomplete - it only works correctly on ESP-IDF where + // esp_http_client_read() decodes chunks internally. On Arduino, using getStreamPtr() + // returns raw TCP data with chunk framing (e.g., "12a\r\n{json}\r\n0\r\n\r\n") instead + // of decoded content. This wasn't noticed because requests would complete and payloads + // were only examined on IDF. The long transfer times were also masked by the misleading + // "HTTP on Arduino version >= 3.1 is **very** slow" warning above. This causes two issues: + // 1. Response body is corrupted - contains chunk size headers mixed with data + // 2. Cannot detect end of transfer - connection stays open (keep-alive), causing timeout + // The proper fix would be to use getString() for chunked responses, which decodes chunks + // internally, but this buffers the entire response in memory. int content_length = container->client_.getSize(); ESP_LOGD(TAG, "Content-Length: %d", content_length); container->content_length = (size_t) content_length; @@ -178,9 +190,9 @@ int HttpContainerArduino::read(uint8_t *buf, size_t max_len) { if (bufsize == 0) { this->duration_ms += (millis() - start); - // Check if we've read all expected content (only valid when content_length is known and not SIZE_MAX) - // For chunked encoding (content_length == SIZE_MAX), we can't use this check - if (this->content_length > 0 && this->bytes_read_ >= this->content_length) { + // Check if we've read all expected content (non-chunked only) + // For chunked encoding (content_length == SIZE_MAX), is_read_complete() returns false + if (this->is_read_complete()) { return 0; // All content read successfully } // No data available - check if connection is still open diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 2b4dee953a..7924e06880 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -239,10 +239,9 @@ int HttpContainerIDF::read(uint8_t *buf, size_t max_len) { const uint32_t start = millis(); watchdog::WatchdogManager wdm(this->parent_->get_watchdog_timeout()); - // Check if we've already read all expected content - // Skip this check when content_length is 0 (chunked transfer encoding or unknown length) - // For chunked responses, esp_http_client_read() will return 0 when all data is received - if (this->content_length > 0 && this->bytes_read_ >= this->content_length) { + // Check if we've already read all expected content (non-chunked only) + // For chunked responses (content_length == 0), esp_http_client_read() handles EOF + if (this->is_read_complete()) { return 0; // All content read successfully } diff --git a/esphome/components/http_request/ota/ota_http_request.cpp b/esphome/components/http_request/ota/ota_http_request.cpp index 8a4b3684cf..058fe4c84f 100644 --- a/esphome/components/http_request/ota/ota_http_request.cpp +++ b/esphome/components/http_request/ota/ota_http_request.cpp @@ -130,9 +130,11 @@ uint8_t OtaHttpRequestComponent::do_ota_() { App.feed_wdt(); yield(); - auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout); + auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == HttpReadLoopResult::RETRY) continue; + if (result == HttpReadLoopResult::COMPLETE) + break; // All data received if (result != HttpReadLoopResult::DATA) { if (result == HttpReadLoopResult::TIMEOUT) { ESP_LOGE(TAG, "Timeout reading data"); From cb91215e038a894d6c987d5db94878f55f2d842f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 30 Jan 2026 13:52:27 -0600 Subject: [PATCH 2/6] 200,304,204,1xx --- esphome/components/http_request/http_request.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index e50b9df754..2f96aedfbb 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -205,6 +205,12 @@ class HttpContainer : public Parented { /// Check if all expected content has been read (only valid for non-chunked responses) /// For chunked responses (content_length == 0 on ESP-IDF, SIZE_MAX on Arduino), returns false bool is_read_complete() const { + // Per RFC 9112, these responses have no body: + // - 1xx (Informational), 204 No Content, 304 Not Modified + if ((this->status_code >= 100 && this->status_code < 200) || this->status_code == HTTP_STATUS_NO_CONTENT || + this->status_code == HTTP_STATUS_NOT_MODIFIED) { + return true; + } return this->content_length > 0 && this->content_length < SIZE_MAX && this->bytes_read_ >= this->content_length; } From f36f171647d1a8482ca25ffbce164f496c92233e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 30 Jan 2026 13:56:18 -0600 Subject: [PATCH 3/6] add comment --- esphome/components/http_request/http_request.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 2f96aedfbb..8269c0b28e 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -236,7 +236,7 @@ class HttpContainer : public Parented { /// @param total_size Total bytes to read /// @param chunk_size Maximum bytes per read call /// @param timeout_ms Read timeout in milliseconds -/// @return HttpReadResult with status and error_code on failure +/// @return HttpReadResult with status and error_code on failure; use container->get_bytes_read() for bytes read inline HttpReadResult http_read_fully(HttpContainer *container, uint8_t *buffer, size_t total_size, size_t chunk_size, uint32_t timeout_ms) { size_t read_index = 0; From 30c94c2c1160b176891109c9c3ef8ea005b124cb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 30 Jan 2026 13:58:23 -0600 Subject: [PATCH 4/6] address bot review comments --- .../components/esp32_hosted/update/esp32_hosted_update.cpp | 6 +++++- esphome/components/http_request/ota/ota_http_request.cpp | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp index 404ca5edb6..b812ff7dee 100644 --- a/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp +++ b/esphome/components/esp32_hosted/update/esp32_hosted_update.cpp @@ -200,6 +200,8 @@ bool Esp32HostedUpdate::fetch_manifest_() { http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == http_request::HttpReadLoopResult::RETRY) continue; + // Note: COMPLETE is currently unreachable since the loop condition checks bytes_read < content_length, + // but this is defensive code in case chunked transfer encoding support is added in the future. if (result != http_request::HttpReadLoopResult::DATA) break; // COMPLETE, ERROR, or TIMEOUT json_str.append(reinterpret_cast(buf), read_or_error); @@ -326,8 +328,10 @@ bool Esp32HostedUpdate::stream_firmware_to_coprocessor_() { http_request::http_read_loop_result(read_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == http_request::HttpReadLoopResult::RETRY) continue; + // Note: COMPLETE is currently unreachable since the loop condition checks bytes_read < content_length, + // but this is defensive code in case chunked transfer encoding support is added in the future. if (result == http_request::HttpReadLoopResult::COMPLETE) - break; // All data received + break; if (result != http_request::HttpReadLoopResult::DATA) { if (result == http_request::HttpReadLoopResult::TIMEOUT) { ESP_LOGE(TAG, "Timeout reading firmware data"); diff --git a/esphome/components/http_request/ota/ota_http_request.cpp b/esphome/components/http_request/ota/ota_http_request.cpp index 058fe4c84f..8f4ecfab2d 100644 --- a/esphome/components/http_request/ota/ota_http_request.cpp +++ b/esphome/components/http_request/ota/ota_http_request.cpp @@ -133,8 +133,10 @@ uint8_t OtaHttpRequestComponent::do_ota_() { auto result = http_read_loop_result(bufsize_or_error, last_data_time, read_timeout, container->is_read_complete()); if (result == HttpReadLoopResult::RETRY) continue; + // Note: COMPLETE is currently unreachable since the loop condition checks bytes_read < content_length, + // but this is defensive code in case chunked transfer encoding support is added for OTA in the future. if (result == HttpReadLoopResult::COMPLETE) - break; // All data received + break; if (result != HttpReadLoopResult::DATA) { if (result == HttpReadLoopResult::TIMEOUT) { ESP_LOGE(TAG, "Timeout reading data"); From c6a7616de08733f739a3fbca20c1366955227974 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 30 Jan 2026 14:16:16 -0600 Subject: [PATCH 5/6] be explict --- esphome/components/http_request/http_request.h | 15 ++++++++++----- .../http_request/http_request_arduino.cpp | 2 ++ .../components/http_request/http_request_idf.cpp | 2 ++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 8269c0b28e..228aa681ac 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -26,6 +26,7 @@ struct Header { enum HttpStatus { HTTP_STATUS_OK = 200, HTTP_STATUS_NO_CONTENT = 204, + HTTP_STATUS_RESET_CONTENT = 205, HTTP_STATUS_PARTIAL_CONTENT = 206, /* 3xx - Redirection */ @@ -199,19 +200,22 @@ class HttpContainer : public Parented { virtual void end() = 0; void set_secure(bool secure) { this->secure_ = secure; } + void set_chunked(bool chunked) { this->is_chunked_ = chunked; } size_t get_bytes_read() const { return this->bytes_read_; } - /// Check if all expected content has been read (only valid for non-chunked responses) - /// For chunked responses (content_length == 0 on ESP-IDF, SIZE_MAX on Arduino), returns false + /// Check if all expected content has been read + /// For chunked responses, returns false (completion detected via read() returning error/EOF) bool is_read_complete() const { // Per RFC 9112, these responses have no body: - // - 1xx (Informational), 204 No Content, 304 Not Modified + // - 1xx (Informational), 204 No Content, 205 Reset Content, 304 Not Modified if ((this->status_code >= 100 && this->status_code < 200) || this->status_code == HTTP_STATUS_NO_CONTENT || - this->status_code == HTTP_STATUS_NOT_MODIFIED) { + this->status_code == HTTP_STATUS_RESET_CONTENT || this->status_code == HTTP_STATUS_NOT_MODIFIED) { return true; } - return this->content_length > 0 && this->content_length < SIZE_MAX && this->bytes_read_ >= this->content_length; + // For non-chunked responses, complete when bytes_read >= content_length + // This handles both Content-Length: 0 and Content-Length: N cases + return !this->is_chunked_ && this->bytes_read_ >= this->content_length; } /** @@ -226,6 +230,7 @@ class HttpContainer : public Parented { protected: size_t bytes_read_{0}; bool secure_{false}; + bool is_chunked_{false}; ///< True if response uses chunked transfer encoding std::map> response_headers_{}; }; diff --git a/esphome/components/http_request/http_request_arduino.cpp b/esphome/components/http_request/http_request_arduino.cpp index 8f17622d2b..2f12b58766 100644 --- a/esphome/components/http_request/http_request_arduino.cpp +++ b/esphome/components/http_request/http_request_arduino.cpp @@ -150,6 +150,8 @@ std::shared_ptr HttpRequestArduino::perform(const std::string &ur int content_length = container->client_.getSize(); ESP_LOGD(TAG, "Content-Length: %d", content_length); container->content_length = (size_t) content_length; + // -1 (SIZE_MAX when cast to size_t) means chunked transfer encoding + container->set_chunked(content_length == -1); container->duration_ms = millis() - start; return container; diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 7924e06880..bd12b7d123 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -160,6 +160,7 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c // esp_http_client_fetch_headers() returns 0 for chunked transfer encoding (no Content-Length header). // The read() method handles content_length == 0 specially to support chunked responses. container->content_length = esp_http_client_fetch_headers(client); + container->set_chunked(esp_http_client_is_chunked_response(client)); container->feed_wdt(); container->status_code = esp_http_client_get_status_code(client); container->feed_wdt(); @@ -195,6 +196,7 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c container->feed_wdt(); container->content_length = esp_http_client_fetch_headers(client); + container->set_chunked(esp_http_client_is_chunked_response(client)); container->feed_wdt(); container->status_code = esp_http_client_get_status_code(client); container->feed_wdt(); From 60028036ed6d6bd5a49d069b0f8eebbaee118762 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 30 Jan 2026 14:28:53 -0600 Subject: [PATCH 6/6] bot nits --- esphome/components/http_request/http_request.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 228aa681ac..f3c99c6de4 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -165,9 +165,9 @@ class HttpRequestComponent; class HttpContainer : public Parented { public: virtual ~HttpContainer() = default; - size_t content_length; - int status_code; - uint32_t duration_ms; + size_t content_length{0}; + int status_code{-1}; ///< -1 indicates no response received yet + uint32_t duration_ms{0}; /** * @brief Read data from the HTTP response body. @@ -241,7 +241,7 @@ class HttpContainer : public Parented { /// @param total_size Total bytes to read /// @param chunk_size Maximum bytes per read call /// @param timeout_ms Read timeout in milliseconds -/// @return HttpReadResult with status and error_code on failure; use container->get_bytes_read() for bytes read +/// @return HttpReadResult with status and error_code on failure; use container->get_bytes_read() for total bytes read inline HttpReadResult http_read_fully(HttpContainer *container, uint8_t *buffer, size_t total_size, size_t chunk_size, uint32_t timeout_ms) { size_t read_index = 0;