From b2b98ef660ad7c7f9582818e75a45d04af4176d0 Mon Sep 17 00:00:00 2001 From: HeatMind Date: Tue, 15 Sep 2026 09:13:24 +0200 Subject: [PATCH] fix(weblog): guard against a weblog_buffer of 0 (pop_front on empty deque) WebLogService::operator<< trims with `log_messages_.size() >= limit_log_messages_`. size() is unsigned, so with limit_log_messages_ == 0 the condition holds even for an empty deque and pop_front() runs on it - undefined behaviour, in practice heap corruption. On a PSRAM board the stored value survives a reboot, so the device crashes again as soon as the first log message is written. Nothing clamps the value on the way in: start() copies settings.weblog_buffer with no lower bound, the PSRAM branch of operator<< passes it straight into limit_log_messages_, and ArduinoJson's | in WebSettings::update() only substitutes the default for a missing or type-incompatible key - a stored 0 is a valid uint16_t. - operator<<: skip the trim loop on an empty deque - start(): keep the existing default when the stored value is 0 - getSetValues(): a missing or zero max_messages keeps the current size instead of storing 0 - WebSettings::update(): repair a stored 0, and derive the psram/no-psram default once instead of twice Refs #3237 --- src/web/WebLogService.cpp | 26 ++++++++++++++++++-------- src/web/WebSettingsService.cpp | 10 ++++++---- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/web/WebLogService.cpp b/src/web/WebLogService.cpp index c61db310e..1069607ce 100644 --- a/src/web/WebLogService.cpp +++ b/src/web/WebLogService.cpp @@ -44,10 +44,13 @@ void WebLogService::begin() { // apply the user settings void WebLogService::start() { EMSESP::webSettingsService.read([&](WebSettings & settings) { - maximum_log_messages_ = settings.weblog_buffer; - limit_log_messages_ = maximum_log_messages_; - compact_ = settings.weblog_compact; - level_ = (uuid::log::Level)settings.weblog_level; + // a stored value of 0 would make limit_log_messages_ 0, see the guard in operator<< + if (settings.weblog_buffer) { + maximum_log_messages_ = settings.weblog_buffer; + } + limit_log_messages_ = maximum_log_messages_; + compact_ = settings.weblog_compact; + level_ = (uuid::log::Level)settings.weblog_level; }); uuid::log::Logger::register_handler(this, level_); if (level_ == uuid::log::Level::OFF) { @@ -80,7 +83,9 @@ void WebLogService::operator<<(std::shared_ptr message) { } } #endif - while (log_messages_.size() >= limit_log_messages_) { + // size() is unsigned, so with limit_log_messages_ == 0 this condition is always true + // and pop_front() would run on an empty deque (undefined behaviour) + while (!log_messages_.empty() && log_messages_.size() >= limit_log_messages_) { log_messages_.pop_front(); } @@ -210,9 +215,14 @@ void WebLogService::getSetValues(AsyncWebServerRequest * request, JsonVariant js } // POST - write the settings - level_ = json["level"]; - maximum_log_messages_ = json["max_messages"]; - compact_ = json["compact"]; + level_ = json["level"]; + // keep the current size if the key is missing or 0 - a 0 would be stored and make + // limit_log_messages_ 0 on the next boot + const size_t max_messages = json["max_messages"] | maximum_log_messages_; + if (max_messages) { + maximum_log_messages_ = max_messages; + } + compact_ = json["compact"]; uuid::log::Logger::register_handler(this, level_); if (level_ == uuid::log::Level::OFF) { diff --git a/src/web/WebSettingsService.cpp b/src/web/WebSettingsService.cpp index e46d1e093..6fe9af679 100644 --- a/src/web/WebSettingsService.cpp +++ b/src/web/WebSettingsService.cpp @@ -328,10 +328,12 @@ StateUpdateResult WebSettings::update(JsonObject root, WebSettings & settings) { settings.email_subject = root["email_subject"] | FACTORY_EMAIL_SUBJECT; // if no psram limit weblog buffer to 25 messages - if (EMSESP::system_.PSram() > 0) { - settings.weblog_buffer = root["weblog_buffer"] | EMSESP_DEFAULT_WEBLOG_BUFFER; - } else { - settings.weblog_buffer = root["weblog_buffer"] | 25; // limit to 25 messages if no psram + const uint16_t weblog_buffer_default = (EMSESP::system_.PSram() > 0) ? EMSESP_DEFAULT_WEBLOG_BUFFER : 25; + settings.weblog_buffer = root["weblog_buffer"] | weblog_buffer_default; + // a stored 0 is a valid uint16 and passes the | default above, but would make + // limit_log_messages_ 0 in WebLogService + if (settings.weblog_buffer == 0) { + settings.weblog_buffer = weblog_buffer_default; } // save the settings if changed from the webUI