You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2026/10/21 07:23:33 UTC
(arrow) branch main updated: GH-54643: [C++] Reject impossible dates and a zero day in the strptime timestamp parser (#54657)
This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 6e4fa50fb02 GH-54643: [C++] Reject impossible dates and a zero day in the strptime timestamp parser (#54657)
6e4fa50fb02 is described below
commit 6e4fa50fb02112904d54c7ec685ec9e8f7932bba
Author: Pearu Peterson <pe...@gmail.com>
AuthorDate: Wed Oct 21 10:23:25 2026 +0300
GH-54643: [C++] Reject impossible dates and a zero day in the strptime timestamp parser (#54657)
### Rationale for this change
`strptime` checks the day of month against 31 at most, and some implementations (macOS, the vendored musl used on Windows) accept day `00`, and the vendored musl also month `00`. The parser rolled an impossible date such as February 30 over into the next month, took a zero day as the first of the month, and a zero month as December of the previous year.
### What changes are included in this PR?
The parser rejects the parsed date if it is not valid, as the ISO-8601 parser does. To tell a zero day from an omitted one, the day is initialized to the first before parsing, instead of being raised to the first afterwards. The two fixes are in one PR because the zero-day fix relies on the date check. The vendored musl `strptime` also gets back the unsigned range check that upstream musl has, so it rejects day and month `00` itself.
### Are these changes tested?
Yes, in `TimestampParser.StrptimeParser` (impossible dates, a zero day, a zero month, and February 29 of a leap year) and `TimestampParser.StrptimeWithoutDay` (a format without a day directive still gives the first of the month). The zero day and zero month cases only fail without the fix on platforms whose `strptime` accepts them.
### Are there any user-facing changes?
Impossible dates, a zero day and a zero month no longer parse with `strptime` formats.
### AI usage disclosure
Developed with AI assistance (Claude Code): the fix, the tests and this description were AI-generated under my direction and reviewed line by line by me. I own and can debug these changes. No copyrighted material is included.
* GitHub Issue: #54643
Authored-by: Pearu Peterson <pe...@gmail.com>
Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
cpp/src/arrow/util/value_parsing.h | 16 +++++++++++++---
cpp/src/arrow/util/value_parsing_test.cc | 24 ++++++++++++++++++++++--
cpp/src/arrow/vendored/musl/strptime.c | 2 +-
3 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h
index 195cdc843ac..1b300074851 100644
--- a/cpp/src/arrow/util/value_parsing.h
+++ b/cpp/src/arrow/util/value_parsing.h
@@ -815,6 +815,9 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length,
std::string clean_copy(buf, length);
struct tm result;
memset(&result, 0, sizeof(struct tm));
+ // Default the day to the first for formats without a day directive, so that
+ // an explicitly parsed zero day is not mistaken for an omitted one
+ result.tm_mday = 1;
#ifdef _WIN32
char* ret = arrow_strptime(clean_copy.c_str(), format, &result);
#else
@@ -826,10 +829,17 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length,
if (!allow_trailing_chars && static_cast<size_t>(ret - clean_copy.c_str()) != length) {
return false;
}
+ // Reject impossible dates such as February 30 or a zero day, which strptime
+ // may accept (the day of month is checked against 31 at most), instead of
+ // rolling them over into the next month; consistent with the ISO-8601 parser.
+ const arrow_vendored::date::year_month_day ymd =
+ arrow_vendored::date::year(result.tm_year + 1900) / (result.tm_mon + 1) /
+ result.tm_mday;
+ if (!ymd.ok()) {
+ return false;
+ }
// ignore the time part
- arrow_vendored::date::sys_seconds secs =
- arrow_vendored::date::sys_days(arrow_vendored::date::year(result.tm_year + 1900) /
- (result.tm_mon + 1) / std::max(result.tm_mday, 1));
+ arrow_vendored::date::sys_seconds secs = arrow_vendored::date::sys_days(ymd);
if (!ignore_time_in_day) {
secs += (std::chrono::hours(result.tm_hour) + std::chrono::minutes(result.tm_min) +
std::chrono::seconds(result.tm_sec));
diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc
index 6eb387fc031..69baba169ff 100644
--- a/cpp/src/arrow/util/value_parsing_test.cc
+++ b/cpp/src/arrow/util/value_parsing_test.cc
@@ -856,7 +856,8 @@ TEST(TimestampParser, StrptimeParser) {
};
std::vector<Case> cases = {{"5/31/2000 12:34:56", "2000-05-31 12:34:56"},
- {"5/31/2000 00:00:00", "2000-05-31 00:00:00"}};
+ {"5/31/2000 00:00:00", "2000-05-31 00:00:00"},
+ {"2/29/2000 12:34:56", "2000-02-29 12:34:56"}};
for (auto unit : TimeUnit::values()) {
for (const auto& case_ : cases) {
@@ -869,7 +870,17 @@ TEST(TimestampParser, StrptimeParser) {
}
// Unparseable strings
- std::vector<std::string> unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6"};
+ std::vector<std::string> 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"};
for (auto& value : unparseables) {
int64_t dummy;
ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, &dummy));
@@ -908,6 +919,15 @@ TEST(TimestampParser, StrptimeLocaleDirectives) {
}
}
+TEST(TimestampParser, StrptimeWithoutDay) {
+ // A format without a day directive gives the first of the month
+ auto parser = TimestampParser::MakeStrptime("%Y-%m");
+ int64_t converted, expected;
+ ASSERT_TRUE((*parser)("2000-02", 7, TimeUnit::SECOND, &converted));
+ ASSERT_TRUE(ParseTimestampISO8601("2000-02-01", 10, TimeUnit::SECOND, &expected));
+ ASSERT_EQ(expected, converted);
+}
+
TEST(TimestampParser, StrptimeZoneOffset) {
if (!kStrptimeSupportsZone) {
GTEST_SKIP() << "strptime does not support %z on this platform";
diff --git a/cpp/src/arrow/vendored/musl/strptime.c b/cpp/src/arrow/vendored/musl/strptime.c
index eb325108b8a..f592f79600d 100644
--- a/cpp/src/arrow/vendored/musl/strptime.c
+++ b/cpp/src/arrow/vendored/musl/strptime.c
@@ -212,7 +212,7 @@ char *strptime(const char *__restrict s, const char *__restrict f, struct tm *__
*dest = 0;
for (i=1; i<=min+range && isdigit(*s); i*=10)
*dest = *dest * 10 + *s++ - '0';
- if (*dest - min >= range) return 0;
+ if ((unsigned)(*dest - min) >= (unsigned)range) return 0;
*dest -= adj;
switch((char *)dest - (char *)tm) {
case offsetof(struct tm, tm_yday):