From b3ef5cf2f3cefbe2da7b5b2d8eb36779292e915b Mon Sep 17 00:00:00 2001 From: Remco van Essen Date: Sat, 17 Oct 2026 01:58:12 +0200 Subject: [PATCH] [online_image] Report an error when a download can never complete (#20594) Co-authored-by: J. Nick Koston Co-authored-by: guillempages Co-authored-by: J. Nick Koston --- .../components/online_image/online_image.cpp | 90 ++++-- .../components/online_image/online_image.h | 2 + .../components/runtime_image/bmp_decoder.h | 9 +- .../components/runtime_image/image_decoder.h | 3 + .../components/runtime_image/qoi_decoder.h | 9 +- .../runtime_image/runtime_image.cpp | 2 + .../components/runtime_image/runtime_image.h | 3 + tests/components/online_image/__init__.py | 14 + .../online_image/test_download_end.cpp | 289 ++++++++++++++++++ .../online_image_stalled_download.yaml | 54 ++++ .../test_online_image_stalled_download.py | 121 ++++++++ 11 files changed, 566 insertions(+), 30 deletions(-) create mode 100644 tests/components/online_image/__init__.py create mode 100644 tests/components/online_image/test_download_end.cpp create mode 100644 tests/integration/fixtures/online_image_stalled_download.yaml create mode 100644 tests/integration/test_online_image_stalled_download.py diff --git a/esphome/components/online_image/online_image.cpp b/esphome/components/online_image/online_image.cpp index 4497d5cad12..59c1db38a58 100644 --- a/esphome/components/online_image/online_image.cpp +++ b/esphome/components/online_image/online_image.cpp @@ -1,5 +1,6 @@ #include "online_image.h" #include "esphome/components/runtime_image/image_decoder.h" +#include "esphome/core/application.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" #include @@ -51,6 +52,13 @@ void OnlineImage::update() { return; } + // A failed allocation at boot, or while growing the buffer for a JPEG, leaves no download buffer + if (this->download_buffer_.size() == 0 && this->download_buffer_.resize(this->download_buffer_initial_size_) == 0) { + // resize() logged the failed allocation + this->download_error_callback_.call(); + return; + } + ESP_LOGD(TAG, "Updating image from %s", this->url_.c_str()); std::vector headers; @@ -133,12 +141,17 @@ void OnlineImage::update() { } // JPEG requires the complete image in the download buffer before decoding - if (format == runtime_image::JPEG && total_size > this->download_buffer_.size()) { - this->download_buffer_.resize(total_size); + if (format == runtime_image::JPEG && total_size > this->download_buffer_.size() && + this->download_buffer_.resize(total_size) == 0) { + // resize() logged the failed allocation; the next update() allocates the configured buffer again + this->end_connection_(); + this->download_error_callback_.call(); + return; } ESP_LOGI(TAG, "Downloading image (Size: %zu)", total_size); this->start_time_ = millis(); + this->last_data_time_ = this->start_time_; this->enable_loop(); } @@ -161,18 +174,7 @@ void OnlineImage::loop() { if (this->is_decode_finished() || (this->downloader_->content_length > 0 && this->downloader_->get_bytes_read() >= this->downloader_->content_length && this->download_buffer_.unread() == 0)) { - // Finalize decoding - this->end_decode(); - - ESP_LOGD(TAG, "Image fully downloaded, %zu bytes in %" PRIu32 " ms", this->downloader_->get_bytes_read(), - millis() - this->start_time_); - - // Save caching headers - this->etag_ = this->downloader_->get_response_header(ETAG_HEADER_NAME); - this->last_modified_ = this->downloader_->get_response_header(LAST_MODIFIED_HEADER_NAME); - - this->download_finished_callback_.call(false); - this->end_connection_(); + this->finish_download_(); return; } @@ -184,6 +186,7 @@ void OnlineImage::loop() { auto len = this->downloader_->read(this->download_buffer_.append(), available); if (len > 0) { + this->last_data_time_ = App.get_loop_component_start_time(); this->download_buffer_.write(len); // Feed data to decoder @@ -204,25 +207,76 @@ void OnlineImage::loop() { this->end_connection_(); this->download_error_callback_.call(); return; + } else if (this->downloader_->is_read_complete()) { + // The response has ended, so data left in the buffer can never be completed + size_t unread = this->download_buffer_.unread(); + if (unread == 0 && this->downloader_->get_bytes_read() > 0) { + // A decoder that counts pixels knows the image was cut off; PNG cannot tell, so it passes + if (this->is_decode_finished() || !this->decoder_knows_end()) { + this->finish_download_(); + return; + } + ESP_LOGE(TAG, "Download ended before the image was complete"); + } else if (unread == 0) { + ESP_LOGE(TAG, "Server sent an empty response"); + } else { + ESP_LOGE(TAG, "Download ended with %zu bytes the decoder could not use", unread); + } + this->end_connection_(); + this->download_error_callback_.call(); + return; + } else if (App.get_loop_component_start_time() - this->last_data_time_ >= this->parent_->get_timeout()) { + // ESP-IDF keeps returning 0 after the server closes the connection early + ESP_LOGE(TAG, "No data received for %" PRIu32 " ms", this->parent_->get_timeout()); + this->end_connection_(); + this->download_error_callback_.call(); + return; } } else { // Buffer is full, need to decode some data first auto consumed = this->feed_data(this->download_buffer_.data(), this->download_buffer_.unread()); if (consumed > 0) { + this->last_data_time_ = App.get_loop_component_start_time(); this->download_buffer_.read(consumed); } else if (consumed < 0) { ESP_LOGE(TAG, "Decode error with full buffer: %d", consumed); this->end_connection_(); this->download_error_callback_.call(); return; - } else { - // Decoder can't process more data, might need complete image - // This is normal for JPEG which needs complete data - ESP_LOGV(TAG, "Decoder waiting for more data"); + } else if (App.get_loop_component_start_time() - this->last_data_time_ >= this->parent_->get_timeout()) { + // The decoder needs more data than the buffer can hold, so it can never make progress. The + // no-data timeout gives a decoder that only had to wait the chance to show otherwise. + ESP_LOGE(TAG, "Image does not fit in the %zu byte download buffer; increase buffer_size", + this->download_buffer_.size()); + this->end_connection_(); + this->download_error_callback_.call(); + return; } } } +void OnlineImage::finish_download_() { + // Must be read before end_decode() ends the decoding session + bool decoder_finished = this->is_decode_finished(); + this->end_decode(); + + ESP_LOGD(TAG, "Image fully downloaded, %zu bytes in %" PRIu32 " ms", this->downloader_->get_bytes_read(), + millis() - this->start_time_); + + // Only cache an image the decoder confirmed as complete; otherwise a cut-off image would be kept by 304 responses + if (decoder_finished) { + this->etag_ = this->downloader_->get_response_header(ETAG_HEADER_NAME); + this->last_modified_ = this->downloader_->get_response_header(LAST_MODIFIED_HEADER_NAME); + } else { + ESP_LOGW(TAG, "Decoder could not confirm the image is complete; not caching it"); + this->etag_.clear(); + this->last_modified_.clear(); + } + + this->download_finished_callback_.call(false); + this->end_connection_(); +} + void OnlineImage::end_connection_() { // Abort any in-progress decode; the decoder object is kept warm for the next decode. // Use RuntimeImage::release() directly to avoid recursion with OnlineImage::release(). diff --git a/esphome/components/online_image/online_image.h b/esphome/components/online_image/online_image.h index 8b3485b8f74..7397c6c5c32 100644 --- a/esphome/components/online_image/online_image.h +++ b/esphome/components/online_image/online_image.h @@ -80,6 +80,7 @@ class OnlineImage final : public PollingComponent, protected: bool validate_url_(const std::string &url); void end_connection_(); + void finish_download_(); CallbackManager download_finished_callback_{}; CallbackManager download_error_callback_{}; @@ -107,6 +108,7 @@ class OnlineImage final : public PollingComponent, std::string last_modified_; uint32_t start_time_{0}; + uint32_t last_data_time_{0}; }; } // namespace esphome::online_image diff --git a/esphome/components/runtime_image/bmp_decoder.h b/esphome/components/runtime_image/bmp_decoder.h index 01acc41f91f..cea7f11c784 100644 --- a/esphome/components/runtime_image/bmp_decoder.h +++ b/esphome/components/runtime_image/bmp_decoder.h @@ -26,13 +26,10 @@ class BmpDecoder : public ImageDecoder { void reset() override; int HOT decode(uint8_t *buffer, size_t size) override; + /// The header gives the pixel count, so the end is known once it has been parsed + bool knows_end() const override { return this->bits_per_pixel_ != 0; } bool is_finished() const override { - if (this->bits_per_pixel_ == 0) { - // header not yet received, so dimensions not yet determined - return false; - } - // BMP is finished when we've decoded all pixel data - return this->paint_index_ >= static_cast(this->width_ * this->height_); + return this->knows_end() && this->paint_index_ >= static_cast(this->width_ * this->height_); } protected: diff --git a/esphome/components/runtime_image/image_decoder.h b/esphome/components/runtime_image/image_decoder.h index 8e0399ac354..6472638261e 100644 --- a/esphome/components/runtime_image/image_decoder.h +++ b/esphome/components/runtime_image/image_decoder.h @@ -126,6 +126,9 @@ class ImageDecoder { return false; } + /// Whether is_finished() can tell a complete image from a cut-off one + virtual bool knows_end() const { return this->expected_size_ > 0; } + protected: /// The image's pixel buffer, for decoders that write their output straight into it uint8_t *image_buffer_() const; diff --git a/esphome/components/runtime_image/qoi_decoder.h b/esphome/components/runtime_image/qoi_decoder.h index c44afd83cf2..e66b10027d8 100644 --- a/esphome/components/runtime_image/qoi_decoder.h +++ b/esphome/components/runtime_image/qoi_decoder.h @@ -25,13 +25,10 @@ class QoiDecoder : public ImageDecoder { void reset() override; int HOT decode(uint8_t *buffer, size_t size) override; + /// The header gives the pixel count, so the end is known once it has been parsed + bool knows_end() const override { return this->bits_per_pixel_ != 0; } bool is_finished() const override { - if (this->bits_per_pixel_ == 0) { - // header not yet received, so dimensions not yet determined - return false; - } - // QOI is finished when we've decoded all pixel data - return this->paint_index_ >= static_cast(this->width_ * this->height_); + return this->knows_end() && this->paint_index_ >= static_cast(this->width_ * this->height_); } protected: diff --git a/esphome/components/runtime_image/runtime_image.cpp b/esphome/components/runtime_image/runtime_image.cpp index 14e0d5665a6..1c6a2bc6002 100644 --- a/esphome/components/runtime_image/runtime_image.cpp +++ b/esphome/components/runtime_image/runtime_image.cpp @@ -262,6 +262,8 @@ bool RuntimeImage::end_decode() { bool RuntimeImage::is_decode_finished() const { return this->is_decoding() && this->decoder_->is_finished(); } +bool RuntimeImage::decoder_knows_end() const { return this->is_decoding() && this->decoder_->knows_end(); } + void RuntimeImage::release() { this->release_buffer_(); // End any active decode session; decoders free the format-specific working buffers diff --git a/esphome/components/runtime_image/runtime_image.h b/esphome/components/runtime_image/runtime_image.h index 2a3ab40a5c1..4dc60213649 100644 --- a/esphome/components/runtime_image/runtime_image.h +++ b/esphome/components/runtime_image/runtime_image.h @@ -96,6 +96,9 @@ class RuntimeImage : public image::Image { */ bool is_decode_finished() const; + /// True while decoding and the decoder can tell a complete image from a cut-off one + bool decoder_knows_end() const; + /** * @brief Check if an image is currently loaded. */ diff --git a/tests/components/online_image/__init__.py b/tests/components/online_image/__init__.py new file mode 100644 index 00000000000..8635a2ba4ee --- /dev/null +++ b/tests/components/online_image/__init__.py @@ -0,0 +1,14 @@ +from esphome.components.runtime_image import get_format +from esphome.types import ConfigType +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # The real to_code needs an image config. The unit tests decode BMP and PNG, and use + # JPEG for the download buffer resize, so compile in those three decoders. + async def to_code_testing(config: ConfigType) -> None: + get_format("BMP").actions() + get_format("JPEG").actions() + get_format("PNG").actions() + + manifest.to_code = to_code_testing diff --git a/tests/components/online_image/test_download_end.cpp b/tests/components/online_image/test_download_end.cpp new file mode 100644 index 00000000000..d844cf845e8 --- /dev/null +++ b/tests/components/online_image/test_download_end.cpp @@ -0,0 +1,289 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "esphome/components/http_request/http_request.h" +#include "esphome/components/online_image/online_image.h" +#include "esphome/core/application.h" +#include "esphome/core/hal.h" + +namespace esphome::online_image::testing { + +// Serves a fixed body, handing out at most `available` bytes of it. +class FakeContainer : public http_request::HttpContainer { + public: + FakeContainer(std::vector body, size_t available, bool chunked) + : body_(std::move(body)), available_(std::min(available, this->body_.size())) { + this->status_code = 200; + this->content_length = chunked ? 0 : this->body_.size(); + this->set_chunked(chunked); + } + + int read(uint8_t *buf, size_t max_len) override { + size_t len = std::min(max_len, this->available_ - this->bytes_read_); + std::memcpy(buf, this->body_.data() + this->bytes_read_, len); + this->bytes_read_ += len; + return static_cast(len); + } + void end() override {} + + // Like ESP-IDF: a chunked response is complete once its last chunk has been read + bool is_read_complete() const override { + if (this->is_chunked_) + return this->bytes_read_ >= this->body_.size(); + return HttpContainer::is_read_complete(); + } + + void set_content_length(size_t length) { this->content_length = length; } + void add_response_header(const std::string &name, const std::string &value) { + this->response_headers_.push_back({name, value}); + } + + protected: + std::vector body_; + size_t available_; +}; + +class FakeHttpRequest : public http_request::HttpRequestComponent { + public: + std::shared_ptr next; + std::vector sent_headers; + + bool sent_header(const std::string &name) const { + return std::any_of(this->sent_headers.begin(), this->sent_headers.end(), + [&name](const http_request::Header &header) { return header.name == name; }); + } + + protected: + std::shared_ptr perform(const char *url, const char *method, const std::string &body, + const std::vector &request_headers, + const std::vector &collect_headers, + uint32_t timeout) override { + this->sent_headers = request_headers; + return std::move(this->next); + } +}; + +// A 24-bit BMP, 2x2 pixels: 54 header bytes, then two rows of 6 pixel bytes and 2 padding bytes. +static constexpr size_t BMP_HEADER_SIZE = 54; +static constexpr size_t BMP_ROW_SIZE = 8; + +static std::vector make_bmp() { + std::vector bmp(BMP_HEADER_SIZE + 2 * BMP_ROW_SIZE, 0); + auto put32 = [&bmp](size_t offset, uint32_t value) { + for (size_t i = 0; i < 4; i++) + bmp[offset + i] = (value >> (8 * i)) & 0xFF; + }; + bmp[0] = 'B'; + bmp[1] = 'M'; + put32(2, bmp.size()); + put32(10, BMP_HEADER_SIZE); + put32(14, 40); // DIB header size + put32(18, 2); // width + put32(22, 2); // height + bmp[26] = 1; // color planes + bmp[28] = 24; // bits per pixel + return bmp; +} + +// An 8x8, 8-bit BMP whose 256-color table puts the pixels at offset 1078, past a 256 byte buffer. +static std::vector make_palette_bmp() { + constexpr size_t data_offset = 14 + 40 + 256 * 4; + std::vector bmp(data_offset + 8 * 8, 0); + auto put32 = [&bmp](size_t offset, uint32_t value) { + for (size_t i = 0; i < 4; i++) + bmp[offset + i] = (value >> (8 * i)) & 0xFF; + }; + bmp[0] = 'B'; + bmp[1] = 'M'; + put32(2, bmp.size()); + put32(10, data_offset); + put32(14, 40); // DIB header size + put32(18, 8); // width + put32(22, 8); // height + bmp[26] = 1; // color planes + bmp[28] = 8; // bits per pixel + put32(34, 64); // image size + put32(46, 256); // colors in the table + return bmp; +} + +// A 1x1 RGB PNG: 8 signature bytes, a 25 byte IHDR chunk, a 24 byte IDAT chunk and a 12 byte IEND chunk. +static constexpr size_t PNG_HEADER_SIZE = 8 + 25; +static constexpr std::array PNG_1X1{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, 0x00, 0x00, 0x00, + 0x0C, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9C, 0x63, 0xF8, 0xCF, 0xC0, 0x00, 0x00, 0x03, 0x01, 0x01, 0x00, 0xC9, + 0xFE, 0x92, 0xEF, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4E, 0x44, 0xAE, 0x42, 0x60, 0x82}; + +static void set_loop_time(uint32_t now) { LoopBlockingGuard clock(nullptr, LOG_STR("test"), now); } + +class OnlineImageDownloadEnd : public ::testing::Test { + protected: + std::unique_ptr make_image_(runtime_image::ImageFormat format) { + auto img = std::make_unique("http://example.com/image", 0, 0, format, image::IMAGE_TYPE_RGB, + image::TRANSPARENCY_OPAQUE, nullptr, 256); + img->set_parent(&this->http_); + return img; + } + + void start_(OnlineImage &image, const std::shared_ptr &container) { + image.add_on_finished_callback([this](bool) { this->finished_++; }); + image.add_on_error_callback([this]() { this->errors_++; }); + this->http_.next = container; + image.update(); + } + + FakeHttpRequest http_; + int finished_{0}; + int errors_{0}; +}; + +// A chunked stream can end without the decoder reporting it is finished. PNG cannot tell a +// complete image from a cut-off one, so when the decoder used every byte the download succeeds, +// but the image is not cached: a 304 response must not keep it. +TEST_F(OnlineImageDownloadEnd, ChunkedPngCutOffFinishesWithoutCaching) { + std::vector png(PNG_1X1.begin(), PNG_1X1.begin() + PNG_HEADER_SIZE); // ends after IHDR + auto container = std::make_shared(png, png.size(), true); + container->add_response_header("etag", "\"abc\""); + container->add_response_header("last-modified", "Mon, 12 Oct 2026 10:00:00 GMT"); + auto image = this->make_image_(runtime_image::PNG); + this->start_(*image, container); + + for (int i = 0; i < 4 && this->finished_ + this->errors_ == 0; i++) + image->loop(); + + EXPECT_EQ(this->finished_, 1); + EXPECT_EQ(this->errors_, 0); + + this->http_.next = std::make_shared(png, 0, false); + image->update(); + EXPECT_FALSE(this->http_.sent_header("if-none-match")); + EXPECT_FALSE(this->http_.sent_header("if-modified-since")); +} + +// BMP knows its pixel count from the header, so a chunked BMP cut off at a row boundary is an error. +TEST_F(OnlineImageDownloadEnd, ChunkedBmpCutOffAtRowReportsError) { + std::vector bmp = make_bmp(); + bmp.resize(BMP_HEADER_SIZE + BMP_ROW_SIZE); // only the first row + auto container = std::make_shared(bmp, bmp.size(), true); + auto image = this->make_image_(runtime_image::BMP); + this->start_(*image, container); + + for (int i = 0; i < 4 && this->finished_ + this->errors_ == 0; i++) + image->loop(); + + EXPECT_EQ(this->errors_, 1); + EXPECT_EQ(this->finished_, 0); +} + +// An image the decoder confirmed as complete is cached. +TEST_F(OnlineImageDownloadEnd, CompleteImageIsCached) { + std::vector bmp = make_bmp(); + auto container = std::make_shared(bmp, bmp.size(), false); + container->add_response_header("etag", "\"abc\""); + container->add_response_header("last-modified", "Mon, 12 Oct 2026 10:00:00 GMT"); + auto image = this->make_image_(runtime_image::BMP); + this->start_(*image, container); + + for (int i = 0; i < 4 && this->finished_ + this->errors_ == 0; i++) + image->loop(); + ASSERT_EQ(this->finished_, 1); + + this->http_.next = std::make_shared(bmp, 0, false); + image->update(); + EXPECT_TRUE(this->http_.sent_header("if-none-match")); + EXPECT_TRUE(this->http_.sent_header("if-modified-since")); +} + +// ESP-IDF keeps returning 0 after the server closes the connection early. +TEST_F(OnlineImageDownloadEnd, NoDataForTimeoutReportsError) { + this->http_.set_timeout(100); + auto container = std::make_shared(make_bmp(), 20, false); + auto image = this->make_image_(runtime_image::BMP); + this->start_(*image, container); + + const uint32_t now = millis(); + set_loop_time(now); + image->loop(); + ASSERT_EQ(container->get_bytes_read(), 20u); + set_loop_time(now + 99); + image->loop(); + EXPECT_EQ(this->errors_, 0); + + set_loop_time(now + 100); + image->loop(); + EXPECT_EQ(this->errors_, 1); + EXPECT_EQ(this->finished_, 0); +} + +// The time spent connecting and reading the headers must not count as time without data. +TEST_F(OnlineImageDownloadEnd, NoDataTimeoutStartsAfterRequest) { + this->http_.set_timeout(100); + auto container = std::make_shared(make_bmp(), 0, false); + auto image = this->make_image_(runtime_image::BMP); + // update() runs in a dispatch that started before the blocking request, here one second earlier + set_loop_time(millis() - 1000); + this->start_(*image, container); + + const uint32_t now = millis(); + set_loop_time(now); + image->loop(); + EXPECT_EQ(this->errors_, 0); + + set_loop_time(now + 100); + image->loop(); + EXPECT_EQ(this->errors_, 1); +} + +// A full buffer the decoder cannot use waits for the no-data timeout before it fails, so a decoder +// that only had to wait is not cut off early. +TEST_F(OnlineImageDownloadEnd, FullBufferFailsAfterTimeout) { + this->http_.set_timeout(100); + auto bmp = make_palette_bmp(); + auto container = std::make_shared(bmp, bmp.size(), false); + auto image = this->make_image_(runtime_image::BMP); + this->start_(*image, container); + + const uint32_t now = millis(); + set_loop_time(now); + for (int i = 0; i < 8; i++) + image->loop(); + EXPECT_EQ(this->errors_, 0); + set_loop_time(now + 99); + image->loop(); + EXPECT_EQ(this->errors_, 0); + + set_loop_time(now + 100); + image->loop(); + EXPECT_EQ(this->errors_, 1); + EXPECT_EQ(this->finished_, 0); +} + +// A JPEG too big for the heap must fail, and the next download must get the configured buffer back. +TEST_F(OnlineImageDownloadEnd, JpegBufferAllocationFailureReportsError) { + auto image = this->make_image_(runtime_image::JPEG); + auto huge = std::make_shared(std::vector(16), 16, false); + huge->set_content_length(SIZE_MAX / 2); + this->start_(*image, huge); + EXPECT_EQ(this->errors_, 1); + + // A chunked response does not resize the buffer. The body is not a real JPEG, so the decoder + // rejects it, but only once it has been read into the buffer. + auto next = std::make_shared(std::vector(100, 0xAA), 100, true); + this->http_.next = next; + image->update(); + image->loop(); + EXPECT_EQ(next->get_bytes_read(), 100u); + EXPECT_EQ(this->errors_, 2); + EXPECT_EQ(this->finished_, 0); +} + +} // namespace esphome::online_image::testing diff --git a/tests/integration/fixtures/online_image_stalled_download.yaml b/tests/integration/fixtures/online_image_stalled_download.yaml new file mode 100644 index 00000000000..1be383ee9be --- /dev/null +++ b/tests/integration/fixtures/online_image_stalled_download.yaml @@ -0,0 +1,54 @@ +esphome: + name: online-image-stalled + +host: + +http_request: + timeout: 500ms + +display: + +image: + - platform: online_image + url: http://127.0.0.1:HTTP_PORT/truncated.bmp + id: truncated_img + format: BMP + type: RGB + on_download_finished: + logger.log: "download finished: truncated" + on_error: + logger.log: "download error: truncated" + - platform: online_image + url: http://127.0.0.1:HTTP_PORT/large_header.bmp + id: large_header_img + format: BMP + type: RGB + buffer_size: 256 + on_download_finished: + logger.log: "download finished: large_header" + on_error: + logger.log: "download error: large_header" + - platform: online_image + url: http://127.0.0.1:HTTP_PORT/empty.bmp + id: empty_img + format: BMP + type: RGB + on_download_finished: + logger.log: "download finished: empty" + on_error: + logger.log: "download error: empty" + +api: + actions: + - action: fetch_truncated + then: + - component.update: truncated_img + - action: fetch_large_header + then: + - component.update: large_header_img + - action: fetch_empty + then: + - component.update: empty_img + +logger: + level: DEBUG diff --git a/tests/integration/test_online_image_stalled_download.py b/tests/integration/test_online_image_stalled_download.py new file mode 100644 index 00000000000..35c549afe60 --- /dev/null +++ b/tests/integration/test_online_image_stalled_download.py @@ -0,0 +1,121 @@ +"""Downloads that can never complete must report an error instead of waiting forever.""" + +from __future__ import annotations + +import asyncio +import struct + +import pytest + +from .online_image_utils import BMP_IMAGE +from .types import APIClientConnectedFactory, RunCompiledFunction + +# 24-bit BMP cut off one byte into the sixth pixel: the decoder keeps that byte, +# waiting for the rest of the pixel, but the response has already ended. +TRUNCATED_BMP = BMP_IMAGE[: 54 + 5 * 3 + 1] + + +def _make_palette_bmp() -> bytes: + """Build an 8x8, 8-bit BMP whose 256-color table puts the pixels at offset 1078.""" + width = height = 8 + data_offset = 14 + 40 + 256 * 4 + file_size = data_offset + width * height + file_header = b"BM" + struct.pack(" None: + try: + async with asyncio.timeout(1.0): + request_line = await reader.readuntil(b"\r\n") + await reader.readuntil(b"\r\n\r\n") + path = request_line.split(b" ")[1].decode() + body = IMAGES[path] + writer.write( + b"\r\n".join( + [ + b"HTTP/1.1 200 OK", + b"Content-Length: %d" % len(body), + b"Content-Type: image/bmp", + b"Connection: close", + b"", + b"", + ] + ) + + body + ) + await writer.drain() + finally: + writer.close() + + +@pytest.mark.asyncio +async def test_online_image_stalled_download( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Esphome should report an error when a download can never complete.""" + loop = asyncio.get_running_loop() + expected_logs = { + "truncated_reason": "Download ended with 1 bytes the decoder could not use", + "truncated_error": "download error: truncated", + "large_header_reason": "Image does not fit in the 256 byte download buffer", + "large_header_error": "download error: large_header", + "empty_reason": "Server sent an empty response", + "empty_error": "download error: empty", + } + log_futures = {key: loop.create_future() for key in expected_logs} + finished_lines: list[str] = [] + + def check_output(line: str) -> None: + for key, text in expected_logs.items(): + if text in line and not log_futures[key].done(): + log_futures[key].set_result(True) + if "download finished" in line: + finished_lines.append(line) + + server = await asyncio.start_server(handle_http, "127.0.0.1", 0) + http_server_port = server.sockets[0].getsockname()[1] + config = yaml_config.replace("HTTP_PORT", str(http_server_port)) + + async with ( + server, + run_compiled(config, line_callback=check_output), + api_client_connected() as client, + ): + _, services = await client.list_entities_services() + actions = {s.name: s for s in services} + + await client.execute_service(actions["fetch_truncated"], {}) + async with asyncio.timeout(2.0): + await log_futures["truncated_reason"] + await log_futures["truncated_error"] + + await client.execute_service(actions["fetch_large_header"], {}) + async with asyncio.timeout(2.0): + await log_futures["large_header_reason"] + await log_futures["large_header_error"] + + await client.execute_service(actions["fetch_empty"], {}) + async with asyncio.timeout(2.0): + await log_futures["empty_reason"] + await log_futures["empty_error"] + + assert not finished_lines, f"Unexpected successful download: {finished_lines}"