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(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 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 unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6"}; + 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"}; 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):