From 496c09b333091c6c8a948621046d0eda58deafb0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 20 Dec 2025 11:00:03 -1000 Subject: [PATCH 1/4] bounds fixes --- esphome/components/syslog/esphome_syslog.cpp | 27 ++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/esphome/components/syslog/esphome_syslog.cpp b/esphome/components/syslog/esphome_syslog.cpp index 610a1243a3..02f2bbe8c5 100644 --- a/esphome/components/syslog/esphome_syslog.cpp +++ b/esphome/components/syslog/esphome_syslog.cpp @@ -42,31 +42,38 @@ void Syslog::log_(const int level, const char *tag, const char *message, size_t len -= 11; } - // Build syslog packet on stack - 508 is max UDP packet size + // Build syslog packet on stack (508 bytes chosen as practical limit for syslog over UDP) char packet[508]; size_t offset = 0; + size_t remaining = sizeof(packet); // Write PRI - int ret = snprintf(packet, sizeof(packet), "<%d>", pri); - if (ret > 0) + int ret = snprintf(packet, remaining, "<%d>", pri); + if (ret > 0 && static_cast(ret) < remaining) { offset = ret; + remaining -= ret; + } // Write timestamp directly into packet (RFC 5424: use "-" if time not valid) auto now = this->time_->now(); if (now.is_valid()) { - offset += now.strftime(packet + offset, sizeof(packet) - offset, "%b %e %H:%M:%S"); - } else { + size_t written = now.strftime(packet + offset, remaining, "%b %e %H:%M:%S"); + offset += written; + remaining -= written; + } else if (remaining > 0) { packet[offset++] = '-'; + remaining--; } // Write hostname, tag, and message - ret = snprintf(packet + offset, sizeof(packet) - offset, " %s %s: %.*s", App.get_name().c_str(), tag, (int) len, - message); - if (ret > 0) - offset += ret; + ret = snprintf(packet + offset, remaining, " %s %s: %.*s", App.get_name().c_str(), tag, (int) len, message); + if (ret > 0) { + // snprintf returns chars that would be written; clamp to actual buffer space + offset += std::min(static_cast(ret), remaining > 0 ? remaining - 1 : 0); + } if (offset > 0) { - this->parent_->send_packet(reinterpret_cast(packet), std::min(offset, sizeof(packet) - 1)); + this->parent_->send_packet(reinterpret_cast(packet), offset); } } From a0d1a10d17faebaa788c93e0b8b1e0fb049a1548 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 20 Dec 2025 11:00:31 -1000 Subject: [PATCH 2/4] Update tests/integration/test_syslog.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/integration/test_syslog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_syslog.py b/tests/integration/test_syslog.py index 552dbc610e..fee493c5cb 100644 --- a/tests/integration/test_syslog.py +++ b/tests/integration/test_syslog.py @@ -33,7 +33,7 @@ class ParsedSyslogMessage(TypedDict): # Example: <134>Dec 20 14:30:45 syslog-test app: [D][app:029]: Running... SYSLOG_PATTERN = re.compile( r"<(\d+)>" # PRI (priority = facility * 8 + severity) - r"(\S+ +\d+ \d+:\d+:\d+|-)" # TIMESTAMP (BSD format or NILVALUE "-") + r"(\S+ +\d+ \d+:\d+:\d+|-)" # TIMESTAMP (BSD-style "%b %e %H:%M:%S", e.g. "Dec 20 14:30:45", or NILVALUE "-") r" (\S+)" # HOSTNAME r" (\S+):" # TAG r" (.*)" # MESSAGE From a9c294bc03f5be956b1a68934c00795e6833d077 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 20 Dec 2025 11:09:52 -1000 Subject: [PATCH 3/4] copilot edge cases --- esphome/components/syslog/esphome_syslog.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/esphome/components/syslog/esphome_syslog.cpp b/esphome/components/syslog/esphome_syslog.cpp index 02f2bbe8c5..8e7f809ecc 100644 --- a/esphome/components/syslog/esphome_syslog.cpp +++ b/esphome/components/syslog/esphome_syslog.cpp @@ -47,19 +47,26 @@ void Syslog::log_(const int level, const char *tag, const char *message, size_t size_t offset = 0; size_t remaining = sizeof(packet); - // Write PRI + // Write PRI - abort if this fails as packet would be malformed int ret = snprintf(packet, remaining, "<%d>", pri); - if (ret > 0 && static_cast(ret) < remaining) { - offset = ret; - remaining -= ret; + if (ret <= 0 || static_cast(ret) >= remaining) { + return; } + offset = ret; + remaining -= ret; // Write timestamp directly into packet (RFC 5424: use "-" if time not valid) auto now = this->time_->now(); if (now.is_valid()) { size_t written = now.strftime(packet + offset, remaining, "%b %e %H:%M:%S"); - offset += written; - remaining -= written; + if (written > 0) { + offset += written; + remaining -= written; + } else if (remaining > 0) { + // strftime failed; write NILVALUE as fallback + packet[offset++] = '-'; + remaining--; + } } else if (remaining > 0) { packet[offset++] = '-'; remaining--; From 589942f52c6f0b12f0a1f469156386529817bc01 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 20 Dec 2025 11:11:54 -1000 Subject: [PATCH 4/4] simplify logic --- esphome/components/syslog/esphome_syslog.cpp | 16 ++++--------- tests/integration/fixtures/syslog.yaml | 5 ++++ tests/integration/test_syslog.py | 24 ++++++++++++++++++++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/esphome/components/syslog/esphome_syslog.cpp b/esphome/components/syslog/esphome_syslog.cpp index 8e7f809ecc..2fef1889f1 100644 --- a/esphome/components/syslog/esphome_syslog.cpp +++ b/esphome/components/syslog/esphome_syslog.cpp @@ -55,18 +55,12 @@ void Syslog::log_(const int level, const char *tag, const char *message, size_t offset = ret; remaining -= ret; - // Write timestamp directly into packet (RFC 5424: use "-" if time not valid) + // Write timestamp directly into packet (RFC 5424: use "-" if time not valid or strftime fails) auto now = this->time_->now(); - if (now.is_valid()) { - size_t written = now.strftime(packet + offset, remaining, "%b %e %H:%M:%S"); - if (written > 0) { - offset += written; - remaining -= written; - } else if (remaining > 0) { - // strftime failed; write NILVALUE as fallback - packet[offset++] = '-'; - remaining--; - } + size_t ts_written = now.is_valid() ? now.strftime(packet + offset, remaining, "%b %e %H:%M:%S") : 0; + if (ts_written > 0) { + offset += ts_written; + remaining -= ts_written; } else if (remaining > 0) { packet[offset++] = '-'; remaining--; diff --git a/tests/integration/fixtures/syslog.yaml b/tests/integration/fixtures/syslog.yaml index fee00eb8ff..df376087e3 100644 --- a/tests/integration/fixtures/syslog.yaml +++ b/tests/integration/fixtures/syslog.yaml @@ -16,6 +16,11 @@ api: "DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD" "EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE" "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"); + - service: log_short_message + then: + - lambda: |- + // Log a short message that should arrive complete (not truncated) + ESP_LOGI("shorttest", "BEGIN|SHORT_MESSAGE_CONTENT|FINISH"); logger: level: DEBUG diff --git a/tests/integration/test_syslog.py b/tests/integration/test_syslog.py index fee493c5cb..b31a19392c 100644 --- a/tests/integration/test_syslog.py +++ b/tests/integration/test_syslog.py @@ -258,3 +258,27 @@ async def test_syslog( assert "|END" not in trunc_msg, ( "Message should be truncated before END marker" ) + + # Test short message - should arrive complete (not truncated) + short_service = next( + (s for s in services if s.name == "log_short_message"), None + ) + assert short_service is not None, "log_short_message service not found" + + await client.execute_service(short_service, {}) + + try: + short_msg = await receiver.wait_for_pattern(r"shorttest.*BEGIN\|") + except TimeoutError: + pytest.fail( + f"Short test message not received. Got: {receiver.messages[-10:]}" + ) + + # Verify short message arrived complete with both markers + assert "BEGIN|" in short_msg, "Short message missing BEGIN marker" + assert "|FINISH" in short_msg, ( + f"Short message truncated unexpectedly: {short_msg}" + ) + assert "SHORT_MESSAGE_CONTENT" in short_msg, ( + f"Short message content missing: {short_msg}" + )