From d84293931ba1bfe3583fd574d19a4bf3943aa373 Mon Sep 17 00:00:00 2001 From: guillempages Date: Sun, 23 Aug 2026 21:58:01 +0200 Subject: [PATCH] [online_image] Support image format auto-detection (#16337) Co-authored-by: J. Nick Koston Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: J. Nick Koston --- .../http_request/http_request_idf.cpp | 3 + esphome/components/online_image/image.py | 13 +- .../components/online_image/online_image.cpp | 52 +++++- esphome/components/runtime_image/__init__.py | 27 ++- .../components/runtime_image/image_format.h | 3 +- .../runtime_image/runtime_image.cpp | 18 +- .../components/runtime_image/runtime_image.h | 4 +- tests/components/online_image/common.yaml | 5 + .../runtime_image/test_decoder_reuse.cpp | 68 +++++--- ...ine_image_auto_detects_image_bmp_mime.yaml | 28 ++++ ...uto_detects_redirected_image_bmp_mime.yaml | 28 ++++ .../fixtures/online_image_bmp.yaml | 5 +- tests/integration/online_image_utils.py | 158 ++++++++++++++++++ ...nline_image_auto_detects_image_bmp_mime.py | 71 ++++++++ ..._auto_detects_redirected_image_bmp_mime.py | 71 ++++++++ tests/integration/test_online_image_bmp.py | 63 +------ 16 files changed, 513 insertions(+), 104 deletions(-) create mode 100644 tests/integration/fixtures/online_image_auto_detects_image_bmp_mime.yaml create mode 100644 tests/integration/fixtures/online_image_auto_detects_redirected_image_bmp_mime.yaml create mode 100644 tests/integration/online_image_utils.py create mode 100644 tests/integration/test_online_image_auto_detects_image_bmp_mime.py create mode 100644 tests/integration/test_online_image_auto_detects_redirected_image_bmp_mime.py diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index ddff954950..470ed332f1 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -196,6 +196,9 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c } container->feed_wdt(); + // IDF is the only backend reusing the container across redirect hops; + // drop the previous hop's headers (Arduino/host collect only the final response) + container->response_headers_.clear(); container->content_length = esp_http_client_fetch_headers(client); container->set_chunked(esp_http_client_is_chunked_response(client)); container->feed_wdt(); diff --git a/esphome/components/online_image/image.py b/esphome/components/online_image/image.py index ae785d17f9..3e9517937e 100644 --- a/esphome/components/online_image/image.py +++ b/esphome/components/online_image/image.py @@ -4,8 +4,16 @@ from esphome.components import runtime_image from esphome.components.const import CONF_REQUEST_HEADERS from esphome.components.http_request import CONF_HTTP_REQUEST_ID, HttpRequestComponent from esphome.components.image import CONF_TRANSPARENCY, add_metadata +from esphome.components.runtime_image import IMAGE_FORMATS import esphome.config_validation as cv -from esphome.const import CONF_BUFFER_SIZE, CONF_ID, CONF_ON_ERROR, CONF_TYPE, CONF_URL +from esphome.const import ( + CONF_BUFFER_SIZE, + CONF_FORMAT, + CONF_ID, + CONF_ON_ERROR, + CONF_TYPE, + CONF_URL, +) from esphome.core import ID, Lambda from esphome.cpp_generator import MockObj, TemplateArgsType from esphome.types import ConfigType @@ -31,7 +39,6 @@ ReleaseImageAction = online_image_ns.class_( "OnlineImageReleaseAction", automation.Action, cg.Parented.template(OnlineImage) ) - ONLINE_IMAGE_SCHEMA = ( runtime_image.runtime_image_schema(OnlineImage) .extend( @@ -39,6 +46,8 @@ ONLINE_IMAGE_SCHEMA = ( # Online Image specific options cv.GenerateID(CONF_HTTP_REQUEST_ID): cv.use_id(HttpRequestComponent), cv.Required(CONF_URL): cv.url, + # AUTO (Content-Type detection) is online_image specific; not in the shared registry + cv.Required(CONF_FORMAT): cv.one_of(*IMAGE_FORMATS, "AUTO", upper=True), cv.Optional(CONF_BUFFER_SIZE, default=65536): cv.int_range(256, 65536), cv.Optional(CONF_REQUEST_HEADERS): cv.All( cv.Schema({cv.string: cv.templatable(cv.string)}) diff --git a/esphome/components/online_image/online_image.cpp b/esphome/components/online_image/online_image.cpp index fe4f727cd6..a2662ff0e3 100644 --- a/esphome/components/online_image/online_image.cpp +++ b/esphome/components/online_image/online_image.cpp @@ -1,9 +1,11 @@ #include "online_image.h" #include "esphome/components/runtime_image/image_decoder.h" +#include "esphome/core/helpers.h" #include "esphome/core/log.h" #include static const char *const TAG = "online_image"; +static const char *const CONTENT_TYPE_HEADER_NAME = "content-type"; static const char *const ETAG_HEADER_NAME = "etag"; static const char *const IF_NONE_MATCH_HEADER_NAME = "if-none-match"; static const char *const LAST_MODIFIED_HEADER_NAME = "last-modified"; @@ -62,7 +64,8 @@ void OnlineImage::update() { // Add Accept header based on image format const char *accept_mime_type; - switch (this->get_format()) { + runtime_image::ImageFormat format = this->get_format(); + switch (format) { #ifdef USE_RUNTIME_IMAGE_BMP case runtime_image::BMP: accept_mime_type = "image/bmp,*/*;q=0.8"; @@ -89,8 +92,8 @@ void OnlineImage::update() { headers.push_back(http_request::Header{header.first, header.second.value()}); } - this->downloader_ = this->parent_->get(this->url_, headers, {ETAG_HEADER_NAME, LAST_MODIFIED_HEADER_NAME}); - + this->downloader_ = + this->parent_->get(this->url_, headers, {ETAG_HEADER_NAME, LAST_MODIFIED_HEADER_NAME, CONTENT_TYPE_HEADER_NAME}); if (this->downloader_ == nullptr) { ESP_LOGE(TAG, "Download failed."); this->end_connection_(); @@ -115,17 +118,54 @@ void OnlineImage::update() { ESP_LOGD(TAG, "Starting download"); size_t total_size = this->downloader_->content_length; + ESP_LOGV(TAG, "Content-Length: %zu", total_size); + + if (format == runtime_image::AUTO) { + // Try to auto-detect format from Content-Type header + auto content_type_header = this->downloader_->get_response_header(CONTENT_TYPE_HEADER_NAME); + const char *content_type = content_type_header.c_str(); + ESP_LOGV(TAG, "Content-Type: %s", content_type); + // Includes aliases seen from real servers (older IIS, CDNs, S3) + if (str_contains_ignore_case(content_type, "image/bmp") || + str_contains_ignore_case(content_type, "image/x-ms-bmp") || + str_contains_ignore_case(content_type, "image/x-bmp")) { + format = runtime_image::BMP; + } else if (str_contains_ignore_case(content_type, "image/jpeg") || + str_contains_ignore_case(content_type, "image/jpg")) { + format = runtime_image::JPEG; + } else if (str_contains_ignore_case(content_type, "image/png") || + str_contains_ignore_case(content_type, "image/x-png")) { + format = runtime_image::PNG; + } else if (str_contains_ignore_case(content_type, "image/")) { + ESP_LOGW(TAG, "Unsupported image type: '%s'", content_type); + this->end_connection_(); + this->download_error_callback_.call(); + return; + } else { + // TODO: implement auto-detection in runtime_image by sniffing the first few bytes of the image data + if (content_type_header.empty()) { + ESP_LOGW(TAG, "Server sent no Content-Type header; cannot determine image format. Set `format:` explicitly"); + } else { + ESP_LOGE(TAG, "Could not determine image format from Content-Type: '%s'. Set `format:` explicitly", + content_type); + } + this->end_connection_(); + this->download_error_callback_.call(); + return; + } + } + ESP_LOGD(TAG, "Using image format: %d", format); // Initialize decoder with the known format - if (!this->begin_decode(total_size)) { - ESP_LOGE(TAG, "Failed to initialize decoder for format %d", this->get_format()); + if (!this->begin_decode(total_size, format)) { + ESP_LOGE(TAG, "Failed to initialize decoder for format %d", format); this->end_connection_(); this->download_error_callback_.call(); return; } // JPEG requires the complete image in the download buffer before decoding - if (this->get_format() == runtime_image::JPEG && total_size > this->download_buffer_.size()) { + if (format == runtime_image::JPEG && total_size > this->download_buffer_.size()) { this->download_buffer_.resize(total_size); } diff --git a/esphome/components/runtime_image/__init__.py b/esphome/components/runtime_image/__init__.py index 3c130a7d75..0d4345db5b 100644 --- a/esphome/components/runtime_image/__init__.py +++ b/esphome/components/runtime_image/__init__.py @@ -58,6 +58,18 @@ class Format: """Add defines and libraries needed for this format.""" +class AUTOFormat(Format): + """AUTO format - detect from MIME type.""" + + def __init__(self): + super().__init__("AUTO", None) + + def actions(self) -> None: + # dict.fromkeys dedupes the JPG/JPEG alias so each format runs once + for image_format in dict.fromkeys(IMAGE_FORMATS.values()): + image_format.actions() + + class BMPFormat(Format): """BMP format decoder configuration.""" @@ -102,18 +114,25 @@ class PNGFormat(Format): cg.add_library("pngle", "1.1.0") -# Registry of available formats +# Decodable formats only; platforms that support runtime detection accept +# "AUTO" in their own schema and get_format() resolves it +_JPEG_FORMAT = JPEGFormat() IMAGE_FORMATS = { "BMP": BMPFormat(), - "JPEG": JPEGFormat(), + "JPEG": _JPEG_FORMAT, + "JPG": _JPEG_FORMAT, # Alias for JPEG "PNG": PNGFormat(), - "JPG": JPEGFormat(), # Alias for JPEG } +AUTO_FORMAT = AUTOFormat() + def get_format(format_name: str) -> Format | None: """Get a format instance by name.""" - return IMAGE_FORMATS.get(format_name.upper()) + name = format_name.upper() + if name == "AUTO": + return AUTO_FORMAT + return IMAGE_FORMATS.get(name) def enable_format(format_name: str) -> Format | None: diff --git a/esphome/components/runtime_image/image_format.h b/esphome/components/runtime_image/image_format.h index 524e52d7bc..ca6e0782b9 100644 --- a/esphome/components/runtime_image/image_format.h +++ b/esphome/components/runtime_image/image_format.h @@ -6,7 +6,8 @@ namespace esphome::runtime_image { * @brief Image format types that can be decoded dynamically. */ enum ImageFormat { - /** Automatically detect from data. Not implemented yet. */ + /** Format is supplied per decode, e.g. detected from the Content-Type header + * by online_image; sniffing the image data is not implemented. */ AUTO, /** JPEG format. */ JPEG, diff --git a/esphome/components/runtime_image/runtime_image.cpp b/esphome/components/runtime_image/runtime_image.cpp index e269f7d8f3..254624caf4 100644 --- a/esphome/components/runtime_image/runtime_image.cpp +++ b/esphome/components/runtime_image/runtime_image.cpp @@ -171,22 +171,27 @@ void RuntimeImage::draw(int x, int y, display::Display *display, Color color_on, // If no image is loaded and no placeholder, nothing to draw } -bool RuntimeImage::begin_decode(size_t expected_size) { +bool RuntimeImage::begin_decode(size_t expected_size, ImageFormat format) { if (this->is_decoding()) { ESP_LOGW(TAG, "Decoding already in progress"); return false; } + if (format == AUTO && this->format_ != AUTO) { + // Fall back to the configured format before the reuse check below + format = this->format_; + } + // An idle decoder for a different format cannot be reused - if (this->decoder_ != nullptr && this->decoder_->get_format() != this->format_) { - ESP_LOGD(TAG, "Decoder format mismatch: current: %d, new: %d", this->decoder_->get_format(), this->format_); + if (this->decoder_ != nullptr && this->decoder_->get_format() != format) { + ESP_LOGD(TAG, "Decoder format mismatch: current: %d, new: %d", this->decoder_->get_format(), format); this->decoder_ = nullptr; } if (!this->decoder_) { - this->decoder_ = this->create_decoder_(this->format_); + this->decoder_ = this->create_decoder_(format); if (!this->decoder_) { - ESP_LOGE(TAG, "Failed to create decoder for format %d", this->format_); + ESP_LOGE(TAG, "Failed to create decoder for format %d", format); return false; } } @@ -364,6 +369,9 @@ std::unique_ptr RuntimeImage::create_decoder_(ImageFormat format) case PNG: return make_unique(this); #endif + case AUTO: + ESP_LOGE(TAG, "Image format could not be determined; set `format:` explicitly in the configuration"); + return nullptr; default: ESP_LOGE(TAG, "Unsupported image format: %d", format); return nullptr; diff --git a/esphome/components/runtime_image/runtime_image.h b/esphome/components/runtime_image/runtime_image.h index cfac253fdb..55d5c0ae86 100644 --- a/esphome/components/runtime_image/runtime_image.h +++ b/esphome/components/runtime_image/runtime_image.h @@ -62,9 +62,10 @@ class RuntimeImage : public image::Image { * @brief Begin decoding an image. * * @param expected_size Optional hint about the expected data size. + * @param format The image format to decode (defaults to AUTO, which uses the value set at construction). * @return true if decoder was successfully initialized. */ - bool begin_decode(size_t expected_size = 0); + bool begin_decode(size_t expected_size = 0, ImageFormat format = AUTO); /** * @brief Feed data to the decoder. @@ -103,6 +104,7 @@ class RuntimeImage : public image::Image { /** * @brief Get the image format. */ + /// Configured format; a format resolved per decode lives on the active decoder ImageFormat get_format() const { return this->format_; } /** diff --git a/tests/components/online_image/common.yaml b/tests/components/online_image/common.yaml index f71cf63de9..85901287c7 100644 --- a/tests/components/online_image/common.yaml +++ b/tests/components/online_image/common.yaml @@ -57,6 +57,11 @@ image: url: http://www.faqs.org/images/library.jpg format: JPG type: RGB565 + - platform: online_image + id: online_auto_image + url: http://www.faqs.org/images/library.jpg + format: AUTO + type: RGB565 # Check the set_url action esphome: diff --git a/tests/components/runtime_image/test_decoder_reuse.cpp b/tests/components/runtime_image/test_decoder_reuse.cpp index 87e77b00be..9c2d00b747 100644 --- a/tests/components/runtime_image/test_decoder_reuse.cpp +++ b/tests/components/runtime_image/test_decoder_reuse.cpp @@ -77,18 +77,12 @@ class TestableRuntimeImage : public RuntimeImage { : RuntimeImage(format, image::IMAGE_TYPE_RGB, image::TRANSPARENCY_OPAQUE, nullptr, false, 0, 0) {} ImageDecoder *decoder() { return this->decoder_.get(); } - - /// Simulates the state a dynamic-format producer (PR #16337) would leave behind: - /// a cached decoder whose format no longer matches the image's format. - /// TODO: once #16337 adds a public way to change the format, drive the mismatch - /// through it and delete this seam. - void plant_decoder(ImageFormat format) { this->decoder_ = this->create_decoder_(format); } }; /// Runs one full decode session. Returns true when every stage succeeded. -static bool decode_all(TestableRuntimeImage &img, const uint8_t *data, size_t len) { +static bool decode_all(TestableRuntimeImage &img, const uint8_t *data, size_t len, ImageFormat format = AUTO) { std::vector buffer(data, data + len); // feed_data needs mutable bytes - if (!img.begin_decode(len)) { + if (!img.begin_decode(len, format)) { return false; } size_t offset = 0; @@ -203,25 +197,51 @@ TEST(RuntimeImageDecoder, ChunkedFeedDecodesLikeDownloadLoop) { } TEST(RuntimeImageDecoder, FormatSwitchEvictsMismatchedDecoder) { - // PNG image holding a stale BMP decoder: begin_decode must evict and recreate. - TestableRuntimeImage png_img(PNG); - png_img.plant_decoder(BMP); - ASSERT_NE(png_img.decoder(), nullptr); - ASSERT_EQ(png_img.decoder()->get_format(), BMP); + // Drive the format switch through begin_decode()'s format parameter, the way + // a dynamic-format producer (online_image MIME detection) does. + TestableRuntimeImage img(AUTO); - ASSERT_TRUE(decode_all(png_img, PNG_RGB, sizeof(PNG_RGB))); - EXPECT_EQ(png_img.decoder()->get_format(), PNG); - expect_pixels(png_img, PNG_RGB_EXPECTED); + ASSERT_TRUE(decode_all(img, BMP_24BPP, sizeof(BMP_24BPP), BMP)); + ASSERT_NE(img.decoder(), nullptr); + ASSERT_EQ(img.decoder()->get_format(), BMP); + expect_pixels(img, BMP_24BPP_EXPECTED); - // And the other direction: BMP image holding a stale PNG decoder. - TestableRuntimeImage bmp_img(BMP); - bmp_img.plant_decoder(PNG); - ASSERT_NE(bmp_img.decoder(), nullptr); - ASSERT_EQ(bmp_img.decoder()->get_format(), PNG); + // Same explicit format again: the decoder must stay warm. + ImageDecoder *bmp_decoder = img.decoder(); + ASSERT_TRUE(decode_all(img, BMP_8BPP, sizeof(BMP_8BPP), BMP)); + expect_pixels(img, BMP_8BPP_EXPECTED); + EXPECT_EQ(img.decoder(), bmp_decoder); - ASSERT_TRUE(decode_all(bmp_img, BMP_24BPP, sizeof(BMP_24BPP))); - EXPECT_EQ(bmp_img.decoder()->get_format(), BMP); - expect_pixels(bmp_img, BMP_24BPP_EXPECTED); + // Different format: the stale decoder must be evicted and recreated. + ASSERT_TRUE(decode_all(img, PNG_RGB, sizeof(PNG_RGB), PNG)); + EXPECT_EQ(img.decoder()->get_format(), PNG); + expect_pixels(img, PNG_RGB_EXPECTED); + + // And back again. + ASSERT_TRUE(decode_all(img, BMP_24BPP, sizeof(BMP_24BPP), BMP)); + EXPECT_EQ(img.decoder()->get_format(), BMP); + expect_pixels(img, BMP_24BPP_EXPECTED); +} + +TEST(RuntimeImageDecoder, AutoFormatFallsBackToConfiguredAndKeepsDecoderWarm) { + // With a configured format, an AUTO begin_decode() must resolve to the + // configured format before the reuse check instead of evicting the decoder. + TestableRuntimeImage img(BMP); + + ASSERT_TRUE(decode_all(img, BMP_24BPP, sizeof(BMP_24BPP), AUTO)); + ImageDecoder *first = img.decoder(); + ASSERT_NE(first, nullptr); + EXPECT_EQ(first->get_format(), BMP); + + ASSERT_TRUE(decode_all(img, BMP_24BPP, sizeof(BMP_24BPP), AUTO)); + expect_pixels(img, BMP_24BPP_EXPECTED); + EXPECT_EQ(img.decoder(), first) << "AUTO must not evict the configured-format decoder"; +} + +TEST(RuntimeImageDecoder, AutoWithoutConfiguredFormatFails) { + // Neither a configured format nor an explicit one: there is nothing to decode with. + TestableRuntimeImage img(AUTO); + EXPECT_FALSE(img.begin_decode(64)); } TEST(RuntimeImageDecoder, ReleaseKeepsDecoderWarm) { diff --git a/tests/integration/fixtures/online_image_auto_detects_image_bmp_mime.yaml b/tests/integration/fixtures/online_image_auto_detects_image_bmp_mime.yaml new file mode 100644 index 0000000000..95e42b6755 --- /dev/null +++ b/tests/integration/fixtures/online_image_auto_detects_image_bmp_mime.yaml @@ -0,0 +1,28 @@ +esphome: + name: online-image-bmp + +host: + +http_request: + +display: + +image: + - platform: online_image + url: http://127.0.0.1:HTTP_PORT/foo.bmp + format: AUTO + id: myimg + type: RGB + on_download_finished: + logger.log: + format: "download finished. cache hit: %u" + args: [cached] + +api: + actions: + - action: fetch_image + then: + - component.update: myimg + +logger: + level: DEBUG diff --git a/tests/integration/fixtures/online_image_auto_detects_redirected_image_bmp_mime.yaml b/tests/integration/fixtures/online_image_auto_detects_redirected_image_bmp_mime.yaml new file mode 100644 index 0000000000..837be776f1 --- /dev/null +++ b/tests/integration/fixtures/online_image_auto_detects_redirected_image_bmp_mime.yaml @@ -0,0 +1,28 @@ +esphome: + name: online-image-bmp + +host: + +http_request: + +display: + +image: + - platform: online_image + url: http://127.0.0.1:HTTP_PORT/foo.bmp + id: myimg + format: AUTO + type: RGB + on_download_finished: + logger.log: + format: "download finished. cache hit: %u" + args: [cached] + +api: + actions: + - action: fetch_image + then: + - component.update: myimg + +logger: + level: DEBUG diff --git a/tests/integration/fixtures/online_image_bmp.yaml b/tests/integration/fixtures/online_image_bmp.yaml index e36514e9ae..2cd0658b56 100644 --- a/tests/integration/fixtures/online_image_bmp.yaml +++ b/tests/integration/fixtures/online_image_bmp.yaml @@ -7,8 +7,9 @@ http_request: display: -online_image: - - url: http://127.0.0.1:HTTP_PORT/foo.bmp +image: + - platform: online_image + url: http://127.0.0.1:HTTP_PORT/foo.bmp id: myimg format: BMP type: RGB diff --git a/tests/integration/online_image_utils.py b/tests/integration/online_image_utils.py new file mode 100644 index 0000000000..02f0023cb5 --- /dev/null +++ b/tests/integration/online_image_utils.py @@ -0,0 +1,158 @@ +"""Shared fixture server and log helpers for the online_image integration tests.""" + +from __future__ import annotations + +import asyncio +from collections.abc import Callable +import re + +# black 8x8 RGB BMP, generated with +# from PIL import Image +# from io import BytesIO +# b = BytesIO() +# img = Image.new("RGB", (8, 8)) +# img.save(b, format="BMP") +# b.getvalue() +BMP_IMAGE = b"BM\xf6\x00\x00\x00\x00\x00\x00\x006\x00\x00\x00(\x00\x00\x00\x08\x00\x00\x00\x08\x00\x00\x00\x01\x00\x18\x00\x00\x00\x00\x00\xc0\x00\x00\x00\xc4\x0e\x00\x00\xc4\x0e\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +LEN_BMP_IMAGE = len(BMP_IMAGE) + + +async def wait_for_download( + downloaded_bytes_future: asyncio.Future, + server_error_future: asyncio.Future, +) -> int: + """Await the downloaded byte count, raising a server handler error first.""" + await asyncio.wait( + {downloaded_bytes_future, server_error_future}, + return_when=asyncio.FIRST_COMPLETED, + ) + if server_error_future.done() and (exc := server_error_future.exception()): + raise exc + # Retrieve a late teardown error so asyncio does not log it at GC + server_error_future.add_done_callback(lambda f: f.exception()) + return downloaded_bytes_future.result() + + +def make_download_watcher( + downloaded_bytes_future: asyncio.Future, + download_finished_future: asyncio.Future, +) -> Callable[[str], None]: + """Build a line callback resolving the futures from the device log.""" + + def check_output(line: str) -> None: + if ( + match := re.search(r"Image fully downloaded, (\d+) bytes", line) + ) and not downloaded_bytes_future.done(): + downloaded_bytes_future.set_result(int(match.group(1))) + if "download finished" in line and not download_finished_future.done(): + download_finished_future.set_result(True) + + return check_output + + +def handle_http( + http_request_future, + content_type: str = "text/plain", + *, + request_path: str = "/foo.bmp", + request_line_consumed: bool = False, + server_error_future: asyncio.Future | None = None, +): + async def handler(reader, writer): + try: + # Only read the request line if it hasn't been consumed by a caller + if not request_line_consumed: + async with asyncio.timeout(1.0): + data = await reader.readuntil(b"\r\n") + + expected_request = f"GET {request_path} HTTP/1.1\r\n".encode() + assert data[: len(expected_request)] == expected_request + + async with asyncio.timeout(1.0): + await reader.readuntil(b"\r\n\r\n") + + if not http_request_future.done(): + http_request_future.set_result(True) + + http_response = [ + b"HTTP/1.1 200 OK", + b"Content-Length: %d" % LEN_BMP_IMAGE, + f"Content-Type: {content_type}".encode(), + b"Connection: close", + b"", + b"", + ] + writer.write(b"\r\n".join(http_response)) + await writer.drain() + + writer.write(BMP_IMAGE) + + await writer.drain() + except Exception as exc: + if server_error_future is not None and not server_error_future.done(): + server_error_future.set_exception(exc) + if not http_request_future.done(): + http_request_future.set_exception(exc) + raise + finally: + writer.close() + + return handler + + +def handle_http_redirect( + http_request_future, final_request_future, server_error_future, port_holder +): + async def handler(reader, writer): + try: + async with asyncio.timeout(1.0): + request = await reader.readuntil(b"\r\n") + + if ( + request[: len(b"GET /foo.bmp HTTP/1.1\r\n")] + == b"GET /foo.bmp HTTP/1.1\r\n" + ): + if not http_request_future.done(): + http_request_future.set_result(True) + async with asyncio.timeout(1.0): + await reader.readuntil(b"\r\n\r\n") + + http_response = [ + b"HTTP/1.1 302 Found", + f"Location: http://127.0.0.1:{port_holder['port']}/final.bmp".encode(), + b"Content-Type: text/html", + b"Content-Length: 0", + b"Connection: close", + b"", + b"", + ] + writer.write(b"\r\n".join(http_response)) + await writer.drain() + return + + assert ( + request[: len(b"GET /final.bmp HTTP/1.1\r\n")] + == b"GET /final.bmp HTTP/1.1\r\n" + ) + if not final_request_future.done(): + final_request_future.set_result(True) + await handle_http( + final_request_future, + "image/bmp", + request_path="/final.bmp", + request_line_consumed=True, + server_error_future=server_error_future, + )(reader, writer) + except Exception as exc: + # Route handler failures to the dedicated error future so they're not silently lost + if not server_error_future.done(): + server_error_future.set_exception(exc) + if not http_request_future.done(): + http_request_future.set_exception(exc) + if not final_request_future.done(): + final_request_future.set_exception(exc) + raise + finally: + writer.close() + + return handler diff --git a/tests/integration/test_online_image_auto_detects_image_bmp_mime.py b/tests/integration/test_online_image_auto_detects_image_bmp_mime.py new file mode 100644 index 0000000000..3d2f6afd95 --- /dev/null +++ b/tests/integration/test_online_image_auto_detects_image_bmp_mime.py @@ -0,0 +1,71 @@ +"""Test that online_image AUTO format detection reads the Content-Type header.""" + +from __future__ import annotations + +import asyncio + +import pytest + +from .online_image_utils import ( + LEN_BMP_IMAGE, + handle_http, + make_download_watcher, + wait_for_download, +) +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_online_image_auto_detects_image_bmp_mime( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """AUTO format detection should honor the final response MIME type without explicit format.""" + loop = asyncio.get_running_loop() + http_request_future = loop.create_future() + server_error_future = loop.create_future() + download_finished_future = loop.create_future() + downloaded_bytes_future = loop.create_future() + + check_output = make_download_watcher( + downloaded_bytes_future, download_finished_future + ) + + server = await asyncio.start_server( + handle_http( + http_request_future, + "image/bmp", + server_error_future=server_error_future, + ), + "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, + ): + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "online-image-bmp" + + _, services = await client.list_entities_services() + request_service = next((s for s in services if s.name == "fetch_image"), None) + assert request_service is not None + + await client.execute_service(request_service, {}) + + async with asyncio.timeout(0.1): + await http_request_future + + async with asyncio.timeout(0.5): + numbytes = await wait_for_download( + downloaded_bytes_future, server_error_future + ) + assert numbytes == LEN_BMP_IMAGE + await download_finished_future diff --git a/tests/integration/test_online_image_auto_detects_redirected_image_bmp_mime.py b/tests/integration/test_online_image_auto_detects_redirected_image_bmp_mime.py new file mode 100644 index 0000000000..3fe28ac413 --- /dev/null +++ b/tests/integration/test_online_image_auto_detects_redirected_image_bmp_mime.py @@ -0,0 +1,71 @@ +"""Test that AUTO format detection uses the final Content-Type after redirects.""" + +from __future__ import annotations + +import asyncio + +import pytest + +from .online_image_utils import ( + LEN_BMP_IMAGE, + handle_http_redirect, + make_download_watcher, + wait_for_download, +) +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_online_image_auto_detects_redirected_image_bmp_mime( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Redirect hops should not leave the 302 HTML Content-Type in place for the final image.""" + loop = asyncio.get_running_loop() + http_request_future = loop.create_future() + final_request_future = loop.create_future() + server_error_future = loop.create_future() + download_finished_future = loop.create_future() + downloaded_bytes_future = loop.create_future() + + check_output = make_download_watcher( + downloaded_bytes_future, download_finished_future + ) + + port_holder = {} + server = await asyncio.start_server( + handle_http_redirect( + http_request_future, final_request_future, server_error_future, port_holder + ), + "127.0.0.1", + 0, + ) + port_holder["port"] = server.sockets[0].getsockname()[1] + + config = yaml_config.replace("HTTP_PORT", str(port_holder["port"])) + + async with ( + server, + run_compiled(config, line_callback=check_output), + api_client_connected() as client, + ): + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "online-image-bmp" + + _, services = await client.list_entities_services() + request_service = next((s for s in services if s.name == "fetch_image"), None) + assert request_service is not None + + await client.execute_service(request_service, {}) + + async with asyncio.timeout(0.1): + await http_request_future + async with asyncio.timeout(0.5): + await final_request_future + numbytes = await wait_for_download( + downloaded_bytes_future, server_error_future + ) + assert numbytes == LEN_BMP_IMAGE + await download_finished_future diff --git a/tests/integration/test_online_image_bmp.py b/tests/integration/test_online_image_bmp.py index 7c32154fdd..871a97b753 100644 --- a/tests/integration/test_online_image_bmp.py +++ b/tests/integration/test_online_image_bmp.py @@ -1,62 +1,12 @@ from __future__ import annotations import asyncio -import re import pytest +from .online_image_utils import LEN_BMP_IMAGE, handle_http, make_download_watcher from .types import APIClientConnectedFactory, RunCompiledFunction -# black 8x8 RGB BMP, generated with -# from PIL import Image -# from io import BytesIO -# b = BytesIO() -# img = Image.new("RGB", (8, 8)) -# img.save(b, format="BMP") -# b.getvalue() -BMP_IMAGE = b"BM\xf6\x00\x00\x00\x00\x00\x00\x006\x00\x00\x00(\x00\x00\x00\x08\x00\x00\x00\x08\x00\x00\x00\x01\x00\x18\x00\x00\x00\x00\x00\xc0\x00\x00\x00\xc4\x0e\x00\x00\xc4\x0e\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" -LEN_BMP_IMAGE = len(BMP_IMAGE) - - -def handle_http(http_request_future): - async def handler(reader, writer): - try: - async with asyncio.timeout(1.0): - data = await reader.readuntil(b"\r\n") - - # ensure our request matches the expectation - expected_request = b"GET /foo.bmp HTTP/1.1\r\n" - assert data[: len(expected_request)] == expected_request - - # consume rest of request - async with asyncio.timeout(1.0): - data = await reader.readuntil(b"\r\n\r\n") - - http_request_future.set_result(True) - - http_response = [ - b"HTTP/1.1 200 OK", - b"Content-Length: %d" % LEN_BMP_IMAGE, - b"Content-Type: text/plain", - b"Connection: close", - b"", - b"", - ] - writer.write(b"\r\n".join(http_response)) - await writer.drain() - - writer.write(BMP_IMAGE) - - await writer.drain() - except Exception as exc: - if not http_request_future.done(): - http_request_future.set_exception(exc) - raise - finally: - writer.close() - - return handler - @pytest.mark.asyncio async def test_online_image_bmp( @@ -72,14 +22,9 @@ async def test_online_image_bmp( download_finished_future = loop.create_future() downloaded_bytes_future = loop.create_future() - def check_output(line: str) -> None: - """Check log output for expected messages.""" - - if match := re.search(r"Image fully downloaded, (\d+) bytes", line): - downloaded_bytes_future.set_result(int(match.group(1))) - - if "download finished" in line: - download_finished_future.set_result(True) + check_output = make_download_watcher( + downloaded_bytes_future, download_finished_future + ) server = await asyncio.start_server( handle_http(http_request_future), "127.0.0.1", 0