Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mre opened this issue Jul 5, 2024 · 2 comments
Open

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

mre opened this issue Jul 5, 2024 · 2 comments

Comments

@mre
Copy link

mre commented Jul 5, 2024

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
Copy link
Member

djc commented Jul 5, 2024

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
Copy link
Collaborator

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 #1143 I added support for this case only when parsing an ISO 8601 formatted datetime.

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

No branches or pull requests

3 participants