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

NaiveTime parsing bug when seconds are 60 #1610

Open
wcarmon opened this issue Aug 21, 2024 · 1 comment
Open

NaiveTime parsing bug when seconds are 60 #1610

wcarmon opened this issue Aug 21, 2024 · 1 comment

Comments

@wcarmon
Copy link

wcarmon commented Aug 21, 2024

Here's an minimal recreation

        // -- passes as expected since seconds are too large
        assert_eq!(None, NaiveTime::from_hms_opt(11, 22, 60));

        // -- bug:  should pass because seconds are too large
        assert!(NaiveTime::parse_from_str("11:22:60", "%H:%M:%S").is_err());

At a minimum, we have inconsistent handling for seconds that are too large.

Here's a playground link with the same assertions:

@djc
Copy link
Member

djc commented Aug 26, 2024

So this is an interesting case because there is some support for leap seconds in chrono, and we represent those as having a 60th second. However, apparently we might not allow parsing the 60th second, which may or may not be a defensible choice. The inconsistency is unfortunate, but I honestly don't have a strong opinion on how to improve this.

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

2 participants