From e73642bdc473a99b7f0c1016fbbb9bfcfe90e990 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Mon, 19 Oct 2026 20:59:43 +0300 Subject: [PATCH] GH-54647: [C++] Reject leap seconds in the strptime timestamp parser strptime accepts a seconds value of 60 (glibc also 61), and the parser added it to the time of day, wrapping it into the next minute. Arrow cannot represent leap seconds, and the ISO-8601 parser rejects them, so reject them here too. The check applies only when the time of day is used, so Gandiva's to_date, which ignores the time, still accepts such values. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing.h | 6 ++++++ cpp/src/arrow/util/value_parsing_test.cc | 25 +++++++++++++----------- cpp/src/gandiva/to_date_holder_test.cc | 9 +++++++++ 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 1c2102fe370a..d845a71cf1f8 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -848,6 +848,12 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, // ignore the time part arrow_vendored::date::sys_seconds secs = arrow_vendored::date::sys_days(ymd); if (!ignore_time_in_day) { + // Reject leap seconds, which Arrow cannot represent, instead of adding them + // to the time of day (which wraps them into the next minute); consistent + // with the ISO-8601 parser. + if (result.tm_sec >= 60) { + return false; + } secs += (std::chrono::hours(result.tm_hour) + std::chrono::minutes(result.tm_min) + std::chrono::seconds(result.tm_sec)); #if !defined(_WIN32) && !defined(_AIX) diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index 875418f3195d..2a2d39777bf3 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -870,17 +870,20 @@ TEST(TimestampParser, StrptimeParser) { } // Unparseable strings - std::vector unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6", - // Impossible dates are rejected rather - // than rolled over into the next month - "2/30/2000 12:34:56", "4/31/2000 12:34:56", - "2/29/2001 12:34:56", - // A zero day is rejected rather than - // taken as the first of the month - "2/00/2000 12:34:56", - // A zero month is rejected rather than - // taken as December of the previous year - "0/15/2000 12:34:56"}; + std::vector unparseables = { + "foo", "5/1/2000", "5/1/2000 12:34:56:6", + // Impossible dates are rejected rather + // than rolled over into the next month + "2/30/2000 12:34:56", "4/31/2000 12:34:56", "2/29/2001 12:34:56", + // A zero day is rejected rather than + // taken as the first of the month + "2/00/2000 12:34:56", + // A zero month is rejected rather than + // taken as December of the previous year + "0/15/2000 12:34:56", + // Leap seconds are rejected, as by the + // ISO-8601 parser + "5/31/2000 12:30:60", "5/31/2000 23:59:60", "5/31/2000 23:59:61"}; for (auto& value : unparseables) { int64_t dummy; ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, &dummy)); diff --git a/cpp/src/gandiva/to_date_holder_test.cc b/cpp/src/gandiva/to_date_holder_test.cc index 1612d9b4f440..ada3e55d2fdc 100644 --- a/cpp/src/gandiva/to_date_holder_test.cc +++ b/cpp/src/gandiva/to_date_holder_test.cc @@ -85,6 +85,15 @@ TEST_F(TestToDateHolder, TestSimpleDateTime) { millis_since_epoch = to_date(&execution_context_, s.data(), (int)s.length(), true, &out_valid); EXPECT_EQ(millis_since_epoch, 534643200000); + + // A leap second does not make the value invalid, as the time is ignored + EXPECT_OK_AND_ASSIGN(auto to_date_holder24, + ToDateHolder::Make("YYYY-MM-DD HH24:MI:SS", 1)); + s = std::string("2020-12-31 23:59:60"); + millis_since_epoch = (*to_date_holder24)(&execution_context_, s.data(), (int)s.length(), + true, &out_valid); + EXPECT_TRUE(out_valid); + EXPECT_EQ(millis_since_epoch, 1609372800000); } TEST_F(TestToDateHolder, TestSimpleDate) {