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

Bug with leap second parsing ISO 8601 #674

Closed
dracarys18 opened this issue Apr 8, 2024 · 6 comments
Closed

Bug with leap second parsing ISO 8601 #674

dracarys18 opened this issue Apr 8, 2024 · 6 comments
Labels
A-parsing Area: parsing A-well-known-format-description Area: well known format descriptions C-bug Category: bug in current code

Comments

@dracarys18
Copy link
Contributor

Facing an issue while parsing leap seconds in Iso8601. Currently in our system we generate current time and format it to Iso8601 format serialize and save it in redis. This serialization code goes like this,

    pub fn serialize<S>(date_time: &PrimitiveDateTime, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        date_time
            .assume_utc()
            .format(&Iso8601::<FORMAT_CONFIG>)
            .map_err(S::Error::custom)?
            .serialize(serializer)
    }

And this method generates leap second every once in a while, example: 2023-12-11T11:31:60.000Z.

But while parsing it back to PrimitiveDateTime I am facing en error which says second must be in the range 0..=59. Deserializer code looks like this

    pub fn deserialize<'a, D>(deserializer: D) -> Result<PrimitiveDateTime, D::Error>
    where
        D: Deserializer<'a>,
    {
        let val = String::deserialize(deserializer)?;
        let time = OffsetDateTime::parse(&val, &Iso8601::<FORMAT_CONFIG>)
            .map_err(de::Error::custom)?
            .to_offset(UtcOffset::UTC);

        Ok(PrimitiveDateTime::new(time.date(), time.time()))
    }
@jhpratt
Copy link
Member

jhpratt commented Apr 9, 2024

Two things here:

  1. Leap seconds are not properly handled. A trivial demo shows this using a leap second that has actually occurred. This is a bug and will be fixed.
  2. If you are using 2023-12-11T11:31:60.000Z as your test case, don't. Leap seconds have only occurred as the last second of either half of the year (i.e. Jun 30, Dec 31 at 23:59:60). December 11 obviously is not that. Theoretically they can happen at the end of any month (per definition of UTC), but in reality preference is given to June or December, with March and September being alternates.

@jhpratt jhpratt added C-bug Category: bug in current code A-parsing Area: parsing A-well-known-format-description Area: well known format descriptions labels Apr 9, 2024
@jhpratt jhpratt changed the title Bug with leap second parsing Bug with leap second parsing ISO 8601 Apr 9, 2024
@dracarys18
Copy link
Contributor Author

  1. @jhpratt This is not a testcase this is from our logs

@jhpratt
Copy link
Member

jhpratt commented Apr 9, 2024

I would suggest looking into your logging system in that case, as it's producing invalid values.

@dracarys18
Copy link
Contributor Author

dracarys18 commented Apr 9, 2024

@jhpratt So this is generated by this function and I got this exact value from logs which was throwing deserialisation error.

@jhpratt
Copy link
Member

jhpratt commented Apr 9, 2024

Time holding a seconds value of 60 would be the only way for the crate to output that string barring a hard-coded literal in the format description. After quickly checking what is called all the way down, there's no way that that is what's causing this. The seconds field is ultimately calculated in Time::adjusting_add, and it's quite easy to verify that the implementation there is correct.

@jhpratt
Copy link
Member

jhpratt commented Apr 10, 2024

Fixed on main. It will be released shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: parsing A-well-known-format-description Area: well known format descriptions C-bug Category: bug in current code
Projects
None yet
Development

No branches or pull requests

2 participants