From bf972bc281bb458d5e8b46a00ad4f996601fa49b Mon Sep 17 00:00:00 2001 From: Guido Jansen Date: Sat, 31 Oct 2026 14:14:37 +0100 Subject: [PATCH 1/5] Fix host mocks for C++ headers and filesystem persistence --- test_host/mocks/freertos/FreeRTOS.h | 6 +++--- test_host/mocks/sys/stat.h | 21 +++------------------ 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/test_host/mocks/freertos/FreeRTOS.h b/test_host/mocks/freertos/FreeRTOS.h index ddff0dc2..7ac205c7 100644 --- a/test_host/mocks/freertos/FreeRTOS.h +++ b/test_host/mocks/freertos/FreeRTOS.h @@ -2,13 +2,13 @@ // Minimal FreeRTOS type definitions for host testing +#include +#include + #ifdef __cplusplus extern "C" { #endif -#include -#include - typedef void* TaskHandle_t; typedef void* QueueHandle_t; typedef void* SemaphoreHandle_t; diff --git a/test_host/mocks/sys/stat.h b/test_host/mocks/sys/stat.h index a5447d66..1e974a6d 100644 --- a/test_host/mocks/sys/stat.h +++ b/test_host/mocks/sys/stat.h @@ -1,20 +1,5 @@ #pragma once -#ifdef __cplusplus -extern "C" { -#endif - -#include - -struct stat { - uint64_t st_size; - uint32_t st_mode; - uint32_t st_mtime; -}; - -inline int stat(const char* path, struct stat* buf) { return -1; } -inline int remove(const char* path) { return -1; } - -#ifdef __cplusplus -} -#endif +// Host persistence tests write real files. Use the host ABI and errno rather +// than a fake stat() that always fails and leaves errno from an earlier call. +#include_next From f552144c70ff8220de8d4a8ff0d4000cfa2f78d3 Mon Sep 17 00:00:00 2001 From: Guido Jansen Date: Sat, 31 Oct 2026 14:14:50 +0100 Subject: [PATCH 2/5] Bound Logs page deduplication and evict rows incrementally Keep duplicate keys in sync with retained entries and remove only evicted DOM rows. Cover long-running retention, filtering and restart markers. --- static/logs.html | 42 +++++++------- test_host/CMakeLists.txt | 4 +- test_host/web/logs_page_tests.cjs | 94 +++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 test_host/web/logs_page_tests.cjs diff --git a/static/logs.html b/static/logs.html index 8ed19513..24eae207 100644 --- a/static/logs.html +++ b/static/logs.html @@ -221,12 +221,12 @@

Logs

: 'No logs available.'; } - function prependRenderedEntries(newEntries) { - if (newEntries.length === 0) return; + function prependRenderedEntries(newEntries, removedEntries = []) { + if (newEntries.length === 0 && removedEntries.length === 0) return; const checkedLevels = getCheckedLevels(); const logsBody = document.getElementById('logsBody'); - const isPlaceholderVisible = logsBody.children.length === 1 && logsBody.textContent.includes('No logs available.'); + const isPlaceholderVisible = renderedLines.length === 0; if (isPlaceholderVisible) { logsBody.innerHTML = ''; } @@ -236,14 +236,23 @@

Logs

renderedLines.unshift(buildRenderedLine(logEntry)); logsBody.insertAdjacentHTML('afterbegin', buildRenderedRow(logEntry)); } + + // Remove only evicted visible rows, keeping the rest of the DOM intact. + for (const entry of removedEntries) { + if (!entry.logRestart && !checkedLevels.includes(entry.level)) continue; + renderedLines.pop(); + logsBody.lastElementChild?.remove(); + } + } + + function logEntryKey(entry) { + return `${entry.millis}|${entry.level}|${entry.sid}|${entry.pid}|${JSON.stringify(entry.message)}`; } function addFetchedEntries(logItems) { const newEntries = []; - let droppedAny = false; for (const logEntry of logItems) { - // const entryKey = `${logEntry.millis}|${logEntry.level}|${logEntry.message}`; - const entryKey = `${logEntry.millis}|${logEntry.level}|${logEntry.sid}|${logEntry.pid}|${JSON.stringify(logEntry.message)}`; + const entryKey = logEntryKey(logEntry); if (seenEntries.has(entryKey)) continue; seenEntries.add(entryKey); @@ -251,16 +260,9 @@

Logs

newEntries.push(logEntry); } - if (storedEntries.length > maxLines) { - const removedEntries = storedEntries.splice(maxLines); - droppedAny = removedEntries.length > 0; - for (const entry of removedEntries) { - const entryKey = `${entry.millis}|${entry.level}|${entry.message}`; - seenEntries.delete(entryKey); - } - } - - return { newEntries, droppedAny }; + const removedEntries = storedEntries.splice(maxLines); + for (const entry of removedEntries) seenEntries.delete(logEntryKey(entry)); + return { newEntries, removedEntries }; } async function loadTimeRelationOnce() { @@ -325,12 +327,8 @@

Logs

logRestart: true }, ...logItems] : logItems; - const { newEntries, droppedAny } = addFetchedEntries(itemsToAdd); - if (droppedAny) { - renderFromStoredData(); - } else { - prependRenderedEntries(newEntries); - } + const { newEntries, removedEntries } = addFetchedEntries(itemsToAdd); + prependRenderedEntries(newEntries, removedEntries); if (Number.isFinite(maxMillis)) { lastSeenMillis = maxMillis; } diff --git a/test_host/CMakeLists.txt b/test_host/CMakeLists.txt index c9d6f925..6b17cac1 100644 --- a/test_host/CMakeLists.txt +++ b/test_host/CMakeLists.txt @@ -269,7 +269,9 @@ find_program(NODE_EXECUTABLE node) if(NODE_EXECUTABLE) add_test(NAME config_page_defaults COMMAND ${NODE_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/web/config_page_tests.cjs) - set_tests_properties(config_page_defaults PROPERTIES ENVIRONMENT "ELECTRON_RUN_AS_NODE=1") + add_test(NAME logs_page_retention + COMMAND ${NODE_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/web/logs_page_tests.cjs) + set_tests_properties(config_page_defaults logs_page_retention PROPERTIES ENVIRONMENT "ELECTRON_RUN_AS_NODE=1") endif() include(CTest) # Per-case CTest entries: timeouts apply per case, diff --git a/test_host/web/logs_page_tests.cjs b/test_host/web/logs_page_tests.cjs new file mode 100644 index 00000000..3ea9b5e6 --- /dev/null +++ b/test_host/web/logs_page_tests.cjs @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Exercise the real page's retention and incremental DOM updates over a long run. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +const html = fs.readFileSync(path.join(__dirname, '../../static/logs.html'), 'utf8'); +const scripts = [...html.matchAll(/]*)?>([\s\S]*?)<\/script>/g)]; +const elements = new Map(); +const levels = ['DEBUG', 'INFO', 'WARN', 'ERROR'].map(value => ({ value, checked: true, addEventListener() {} })); +let fullTableRenders = 0; +const rows = []; +const table = { + get innerHTML() { return rows.map(row => row.html).join(''); }, + set innerHTML(value) { + fullTableRenders++; + rows.length = 0; + for (const match of value.matchAll(//g)) rows.push(makeRow(match[0])); + }, + get children() { return rows; }, + get lastElementChild() { return rows.at(-1); }, + get textContent() { return this.innerHTML; }, + insertAdjacentHTML(where, value) { + assert.equal(where, 'afterbegin'); + rows.unshift(makeRow(value)); + }, +}; +function makeRow(html) { + return { html, remove() { rows.splice(rows.indexOf(this), 1); } }; +} +elements.set('logsBody', table); +const context = vm.createContext({ + console, + handleHome() {}, + document: { + getElementById(id) { + if (!elements.has(id)) elements.set(id, { checked: false, textContent: '', addEventListener() {} }); + return elements.get(id); + }, + querySelectorAll(selector) { + return selector.includes(':checked') ? levels.filter(level => level.checked) : levels; + }, + }, + fetch: async () => ({ ok: true, json: async () => ({ logs: [] }) }), + setTimeout: () => 0, + clearTimeout() {}, +}); +vm.runInContext(scripts.at(-1)[1], context); +const run = code => vm.runInContext(code, context); +function batch(entries) { + context.entries = entries; + run('var batchResult = addFetchedEntries(entries); prependRenderedEntries(batchResult.newEntries, batchResult.removedEntries);'); +} +const entry = i => ({ millis: i, level: i % 2 ? 'INFO' : 'DEBUG', sid: 1, pid: 2, message: { index: i } }); + +(async () => { + // Complete the empty initial fetch before driving updates directly. + await new Promise(resolve => setImmediate(resolve)); + batch(Array.from({ length: 2000 }, (_, i) => entry(i))); + assert.equal(run('storedEntries.length'), 2000); + assert.equal(run('seenEntries.size'), 2000); + assert.equal(rows.length, 2000); + const rendersBeforeEviction = fullTableRenders; + for (let i = 2000; i < 10000; i += 20) batch(Array.from({ length: 20 }, (_, j) => entry(i + j))); + assert.equal(run('storedEntries.length'), 2000); + assert.equal(run('seenEntries.size'), 2000, 'deduplication memory must stay bounded'); + assert.equal(rows.length, 2000); + assert.equal(run('renderedLines.length'), 2000); + assert.equal(fullTableRenders, rendersBeforeEviction, 'eviction must not rebuild the table'); + assert(rows[0].html.includes('9999')); + assert(rows.at(-1).html.includes('8000')); + batch([entry(9999)]); + assert.equal(rows.length, 2000, 'overlapping fetches must not duplicate entries'); + + levels.find(level => level.value === 'DEBUG').checked = false; + run('renderFromStoredData()'); + assert.equal(rows.length, 1000); + for (let i = 10000; i < 12500; i += 10) batch(Array.from({ length: 10 }, (_, j) => entry(i + j))); + assert.equal(rows.length, 1000, 'filtered evicted entries must not remove unrelated rows'); + assert.equal(run('renderedLines.length'), 1000); + assert.equal(run('seenEntries.size'), 2000); + + // Restart markers remain visible even with all levels disabled, and evict normally. + for (const level of levels) level.checked = false; + run('renderFromStoredData()'); + batch([{ millis: 1, level: 'RESTART', message: 'restart', logRestart: true }]); + assert.equal(rows.length, 1); + batch(Array.from({ length: 2001 }, (_, i) => entry(20000 + i))); + assert.equal(rows.length, 0); + assert.equal(run('renderedLines.length'), 0); + assert.equal(run('seenEntries.size'), 2000); + console.log('Log retention, deduplication, filters, restart markers and incremental rendering passed.'); +})().catch(error => { console.error(error); process.exitCode = 1; }); From 67a5e078a4ee11da62ffa996f169ac671d531df1 Mon Sep 17 00:00:00 2001 From: Guido Jansen Date: Sat, 31 Oct 2026 14:14:54 +0100 Subject: [PATCH 3/5] Schedule system diagnostics by elapsed time Throttle heap and socket sampling independently of notification frequency, collect expensive data outside critical sections, and report uptime in seconds. Test sampling and reporting deadlines. --- include/system/detail/monitor_schedule.hpp | 38 ++++++++++++++++ include/system/system_monitor.hpp | 4 +- src/system/system_monitor.cpp | 49 ++++++++++----------- test_host/CMakeLists.txt | 1 + test_host/system/monitor_schedule_tests.cpp | 34 ++++++++++++++ 5 files changed, 99 insertions(+), 27 deletions(-) create mode 100644 include/system/detail/monitor_schedule.hpp create mode 100644 test_host/system/monitor_schedule_tests.cpp diff --git a/include/system/detail/monitor_schedule.hpp b/include/system/detail/monitor_schedule.hpp new file mode 100644 index 00000000..cedbafa6 --- /dev/null +++ b/include/system/detail/monitor_schedule.hpp @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +#pragma once + +#include + +namespace app::detail { + +// Monotonic deadlines: bus notifications must not accelerate health sampling. +class MonitorSchedule { + public: + uint32_t waitMs(uint64_t now_us) const { + if (now_us >= next_status_us_) return 0; + return static_cast((next_status_us_ - now_us + 999) / 1000); + } + + bool statusDue(uint64_t now_us) { + return advance(now_us, next_status_us_, 30ULL * 1000000); + } + bool summaryDue(uint64_t now_us) { + return advance(now_us, next_summary_us_, 300ULL * 1000000); + } + bool trendDue(uint64_t now_us) { + return advance(now_us, next_trend_us_, 3600ULL * 1000000); + } + + private: + static bool advance(uint64_t now_us, uint64_t& next_us, uint64_t period_us) { + if (now_us < next_us) return false; + next_us = now_us + period_us; + return true; + } + + uint64_t next_status_us_ = 0; + uint64_t next_summary_us_ = 0; + uint64_t next_trend_us_ = 0; +}; + +} // namespace app::detail diff --git a/include/system/system_monitor.hpp b/include/system/system_monitor.hpp index bb436332..16f02ad4 100644 --- a/include/system/system_monitor.hpp +++ b/include/system/system_monitor.hpp @@ -7,6 +7,8 @@ #include #include + +#include "system/detail/monitor_schedule.hpp" #include #include #include @@ -100,7 +102,7 @@ class SystemMonitor { HeapSample heap_trend_[heap_trend_capacity] = {}; size_t heap_trend_index_ = 0; size_t heap_trend_count_ = 0; - uint32_t heap_trend_tick_ = 0; + app::detail::MonitorSchedule schedule_; }; #endif diff --git a/src/system/system_monitor.cpp b/src/system/system_monitor.cpp index fa1d7d76..5ce40827 100644 --- a/src/system/system_monitor.cpp +++ b/src/system/system_monitor.cpp @@ -20,8 +20,6 @@ #include "system/logger.hpp" namespace { -constexpr uint32_t system_monitor_period_ms = 30000; -constexpr uint32_t log_summary_interval_ms = 300000; constexpr size_t log_queue_size = 8; constexpr size_t protocol_queue_size = 8; @@ -78,6 +76,7 @@ static StaticQueue_t protocol_queue_cb; TaskHandle_t SystemMonitor::taskHandle() const { return task_handle_; } bool SystemMonitor::begin() { + schedule_ = app::detail::MonitorSchedule{}; status_mux_ = portMUX_INITIALIZER_UNLOCKED; status_.uptime_seconds = 0; status_.free_heap = 0; @@ -291,21 +290,19 @@ void SystemMonitor::taskEntry(void* arg) { } void SystemMonitor::taskLoop() { - uint32_t last_summary = 0; - for (;;) { - if (xTaskNotifyWait(0, 0, nullptr, - pdMS_TO_TICKS(system_monitor_period_ms)) == pdTRUE) { - processLogRequests(); - processProtocolInfo(); + const uint64_t now_us = esp_timer_get_time(); + if (schedule_.statusDue(now_us)) { + collectStatus(); + if (schedule_.summaryDue(now_us)) logSummary(); } - collectStatus(); - - uint32_t now = xTaskGetTickCount() * portTICK_PERIOD_MS; - if (now - last_summary >= log_summary_interval_ms) { - last_summary = now; - logSummary(); + const uint32_t wait_ms = schedule_.waitMs(esp_timer_get_time()); + const TickType_t wait_ticks = + (wait_ms + portTICK_PERIOD_MS - 1) / portTICK_PERIOD_MS; + if (xTaskNotifyWait(0, UINT32_MAX, nullptr, wait_ticks) == pdTRUE) { + processLogRequests(); + processProtocolInfo(); } } } @@ -440,18 +437,21 @@ size_t SystemMonitor::fetchHeapTrend(HeapSample* out, size_t capacity) const { } void SystemMonitor::collectStatus() { - portENTER_CRITICAL(&status_mux_); - status_.uptime_seconds = - static_cast(esp_timer_get_time() / 1000ULL); - status_.free_heap = heap_caps_get_free_size(MALLOC_CAP_8BIT); - status_.min_free_heap = heap_caps_get_minimum_free_size(MALLOC_CAP_8BIT); + const uint64_t now_us = esp_timer_get_time(); multi_heap_info_t info; + // Heap walks and socket calls may be expensive; never mask interrupts here. heap_caps_get_info(&info, MALLOC_CAP_8BIT); + int detected = 0; + int connected = 0; + getSocketStatus(detected, connected); + + const bool sample_trend = schedule_.trendDue(now_us); + portENTER_CRITICAL(&status_mux_); + status_.uptime_seconds = static_cast(now_us / 1000000ULL); + status_.free_heap = info.total_free_bytes; + status_.min_free_heap = info.minimum_free_bytes; status_.largest_free_block = info.largest_free_block; - // Hourly trend sample (collectStatus runs every 30 s): answers - // leak-vs-fragmentation without polling. - if (++heap_trend_tick_ >= 120) { - heap_trend_tick_ = 0; + if (sample_trend) { heap_trend_[heap_trend_index_] = {status_.uptime_seconds, status_.free_heap, status_.min_free_heap, status_.largest_free_block}; @@ -460,9 +460,6 @@ void SystemMonitor::collectStatus() { } portEXIT_CRITICAL(&status_mux_); - int detected = 0; - int connected = 0; - getSocketStatus(detected, connected); sockets_detected_.store(detected); sockets_connected_.store(connected); } diff --git a/test_host/CMakeLists.txt b/test_host/CMakeLists.txt index 6b17cac1..fba74f65 100644 --- a/test_host/CMakeLists.txt +++ b/test_host/CMakeLists.txt @@ -183,6 +183,7 @@ target_link_libraries(network_tests PRIVATE add_executable(system_tests ${CMAKE_CURRENT_SOURCE_DIR}/system/string_pool_tests.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/system/monitor_schedule_tests.cpp ) target_link_libraries(system_tests PRIVATE system_test_support diff --git a/test_host/system/monitor_schedule_tests.cpp b/test_host/system/monitor_schedule_tests.cpp new file mode 100644 index 00000000..31e3fb65 --- /dev/null +++ b/test_host/system/monitor_schedule_tests.cpp @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +#include +#include "system/detail/monitor_schedule.hpp" + +TEST_CASE("Bus notifications do not accelerate health or heap trend sampling", "[monitor]") { + app::detail::MonitorSchedule schedule; + REQUIRE(schedule.statusDue(0)); + REQUIRE(schedule.summaryDue(0)); + REQUIRE(schedule.trendDue(0)); + REQUIRE(schedule.waitMs(0) == 30000); + for (uint64_t now = 1; now < 30000000; now += 500) { + REQUIRE_FALSE(schedule.statusDue(now)); + REQUIRE_FALSE(schedule.summaryDue(now)); + REQUIRE_FALSE(schedule.trendDue(now)); + } + REQUIRE(schedule.statusDue(30000000)); + REQUIRE_FALSE(schedule.statusDue(30000000)); + REQUIRE(schedule.summaryDue(300000000)); + REQUIRE_FALSE(schedule.trendDue(3599999999ULL)); + REQUIRE(schedule.trendDue(3600000000ULL)); +} + +TEST_CASE("Delayed monitoring skips missed intervals and handles long uptimes", "[monitor]") { + app::detail::MonitorSchedule schedule; + constexpr uint64_t now = 100ULL * 24 * 3600 * 1000000; + REQUIRE(schedule.statusDue(now)); + REQUIRE(schedule.summaryDue(now)); + REQUIRE(schedule.trendDue(now)); + REQUIRE_FALSE(schedule.statusDue(now + 1)); + REQUIRE(schedule.waitMs(now + 1) == 30000); + REQUIRE(schedule.waitMs(now + 29999999) == 1); + REQUIRE(schedule.waitMs(now + 30000000) == 0); + REQUIRE(schedule.statusDue(now + 30000000)); +} From d8a436e122b8c4c5aef81e50342945327638c060 Mon Sep 17 00:00:00 2001 From: Guido Jansen Date: Sat, 31 Oct 2026 14:14:55 +0100 Subject: [PATCH 4/5] Prevent string-pool ID overflow and concurrent insertion races Bound IDs and string lengths, synchronize access, and retain existing IDs at capacity. Cover exhaustion and concurrent access. --- include/system/string_pool.hpp | 16 +++++++--- test_host/system/string_pool_tests.cpp | 43 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/include/system/string_pool.hpp b/include/system/string_pool.hpp index e4d546f4..d13a88df 100644 --- a/include/system/string_pool.hpp +++ b/include/system/string_pool.hpp @@ -4,6 +4,7 @@ #include #include +#include #include class StringPool { @@ -11,7 +12,8 @@ class StringPool { static StringPool& instance(); uint8_t intern(std::string_view str) { - if (str.empty() || count_ >= max_strings_) return 0; + if (str.empty() || str.size() > 255) return 0; + std::lock_guard lock(mutex_); for (uint8_t i = 0; i < count_; i++) { if (lengths_[i] == str.size() && @@ -20,7 +22,8 @@ class StringPool { } } - if (buffer_offset_ + str.size() + 1 > buffer_size_) return 0; + if (count_ >= max_strings_ || + buffer_offset_ + str.size() + 1 > buffer_size_) return 0; std::memcpy(buffer_ + buffer_offset_, str.data(), str.size()); buffer_[buffer_offset_ + str.size()] = '\0'; @@ -31,17 +34,22 @@ class StringPool { } std::string_view lookup(uint8_t id) const { + std::lock_guard lock(mutex_); if (id == 0 || id > count_) return {}; const uint8_t idx = id - 1; return std::string_view(buffer_ + offsets_[idx], lengths_[idx]); } - uint8_t count() const { return count_; } + uint8_t count() const { + std::lock_guard lock(mutex_); + return count_; + } private: - static constexpr size_t max_strings_ = 256; + static constexpr size_t max_strings_ = 255; static constexpr size_t buffer_size_ = 2048; + mutable std::mutex mutex_; char buffer_[buffer_size_]{}; uint16_t offsets_[max_strings_]{}; uint8_t lengths_[max_strings_]{}; diff --git a/test_host/system/string_pool_tests.cpp b/test_host/system/string_pool_tests.cpp index 0905dd24..730d0ae2 100644 --- a/test_host/system/string_pool_tests.cpp +++ b/test_host/system/string_pool_tests.cpp @@ -1,5 +1,7 @@ #include #include +#include +#include #include "system/string_pool.hpp" @@ -26,3 +28,44 @@ TEST_CASE("StringPool interns and looks up strings", "[string_pool]") { REQUIRE(pool.intern("") == 0); REQUIRE(pool.lookup(0).empty()); } + +TEST_CASE("StringPool keeps existing IDs valid when its index capacity is full", "[string_pool]") { + StringPool pool; + for (unsigned i = 0; i < 255; ++i) { + REQUIRE(pool.intern(std::to_string(i)) == i + 1); + } + REQUIRE(pool.intern("overflow") == 0); + REQUIRE(pool.count() == 255); + for (unsigned i = 0; i < 255; ++i) { + REQUIRE(pool.lookup(static_cast(i + 1)) == std::to_string(i)); + REQUIRE(pool.intern(std::to_string(i)) == i + 1); + } +} + +TEST_CASE("StringPool rejects oversized names and preserves entries when bytes run out", "[string_pool]") { + StringPool pool; + REQUIRE(pool.intern(std::string(256, 'x')) == 0); + for (int i = 0; i < 8; ++i) REQUIRE(pool.intern(std::string(255, 'a' + i)) != 0); + REQUIRE(pool.intern("no room") == 0); + REQUIRE(pool.intern(std::string(255, 'a')) == 1); + REQUIRE(pool.lookup(1) == std::string(255, 'a')); +} + +TEST_CASE("StringPool concurrently interns shared names without duplicate IDs", "[string_pool]") { + StringPool pool; + std::vector workers; + for (int i = 0; i < 8; ++i) { + workers.emplace_back([&] { + for (int repeat = 0; repeat < 100; ++repeat) { + for (int key = 0; key < 20; ++key) pool.intern(std::to_string(key)); + } + }); + } + for (auto& worker : workers) worker.join(); + REQUIRE(pool.count() == 20); + for (int key = 0; key < 20; ++key) { + const auto id = pool.intern(std::to_string(key)); + REQUIRE(id != 0); + REQUIRE(pool.lookup(id) == std::to_string(key)); + } +} From 50a798d6f89ea7a52954b43a61211ede9b89da3f Mon Sep 17 00:00:00 2001 From: Guido Jansen Date: Sat, 31 Oct 2026 14:15:04 +0100 Subject: [PATCH 5/5] Buffer HTTP streams and stop sending after socket failures Share bounded chunk buffering across streaming endpoints, return send errors without retrying, and shorten socket waits. Finish drift JSON before terminating its response. Test payload preservation, failed sends and valid response termination. --- include/network/http_utils.hpp | 44 ++++++++++++++++++++++ src/api/adc_api.cpp | 16 ++++---- src/api/commands_api.cpp | 8 ++-- src/api/cron_api.cpp | 8 ++-- src/api/devices_api.cpp | 8 ++-- src/api/logs_api.cpp | 24 ++++++------ src/api/metrics_api.cpp | 8 ++-- src/api/network_api.cpp | 16 ++++---- src/api/status_api.cpp | 8 ++-- src/api/system_api.cpp | 24 ++++++------ src/api/values_api.cpp | 8 ++-- src/config/config_manager.cpp | 30 ++++++++------- src/network/http.cpp | 4 +- src/system/upgrade_manager.cpp | 8 ++-- test_host/api/logs_api_tests.cpp | 10 +++++ test_host/config/config_manager_tests.cpp | 24 ++++++++++++ test_host/mocks/esp_http_server.h | 6 +++ test_host/network/http_utils_tests.cpp | 45 +++++++++++++++++++++++ 18 files changed, 215 insertions(+), 84 deletions(-) diff --git a/include/network/http_utils.hpp b/include/network/http_utils.hpp index b33d9e18..0dc1bf9e 100644 --- a/include/network/http_utils.hpp +++ b/include/network/http_utils.hpp @@ -3,6 +3,8 @@ #include #include +#include +#include #include #include @@ -11,6 +13,48 @@ namespace HttpUtils { constexpr size_t max_request_body_size = 8192; constexpr size_t streaming_buffer_size = 4096; +// Coalesce small JSON fragments and stop socket I/O after the first failure. +// Call finish() after all writers/scopes have emitted their closing bytes. +class ChunkedResponse { + public: + explicit ChunkedResponse(httpd_req_t* req) : req_(req) {} + ChunkedResponse(const ChunkedResponse&) = delete; + ChunkedResponse& operator=(const ChunkedResponse&) = delete; + + void write(std::string_view chunk) { + while (!finished_ && result_ == ESP_OK && !chunk.empty()) { + const size_t count = std::min(chunk.size(), sizeof(buffer_) - size_); + std::memcpy(buffer_ + size_, chunk.data(), count); + size_ += count; + chunk.remove_prefix(count); + if (size_ == sizeof(buffer_)) flush(); + } + } + + esp_err_t finish() { + if (!finished_) { + flush(); + if (result_ == ESP_OK) result_ = httpd_resp_send_chunk(req_, nullptr, 0); + finished_ = true; + } + return result_; + } + + private: + void flush() { + if (result_ == ESP_OK && size_ > 0) { + result_ = httpd_resp_send_chunk(req_, buffer_, size_); + size_ = 0; + } + } + + httpd_req_t* req_; + char buffer_[1024]; + size_t size_ = 0; + esp_err_t result_ = ESP_OK; + bool finished_ = false; +}; + class StreamingReader { public: explicit StreamingReader(httpd_req_t* req); diff --git a/src/api/adc_api.cpp b/src/api/adc_api.cpp index 4cbfb06e..9a9b499d 100644 --- a/src/api/adc_api.cpp +++ b/src/api/adc_api.cpp @@ -82,6 +82,7 @@ esp_err_t AdcApi::handleAdcPage(httpd_req_t* req) { } esp_err_t AdcApi::handleAdcRaw(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); if (!instance_->adc_.isRunning() && !instance_->adc_.begin()) { HttpUtils::sendErrorResponse(req, "500 Internal Server Error", "adc_raw", "ADC not running or failed to start"); @@ -132,27 +133,26 @@ esp_err_t AdcApi::handleAdcRaw(httpd_req_t* req) { httpd_resp_set_hdr(req, "X-ADC-Controller-Sample-Rate", tmp6); if (!instance_->adc_.streamRaw( - [req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + [&response](std::string_view chunk) { + response.write(chunk); }, sample_rate, samples_per_channel, channel_mask)) return ESP_FAIL; - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t AdcApi::handleAdcState(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); { - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); auto scope = writer.objectScope(); writer.writeField("running", instance_->adc_.isRunning()); } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t AdcApi::handleAdcEnable(httpd_req_t* req) { diff --git a/src/api/commands_api.cpp b/src/api/commands_api.cpp index ba787282..3a48fb37 100644 --- a/src/api/commands_api.cpp +++ b/src/api/commands_api.cpp @@ -83,13 +83,13 @@ esp_err_t CommandsApi::handleCommandsPage(httpd_req_t* req) { } esp_err_t CommandsApi::handleCommands(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - instance_->command_manager_.fetchCommands([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + instance_->command_manager_.fetchCommands([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t CommandsApi::handleCommandsEvaluate(httpd_req_t* req) { diff --git a/src/api/cron_api.cpp b/src/api/cron_api.cpp index 55a4a517..84304e04 100644 --- a/src/api/cron_api.cpp +++ b/src/api/cron_api.cpp @@ -37,13 +37,13 @@ esp_err_t CronApi::handleCronPage(httpd_req_t* req) { } esp_err_t CronApi::handleCron(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - instance_->cron_.fetchRules([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + instance_->cron_.fetchRules([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t CronApi::handleCronEvaluate(httpd_req_t* req) { diff --git a/src/api/devices_api.cpp b/src/api/devices_api.cpp index 15069b8b..27684a5c 100644 --- a/src/api/devices_api.cpp +++ b/src/api/devices_api.cpp @@ -32,13 +32,13 @@ esp_err_t DevicesApi::handleDevicesPage(httpd_req_t* req) { } esp_err_t DevicesApi::handleDevices(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - getEbusController().fetchDevices([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + getEbusController().fetchDevices([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t DevicesApi::handleDevicesScan(httpd_req_t* req) { diff --git a/src/api/logs_api.cpp b/src/api/logs_api.cpp index 55b19c92..f1c17816 100644 --- a/src/api/logs_api.cpp +++ b/src/api/logs_api.cpp @@ -44,6 +44,7 @@ esp_err_t LogsApi::handleLogsPage(httpd_req_t* req) { } esp_err_t LogsApi::handleLogs(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); uint64_t since_millis = 0; const size_t query_len = httpd_req_get_url_query_len(req); if (query_len > 0) { @@ -68,25 +69,25 @@ esp_err_t LogsApi::handleLogs(httpd_req_t* req) { httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); instance_->logger_.fetchLogs( - [req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + [&response](std::string_view chunk) { + response.write(chunk); }, since_millis); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t LogsApi::handleLogsTimeRelation(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - instance_->logger_.fetchTimeRelation([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + instance_->logger_.fetchTimeRelation([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t LogsApi::handleTap(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); uint64_t since_millis = 0; const size_t query_len = httpd_req_get_url_query_len(req); if (query_len > 0) { @@ -107,12 +108,11 @@ esp_err_t LogsApi::handleTap(httpd_req_t* req) { httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); DeviceStatus::monitor().fetchTap( - [req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + [&response](std::string_view chunk) { + response.write(chunk); }, since_millis); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } #endif diff --git a/src/api/metrics_api.cpp b/src/api/metrics_api.cpp index 75a9f55c..73f21bd1 100644 --- a/src/api/metrics_api.cpp +++ b/src/api/metrics_api.cpp @@ -32,13 +32,13 @@ esp_err_t MetricsApi::handleMetricsPage(httpd_req_t* req) { } esp_err_t MetricsApi::handleMetrics(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - getEbusController().fetchMetrics([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + getEbusController().fetchMetrics([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t MetricsApi::handleMetricsReset(httpd_req_t* req) { diff --git a/src/api/network_api.cpp b/src/api/network_api.cpp index 5e9ac4d4..1d65627d 100644 --- a/src/api/network_api.cpp +++ b/src/api/network_api.cpp @@ -31,10 +31,11 @@ bool NetworkApi::registerHandlers(httpd_handle_t server) { } esp_err_t NetworkApi::handleNetwork(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); { auto scope = writer.objectScope(); @@ -66,11 +67,11 @@ esp_err_t NetworkApi::handleNetwork(httpd_req_t* req) { appendSntpStatus(writer, DeviceStatus::config().sntp); } } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t NetworkApi::handleWifiScan(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); wifi_scan_config_t scan_config = {}; scan_config.show_hidden = true; scan_config.scan_type = WIFI_SCAN_TYPE_ACTIVE; @@ -96,8 +97,8 @@ esp_err_t NetworkApi::handleWifiScan(httpd_req_t* req) { httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); { @@ -150,9 +151,8 @@ esp_err_t NetworkApi::handleWifiScan(httpd_req_t* req) { writer.writeField("authMode", auth_mode); } } - httpd_resp_send_chunk(req, nullptr, 0); esp_wifi_clear_ap_list(); - return ESP_OK; + return response.finish(); } #endif diff --git a/src/api/status_api.cpp b/src/api/status_api.cpp index 27bde526..6fbda0ad 100644 --- a/src/api/status_api.cpp +++ b/src/api/status_api.cpp @@ -45,10 +45,11 @@ esp_err_t StatusApi::handleStatusPage(httpd_req_t* req) { #if defined(EBUS_INTERNAL) esp_err_t StatusApi::handleHealth(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); uint32_t total_messages = 0; @@ -91,8 +92,7 @@ esp_err_t StatusApi::handleHealth(httpd_req_t* req) { writer.writeField("print_drops", logger.getPrintDropCount()); } } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } #endif diff --git a/src/api/system_api.cpp b/src/api/system_api.cpp index 9126414a..675727a5 100644 --- a/src/api/system_api.cpp +++ b/src/api/system_api.cpp @@ -45,10 +45,11 @@ bool SystemApi::registerHandlers(httpd_handle_t server) { } esp_err_t SystemApi::handleSystem(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); { auto scope = writer.objectScope(); @@ -79,17 +80,17 @@ esp_err_t SystemApi::handleSystem(httpd_req_t* req) { } } } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } // Current values + hourly trend samples (leak/fragmentation instrument). // Object shape; the page renders current plus the trend table. esp_err_t SystemApi::handleHeap(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); SystemMonitor::HeapSample trend[SystemMonitor::heap_trend_capacity]; size_t n = DeviceStatus::monitor().fetchHeapTrend( @@ -115,17 +116,17 @@ esp_err_t SystemApi::handleHeap(httpd_req_t* req) { } } } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } // Per-task stacks + CPU shares (who starves the bus thread?). // Object of two arrays; the page renders one table each. esp_err_t SystemApi::handleTasks(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); }); { auto scope = writer.objectScope(); @@ -259,8 +260,7 @@ esp_err_t SystemApi::handleTasks(httpd_req_t* req) { } } } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } #endif diff --git a/src/api/values_api.cpp b/src/api/values_api.cpp index c9829312..2b0e8131 100644 --- a/src/api/values_api.cpp +++ b/src/api/values_api.cpp @@ -41,13 +41,13 @@ esp_err_t ValuesApi::handleValuesPage(httpd_req_t* req) { } esp_err_t ValuesApi::handleValues(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - instance_->command_manager_.fetchValues([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + instance_->command_manager_.fetchValues([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t ValuesApi::handleValuesWrite(httpd_req_t* req) { diff --git a/src/config/config_manager.cpp b/src/config/config_manager.cpp index eea06a22..1795eee6 100644 --- a/src/config/config_manager.cpp +++ b/src/config/config_manager.cpp @@ -285,18 +285,20 @@ esp_err_t handleConfigDrift(httpd_req_t* req) { AppConfig::collectDrift(DeviceStatus::config(), requested, keys); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - ebus::detail::JsonWriter writer([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); - }); - auto root = writer.objectScope(); - writer.writeField("staged", !keys.empty()); - writer.appendKey("keys"); + HttpUtils::ChunkedResponse response(req); { - auto arr = writer.arrayScope(); - for (const auto& k : keys) writer.writeValue(std::string_view(k)); + ebus::detail::JsonWriter writer([&response](std::string_view chunk) { + response.write(chunk); + }); + auto root = writer.objectScope(); + writer.writeField("staged", !keys.empty()); + writer.appendKey("keys"); + { + auto arr = writer.arrayScope(); + for (const auto& k : keys) writer.writeValue(std::string_view(k)); + } } - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } } // namespace @@ -436,13 +438,13 @@ bool ConfigManager::writeConfigJson(std::string_view body, std::string& error) { } esp_err_t ConfigManager::handleGet(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - fetchConfig([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + fetchConfig([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } esp_err_t ConfigManager::handleSet(httpd_req_t* req) { diff --git a/src/network/http.cpp b/src/network/http.cpp index 9a6c7990..18b3c542 100644 --- a/src/network/http.cpp +++ b/src/network/http.cpp @@ -105,8 +105,8 @@ void setupHttpHandlers() { config.stack_size = 8192; config.lru_purge_enable = true; config.max_open_sockets = 2; - config.recv_wait_timeout = 10; - config.send_wait_timeout = 10; + config.recv_wait_timeout = 5; + config.send_wait_timeout = 3; if (httpd_start(&config_server, &config) != ESP_OK) { logger.error("Failed to start HTTP server"); diff --git a/src/system/upgrade_manager.cpp b/src/system/upgrade_manager.cpp index 2cc0d80a..0b27ec3f 100644 --- a/src/system/upgrade_manager.cpp +++ b/src/system/upgrade_manager.cpp @@ -229,13 +229,13 @@ void UpgradeManager::fetchStatus(const ebus::JsonChunkVisitor& visitor) { } esp_err_t UpgradeManager::handleStatus(httpd_req_t* req) { + HttpUtils::ChunkedResponse response(req); httpd_resp_set_type(req, "application/json;charset=utf-8"); HttpUtils::applyCustomHeaders(req); - DeviceStatus::fetchStatus([req](std::string_view chunk) { - httpd_resp_send_chunk(req, chunk.data(), chunk.size()); + DeviceStatus::fetchStatus([&response](std::string_view chunk) { + response.write(chunk); }); - httpd_resp_send_chunk(req, nullptr, 0); - return ESP_OK; + return response.finish(); } bool UpgradeManager::performHttpUpgrade(const std::string& url, diff --git a/test_host/api/logs_api_tests.cpp b/test_host/api/logs_api_tests.cpp index d74f6429..4cf6eaed 100644 --- a/test_host/api/logs_api_tests.cpp +++ b/test_host/api/logs_api_tests.cpp @@ -70,3 +70,13 @@ TEST_CASE("LogsApi tap endpoint forwards since and streams tap JSON", REQUIRE(body == R"({"tap":[],"dropped":0,"capacity":0})"); REQUIRE(req.chunks.back().empty()); } + +TEST_CASE("LogsApi returns a failed send to the server without retrying", "[logs_api]") { + Logger logger_inst; + LogsApi api(logger_inst); + httpd_req_t req{}; + req.chunk_fail_after = 0; + REQUIRE(LogsApi::handleLogs(&req) == ESP_FAIL); + REQUIRE(req.chunk_send_calls == 1); + REQUIRE(req.chunks.empty()); +} diff --git a/test_host/config/config_manager_tests.cpp b/test_host/config/config_manager_tests.cpp index 4ab26781..9c6121cd 100644 --- a/test_host/config/config_manager_tests.cpp +++ b/test_host/config/config_manager_tests.cpp @@ -5,6 +5,7 @@ #include "app/app_limits.hpp" #include "config/app_config_loader.hpp" #include "config/config_manager.hpp" +#include "api/http_stub_state.hpp" // The ConfigManager instance backing these tests. NVS itself is the // in-memory host mock (test_host/mocks/config/nvs_mock.cpp), so each case @@ -161,3 +162,26 @@ TEST_CASE("Empty stored configuration still supplies page defaults", REQUIRE(out.find("\"ebusAddress\":{\"default\":\"ff\"}") != std::string::npos); } + +TEST_CASE("Configuration drift JSON closes before the HTTP terminator", "[config_nvs]") { + config_manager.resetConfig(); + HostHttpStub::clearRoutes(); + ConfigManager::registerHandlers(); + esp_err_t (*handler)(httpd_req_t*) = nullptr; + for (const auto& route : HostHttpStub::routes()) { + if (route.uri == "/api/v1/app/config/drift") handler = route.handler; + } + REQUIRE(handler != nullptr); + httpd_req_t req{}; + REQUIRE(handler(&req) == ESP_OK); + std::string body; + for (const auto& chunk : req.chunks) body += chunk; + REQUIRE(ebus::detail::JsonReader::validate(body)); + REQUIRE(req.chunks.back().empty()); + + httpd_req_t failed{}; + failed.chunk_fail_after = 0; + REQUIRE(handler(&failed) == ESP_FAIL); + REQUIRE(failed.chunk_send_calls == 1); + REQUIRE(failed.chunks.empty()); +} diff --git a/test_host/mocks/esp_http_server.h b/test_host/mocks/esp_http_server.h index eeafc2dc..a77963ac 100644 --- a/test_host/mocks/esp_http_server.h +++ b/test_host/mocks/esp_http_server.h @@ -45,6 +45,8 @@ typedef struct httpd_req { int recv_fail_after = -1; int recv_failure_result = -1; size_t received_bytes = 0; + int chunk_fail_after = -1; + size_t chunk_send_calls = 0; } httpd_req_t; typedef struct httpd_data* httpd_handle_t; typedef struct httpd_uri httpd_uri_t; @@ -82,6 +84,10 @@ inline esp_err_t httpd_resp_send(httpd_req_t* req, const char* buf, inline esp_err_t httpd_resp_send_chunk(httpd_req_t* req, const char* buf, ssize_t buf_len) { if (req == nullptr) return ESP_OK; + const size_t call = req->chunk_send_calls++; + if (req->chunk_fail_after >= 0 && + call >= static_cast(req->chunk_fail_after)) + return ESP_FAIL; if (buf == nullptr || buf_len == 0) { req->chunks.emplace_back(); return ESP_OK; diff --git a/test_host/network/http_utils_tests.cpp b/test_host/network/http_utils_tests.cpp index 8e6e8946..6ebd333b 100644 --- a/test_host/network/http_utils_tests.cpp +++ b/test_host/network/http_utils_tests.cpp @@ -281,3 +281,48 @@ TEST_CASE("registerRoute reports HTTP registration failures", "[http_utils]") { std::string::npos); HttpUtilsTestState::route_result = ESP_OK; } + +TEST_CASE("Chunked responses combine fragments without changing payload bytes", "[http_utils]") { + httpd_req_t req{}; + HttpUtils::ChunkedResponse response(&req); + std::string payload(3500, 'x'); + payload[0] = payload[1023] = payload[1024] = payload.back() = '\0'; + for (size_t offset = 0; offset < payload.size(); offset += 7) { + response.write(std::string_view(payload).substr(offset, 7)); + } + REQUIRE(response.finish() == ESP_OK); + std::string received; + for (const auto& chunk : req.chunks) received += chunk; + REQUIRE(received == payload); + REQUIRE(req.chunks.size() == 5); // Four data chunks and one terminator. + REQUIRE(req.chunk_send_calls == 5); + REQUIRE(req.chunks.back().empty()); + REQUIRE(response.finish() == ESP_OK); + REQUIRE(req.chunks.size() == 5); +} + +TEST_CASE("Chunked responses never retry a failed socket write", "[http_utils]") { + for (int successful_sends : {0, 1, 2}) { + httpd_req_t req{}; + req.chunk_fail_after = successful_sends; + HttpUtils::ChunkedResponse response(&req); + for (int i = 0; i < 100; ++i) response.write(std::string(100, 'x')); + REQUIRE(response.finish() == ESP_FAIL); + REQUIRE(req.chunk_send_calls == static_cast(successful_sends + 1)); + REQUIRE(req.chunks.size() == static_cast(successful_sends)); + response.write("later data"); + REQUIRE(response.finish() == ESP_FAIL); + REQUIRE(req.chunk_send_calls == static_cast(successful_sends + 1)); + } +} + +TEST_CASE("Chunked responses propagate failures during final flush or termination", "[http_utils]") { + for (int successful_sends : {0, 1}) { + httpd_req_t req{}; + req.chunk_fail_after = successful_sends; + HttpUtils::ChunkedResponse response(&req); + response.write("small response"); + REQUIRE(response.finish() == ESP_FAIL); + REQUIRE(req.chunk_send_calls == static_cast(successful_sends + 1)); + } +}