From 300eea034bdcc7e46cd68bcbbd55b6a155faf592 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 29 Jan 2026 23:26:53 -0600 Subject: [PATCH] handle trailing garbage --- esphome/components/time/posix_tz.cpp | 47 ++++++++++-------- esphome/components/time/real_time_clock.cpp | 54 ++++++++++++++++++--- esphome/components/time/real_time_clock.h | 9 ++-- tests/components/time/posix_tz_parser.cpp | 32 ++++++++++-- 4 files changed, 109 insertions(+), 33 deletions(-) diff --git a/esphome/components/time/posix_tz.cpp b/esphome/components/time/posix_tz.cpp index 7a6d77db61..d400cce454 100644 --- a/esphome/components/time/posix_tz.cpp +++ b/esphome/components/time/posix_tz.cpp @@ -379,12 +379,15 @@ bool parse_posix_tz(const char *tz_string, ParsedTimezone &result) { return false; } - if (!internal::skip_tz_name(p)) { - return true; // No valid DST name, no DST + // Check if there's something that looks like a DST name start + // (letter or angle bracket). If not, treat as trailing garbage and return success. + if (!std::isalpha(static_cast(*p)) && *p != '<') { + return true; // No DST, trailing characters ignored } - // We have a DST name - result.has_dst = true; + if (!internal::skip_tz_name(p)) { + return false; // Invalid DST name (started but malformed) + } // Optional DST offset (default is std - 1 hour) if (*p && *p != ',' && (std::isdigit(static_cast(*p)) || *p == '+' || *p == '-')) { @@ -393,23 +396,29 @@ bool parse_posix_tz(const char *tz_string, ParsedTimezone &result) { result.dst_offset_seconds = result.std_offset_seconds - 3600; } - // Parse DST rules if present (POSIX requires both start and end if any rules specified) - if (*p == ',') { - p++; - if (!internal::parse_dst_rule(p, result.dst_start)) { - return false; - } - - // Second rule is required per POSIX - if (*p != ',') { - return false; - } - p++; - if (!internal::parse_dst_rule(p, result.dst_end)) { - return false; - } + // Parse DST rules (required when DST name is present) + if (*p != ',') { + // DST name without rules - treat as no DST since we can't determine transitions + return true; } + p++; + if (!internal::parse_dst_rule(p, result.dst_start)) { + return false; + } + + // Second rule is required per POSIX + if (*p != ',') { + return false; + } + p++; + if (!internal::parse_dst_rule(p, result.dst_end)) { + return false; + } + + // Only set has_dst after successfully parsing both rules + result.has_dst = true; + return true; } diff --git a/esphome/components/time/real_time_clock.cpp b/esphome/components/time/real_time_clock.cpp index 034f83b394..870d689195 100644 --- a/esphome/components/time/real_time_clock.cpp +++ b/esphome/components/time/real_time_clock.cpp @@ -23,6 +23,42 @@ static const char *const TAG = "time"; RealTimeClock::RealTimeClock() = default; +// Helper to format a DST rule for logging +#ifdef USE_TIME_TIMEZONE +static void format_dst_rule(const DSTRule &rule, char *buf, size_t buf_size) { + // Format rule part + int pos = 0; + switch (rule.type) { + case DSTRuleType::MONTH_WEEK_DAY: + pos = snprintf(buf, buf_size, "M%d.%d.%d", rule.month, rule.week, rule.day_of_week); + break; + case DSTRuleType::JULIAN_NO_LEAP: + pos = snprintf(buf, buf_size, "J%d", rule.day); + break; + case DSTRuleType::DAY_OF_YEAR: + pos = snprintf(buf, buf_size, "%d", rule.day); + break; + } + + // Format time part + int32_t time_secs = rule.time_seconds; + char sign = time_secs < 0 ? '-' : '/'; + if (time_secs < 0) + time_secs = -time_secs; + int hours = time_secs / 3600; + int mins = (time_secs % 3600) / 60; + int secs = time_secs % 60; + + if (secs != 0) { + snprintf(buf + pos, buf_size - pos, "%c%d:%02d:%02d", sign, hours, mins, secs); + } else if (mins != 0) { + snprintf(buf + pos, buf_size - pos, "%c%d:%02d", sign, hours, mins); + } else { + snprintf(buf + pos, buf_size - pos, "%c%d", sign, hours); + } +} +#endif + void RealTimeClock::dump_config() { #ifdef USE_TIME_TIMEZONE int std_hours = -this->parsed_tz_.std_offset_seconds / 3600; @@ -30,11 +66,10 @@ void RealTimeClock::dump_config() { ESP_LOGCONFIG(TAG, "Timezone: UTC%+d:%02d", std_hours, std_mins); if (this->parsed_tz_.has_dst) { int dst_hours = -this->parsed_tz_.dst_offset_seconds / 3600; - ESP_LOGCONFIG(TAG, " DST: UTC%+d, M%d.%d.%d/%" PRId32 " - M%d.%d.%d/%" PRId32, dst_hours, - this->parsed_tz_.dst_start.month, this->parsed_tz_.dst_start.week, - this->parsed_tz_.dst_start.day_of_week, this->parsed_tz_.dst_start.time_seconds / 3600, - this->parsed_tz_.dst_end.month, this->parsed_tz_.dst_end.week, this->parsed_tz_.dst_end.day_of_week, - this->parsed_tz_.dst_end.time_seconds / 3600); + char start_buf[24], end_buf[24]; + format_dst_rule(this->parsed_tz_.dst_start, start_buf, sizeof(start_buf)); + format_dst_rule(this->parsed_tz_.dst_end, end_buf, sizeof(end_buf)); + ESP_LOGCONFIG(TAG, " DST: UTC%+d, %s - %s", dst_hours, start_buf, end_buf); } #endif auto time = this->now(); @@ -95,7 +130,14 @@ void RealTimeClock::synchronize_epoch_(uint32_t epoch) { #ifdef USE_TIME_TIMEZONE void RealTimeClock::apply_timezone_(const char *tz) { - // Parse the POSIX TZ string using our custom parser to avoid pulling in scanf (~7.6KB) + // Handle null input + if (tz == nullptr) { + ESP_LOGW(TAG, "Failed to parse timezone: (null)"); + this->parsed_tz_ = ParsedTimezone{}; + return; + } + + // Parse the POSIX TZ string using our custom parser if (!parse_posix_tz(tz, this->parsed_tz_)) { ESP_LOGW(TAG, "Failed to parse timezone: %s", tz); // Reset to UTC on parse failure diff --git a/esphome/components/time/real_time_clock.h b/esphome/components/time/real_time_clock.h index 0f9ec1e993..5b760d23c6 100644 --- a/esphome/components/time/real_time_clock.h +++ b/esphome/components/time/real_time_clock.h @@ -26,9 +26,12 @@ class RealTimeClock : public PollingComponent { /// Set the time zone from a POSIX TZ string. void set_timezone(const char *tz) { this->apply_timezone_(tz); } - /// Set the time zone from a null-terminated string with known length. - /// The length parameter is ignored since our parser uses null-terminated strings. - void set_timezone(const char *tz, size_t /*len*/) { this->apply_timezone_(tz); } + /// Set the time zone from a character buffer with known length. + /// The buffer does not need to be null-terminated; it will be copied. + void set_timezone(const char *tz, size_t len) { + std::string tz_str(tz, len); + this->apply_timezone_(tz_str.c_str()); + } /// Set the time zone from a std::string. void set_timezone(const std::string &tz) { this->apply_timezone_(tz.c_str()); } diff --git a/tests/components/time/posix_tz_parser.cpp b/tests/components/time/posix_tz_parser.cpp index 73b84bed23..1a84a39835 100644 --- a/tests/components/time/posix_tz_parser.cpp +++ b/tests/components/time/posix_tz_parser.cpp @@ -398,19 +398,19 @@ TEST(PosixTzParser, LowercaseJFormat) { TEST(PosixTzParser, DstNameWithoutRules) { ParsedTimezone tz; - // DST name present but no rules - should have has_dst=true with default offset + // DST name present but no rules - treat as no DST since we can't determine transitions ASSERT_TRUE(parse_posix_tz("EST5EDT", tz)); - EXPECT_TRUE(tz.has_dst); + EXPECT_FALSE(tz.has_dst); EXPECT_EQ(tz.std_offset_seconds, 5 * 3600); - EXPECT_EQ(tz.dst_offset_seconds, 4 * 3600); // Default: std - 1 hour } TEST(PosixTzParser, TrailingCharactersIgnored) { ParsedTimezone tz; // Trailing characters after valid TZ should be ignored (parser stops at end of valid input) // This matches libc behavior - ASSERT_TRUE(parse_posix_tz("EST5", tz)); + ASSERT_TRUE(parse_posix_tz("EST5 extra garbage here", tz)); EXPECT_EQ(tz.std_offset_seconds, 5 * 3600); + EXPECT_FALSE(tz.has_dst); } TEST(PosixTzParser, PlainDay365LeapYear) { @@ -751,7 +751,29 @@ TEST(PosixTzParser, EpochToLocalDstTransition) { // Verification against libc // ============================================================================ -class LibcVerificationTest : public ::testing::TestWithParam> {}; +class LibcVerificationTest : public ::testing::TestWithParam> { + protected: + void SetUp() override { + // Save current TZ + const char *current_tz = getenv("TZ"); + saved_tz_ = current_tz ? current_tz : ""; + had_tz_ = current_tz != nullptr; + } + + void TearDown() override { + // Restore TZ + if (had_tz_) { + setenv("TZ", saved_tz_.c_str(), 1); + } else { + unsetenv("TZ"); + } + tzset(); + } + + private: + std::string saved_tz_; + bool had_tz_{false}; +}; TEST_P(LibcVerificationTest, MatchesLibc) { auto [tz_str, epoch] = GetParam();