Skip to content

time: return an error from parse functions for a day past the end of the month - #29228

Merged
medvednikov merged 1 commit into
vlang:masterfrom
Arthur031221:time-parse-invalid-day
Oct 2, 2026
Merged

medvednikov merged 1 commit into
vlang:masterfrom
Arthur031221:time-parse-invalid-day

Conversation

@Arthur031221

Copy link
Copy Markdown
Contributor

Anyone who parses untrusted dates (time.parse, parse_rfc3339, parse_iso8601, parse_rfc2822, and through them json2 and toml decoding of time fields) got a process panic on an impossible date such as 2024-02-30, instead of an error they can handle.

time.parse and the RFC 3339, ISO 8601 and RFC 2822 parsers only checked that the day was between 1 and 31 (or not at all), then passed the fields to time.new, which panics on an invalid date. parse_format already rejects these inputs with an error.

Before, on this branch's parent:

time.parse('2024-02-30 10:00:00')            -> V panic: invalid time: day must be between 1 and 29 for year 2024, month 2
time.parse_rfc3339('2024-02-30T10:00:00Z')   -> same panic
time.parse_iso8601('2024-02-30T10:00:00Z')   -> same panic
time.parse_rfc2822('Thu, 30 Feb 2024 10:00:00 +0100') -> same panic

After, each returns the same message as an error.

The validation in normalize_new_time moves into check_new_time, which returns an error. normalize_new_time panics with the same text as before, so public time.new and Time.new behave exactly as they did. A private new_checked wraps it for the parse functions.

Tests: vlib/time/parse_test.v gains cases for Feb 30, Feb 29 in a non-leap year, Apr 31, the three other parsers, and last valid days. The file panics on the parent commit and passes with the change. ./v test vlib/time passes except parse_autofree_test.v, which fails identically without the change here with "cannot find a writable cache for the V3 ownership compiler". v fmt -verify is clean on the touched files.

…the month

time.parse, parse_rfc3339, parse_iso8601 and parse_rfc2822 only checked
that the day was between 1 and 31, then passed the fields to time.new,
which panics on an impossible date. Input such as '2024-02-30 10:00:00'
or '2024-04-31T10:00:00Z' therefore crashed the program instead of
returning an error, unlike parse_format which already rejects it.

Split the validation out of new into check_new_time, and use a new
new_checked wrapper in the parse functions so the same message is
returned as an error.

@medvednikov medvednikov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source review of aa90c05d: no concrete defect found. Moving field validation into check_new_time preserves the public Time.new/time.new panic behavior through normalize_new_time, while the new private new_checked lets the parsing APIs propagate invalid calendar dates as errors. parse_rfc2822 inherits the fix through parse, and the RFC3339/ISO8601 offset paths validate the wall-clock fields before constructing the result.

I did not use CI as validation: the workflow runs for this head are currently action_required rather than executed.

@medvednikov medvednikov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head aa90c05. No actionable findings. Date parsing now propagates day/month validation errors consistently through RFC3339 offsets, ISO8601, and RFC2822; time.new and Time.new retain their existing panic behavior and message. Exact-head ./v self succeeded; ./v -cc clang -silent vlib/time/parse_test.v passed, including the new invalid-day, leap-year, offset, and parser regressions. The broader time suite also has passing coverage; exact-head GitHub workflows await fork approval and have not executed.

@medvednikov
medvednikov merged commit 532f63b into vlang:master Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants