chronotope / chrono

Date and time library for Rust
Other
3.3k stars 523 forks source link

Support for `24:00:00` in `DateTime` Parsing #1593

Open mre opened 3 months ago

mre commented 3 months ago

Description

Would it be possible to support parsing of ISO 8601 datetime strings that use 24:00:00 to represent midnight? Currently, attempting to parse such a string results in an error.

Current Behavior

The following code throws an error:

use chrono::prelude::*;

fn main() {
    "2014-11-28T24:00:00Z".parse::<DateTime<Utc>>().unwrap();
}

You can see this behavior in action here: Rust Playground Example

Expected Behavior

The code should successfully parse the datetime string, interpreting 24:00:00 as 00:00:00 of the next day (in this case, 2014-11-29T00:00:00Z).

Rationale

This is a valid ISO 8601 convention, as described in the W3C XML Schema Definition Language (XSD) 1.1 Part 2: Datatypes specification:

W3C XML Schema: Datatypes

h -- represents a digit used in the time element "hour". The two digits in a hh format can have values from 0 to 24. If the value of the hour element is 24 then the values of the minutes element and the seconds element must be 00 and 00.

Additional Context

It's worth noting that the library already allows a value of 60 for the second time component (presumably to account for leap seconds), so there is some precedent for handling edge cases like these.

Proposed Solution

Implement a check in the datetime parsing logic that:

  1. Allows "24:00:00" as a valid time.
  2. Converts "24:00:00" to "00:00:00" of the next day.
  3. Ensures that when hour is 24, minutes and seconds are both 00.

Important Considerations

Implementing this change would be a breaking change in the public API. We need to carefully consider potential backwards compatibility issues.

djc commented 3 months ago

Wow, that's a pretty weird convention. I'm open to this -- I don't think merely starting to accept this would have to be considered a breaking change. Would you be able to submit a PR?

pitdicker commented 3 months ago

It has been a long documented guarantee of chrono that it doesn't parse 24:00:00, and I suppose there are uses that depend on that. Are you sure this is acceptable?

For the complete ISO 8601 parsers in https://github.com/chronotope/chrono/pull/1143 I added support for this case only when parsing an ISO 8601 formatted datetime.