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

Panic in conversion of NaiveDateTime to the local time on Windows #977

Closed
vvkostenko opened this issue Feb 27, 2023 · 3 comments
Closed
Labels

Comments

@vvkostenko
Copy link

Code like this works fine:

let naivedatetime = NaiveDate::from_ymd_opt(1900, 1, 1)
    .unwrap().and_hms_opt(0, 0, 0)
    .unwrap();

let datetime_from_utc = Local.from_utc_datetime(&naivedatetime);
let datetime_from_local = Local.from_local_datetime(&naivedatetime).unwrap();

But changing of the first line to

let naivedatetime = NaiveDate::from_ymd_opt(1898, 1, 1)
        .unwrap().and_hms_opt(0, 0, 0)
        .unwrap();

causes a panic: thread 'datetime::tests::test_from_naive_date_time_conversions' panicked at 'attempt to subtract with overflow', src\offset\local\windows.rs:223:18

I face this problem on Windows only. So what range of datetime should be supported by functions Local.from_utc_datetime and Local.from_local_datetime on Windows?

@djc
Copy link
Contributor

djc commented Mar 3, 2023

This looks like a bug. Assuming you're on 0.4.x, the code says:

tm_year: d.year() - 1900,  // this doesn't underflow, we know that d is `NaiveDateTime`.

Which indicates the comment is clearly wrong? This code seems to be using a struct Tm which restricts tm_year to "Years since 1900" which seems just a silly limitation. The Microsoft documentation for the SYSTEMTIME suggests that its wYear supports values "from 1601 through 30827".

So the Windows code here seems pretty broken. Would you be interested in helping to fix it? That would be great.

@vvkostenko
Copy link
Author

@djc, sure. I can try to fix it

@pitdicker
Copy link
Collaborator

This particular issue was fixed in #992. We still panic on years prior to 1601 however.

Because there are other open issues about that case I am going to close this one. See #651 and #1150.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants