Unverified Commit 8251264a authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Fix two bugs in RFC-3339 date formatting (#489)

- Fix year boundaries to strictly adhere to RFC-3339
- Fix nanosecond precision issue
parent a47ada58
Loading
Loading
Loading
Loading
+39 −60
Original line number Diff line number Diff line
@@ -241,7 +241,7 @@ pub mod http_date {
mod test_http_date {
    use proptest::prelude::*;

    use crate::instant::format::{http_date, iso_8601, DateParseError};
    use crate::instant::format::{http_date, rfc3339, DateParseError};
    use crate::Instant;

    #[test]
@@ -353,21 +353,21 @@ mod test_http_date {
    fn valid_iso_date() {
        let date = "1985-04-12T23:20:50.52Z";
        let expected = Instant::from_secs_and_nanos(482196050, 520000000);
        assert_eq!(iso_8601::parse(date), Ok(expected));
        assert_eq!(rfc3339::parse(date), Ok(expected));
    }

    #[test]
    fn iso_date_no_fractional() {
        let date = "1985-04-12T23:20:50Z";
        let expected = Instant::from_secs_and_nanos(482196050, 0);
        assert_eq!(iso_8601::parse(date), Ok(expected));
        assert_eq!(rfc3339::parse(date), Ok(expected));
    }

    #[test]
    fn read_iso_date_comma_split() {
        let date = "1985-04-12T23:20:50Z,1985-04-12T23:20:51Z";
        let (e1, date) = iso_8601::read(date).expect("should succeed");
        let (e2, date2) = iso_8601::read(&date[1..]).expect("should succeed");
        let (e1, date) = rfc3339::read(date).expect("should succeed");
        let (e2, date2) = rfc3339::read(&date[1..]).expect("should succeed");
        assert_eq!(date2, "");
        assert_eq!(date, ",1985-04-12T23:20:51Z");
        let expected = Instant::from_secs_and_nanos(482196050, 0);
@@ -386,7 +386,7 @@ mod test_http_date {
    }
}

pub mod iso_8601 {
pub mod rfc3339 {
    use chrono::format;

    use crate::instant::format::DateParseError;
@@ -403,7 +403,7 @@ pub mod iso_8601 {
        let format = format::StrftimeItems::new("%Y-%m-%dT%H:%M:%S%.fZ");
        // TODO: it may be helpful for debugging to keep these errors around
        chrono::format::parse(&mut date, s, format)
            .map_err(|_| DateParseError::Invalid("invalid iso8601 date"))?;
            .map_err(|_| DateParseError::Invalid("invalid rfc3339 date"))?;
        let utc_date = date
            .to_naive_datetime_with_offset(0)
            .map_err(|_| DateParseError::Invalid("invalid date"))?;
@@ -413,14 +413,14 @@ pub mod iso_8601 {
        ))
    }

    /// Read 1 ISO8601 date from &str and return the remaining str
    /// Read 1 RFC-3339 date from &str and return the remaining str
    pub fn read(s: &str) -> Result<(Instant, &str), DateParseError> {
        let delim = s.find('Z').map(|idx| idx + 1).unwrap_or_else(|| s.len());
        let (head, rest) = s.split_at(delim);
        Ok((parse(head)?, &rest))
    }

    /// Format an [Instant] in the ISO-8601 date format
    /// Format an [Instant] in the RFC-3339 date format
    pub fn format(instant: &Instant) -> String {
        use std::fmt::Write;
        let (year, month, day, hour, minute, second, nanos) = {
@@ -436,23 +436,18 @@ pub mod iso_8601 {
            )
        };

        // This is stated in the assumptions for RFC-3339. ISO-8601 allows for years
        // between -99,999 and 99,999 inclusive, but RFC-3339 is bound between 0 and 9,999.
        assert!(
            year.abs() <= 99_999,
            "years greater than 5 digits are not supported by ISO-8601"
            (0..=9_999).contains(&year),
            "years must be between 0 and 9,999 in RFC-3339"
        );

        let mut out = String::with_capacity(33);
        if (0..=9999).contains(&year) {
            write!(out, "{:04}", year).unwrap();
        } else if year < 0 {
            write!(out, "{:05}", year).unwrap();
        } else {
            write!(out, "+{:05}", year).unwrap();
        }
        write!(
            out,
            "-{:02}-{:02}T{:02}:{:02}:{:02}",
            month, day, hour, minute, second
            "{:04}-{:02}-{:02}T{:02}:{:02}:{:02}",
            year, month, day, hour, minute, second
        )
        .unwrap();
        format_nanos(&mut out, nanos);
@@ -460,18 +455,17 @@ pub mod iso_8601 {
        out
    }

    /// Formats sub-second nanos for RFC-3339 (including the '.').
    /// Expects to be called with a number of `nanos` between 0 and 999_999_999 inclusive.
    fn format_nanos(into: &mut String, nanos: u32) {
        debug_assert!(nanos < 1_000_000_000);
        if nanos > 0 {
            into.push('.');
            let mut place = 100_000_000;
            let mut pushed_non_zero = false;
            while place > 0 {
                let digit = (nanos / place) % 10;
                if pushed_non_zero && digit == 0 {
                    return;
                }
                pushed_non_zero = digit > 0;
            let (mut remaining, mut place) = (nanos, 100_000_000);
            while remaining > 0 {
                let digit = (remaining / place) % 10;
                into.push(char::from(b'0' + (digit as u8)));
                remaining -= digit * place;
                place /= 10;
            }
        }
@@ -479,36 +473,11 @@ pub mod iso_8601 {
}

#[cfg(test)]
mod test_iso_8601 {
    use super::iso_8601::format;
mod test {
    use super::rfc3339::format;
    use crate::Instant;
    use chrono::SecondsFormat;
    use proptest::proptest;

    #[test]
    fn year_edge_cases() {
        assert_eq!(
            "-0001-12-31T18:22:50Z",
            format(&Instant::from_epoch_seconds(-62167239430))
        );
        assert_eq!(
            "0001-05-06T02:15:00Z",
            format(&Instant::from_epoch_seconds(-62124788700))
        );
        assert_eq!(
            "+33658-09-27T01:46:40Z",
            format(&Instant::from_epoch_seconds(1_000_000_000_000))
        );
        assert_eq!(
            "-29719-04-05T22:13:20Z",
            format(&Instant::from_epoch_seconds(-1_000_000_000_000))
        );
        assert_eq!(
            "-1199-02-15T14:13:20Z",
            format(&Instant::from_epoch_seconds(-100_000_000_000))
        );
    }

    #[test]
    fn no_nanos() {
        assert_eq!(
@@ -555,16 +524,26 @@ mod test_iso_8601 {
            "1970-01-01T00:00:00.000000001Z",
            format(&Instant::from_secs_and_nanos(0, 000_000_001))
        );
        assert_eq!(
            "1970-01-01T00:00:00.101Z",
            format(&Instant::from_secs_and_nanos(0, 101_000_000))
        );
    }

    proptest! {
        // Sanity test against chrono (excluding nanos, which format differently)
        // Sanity test against chrono
        #[test]
        fn proptest_iso_8601(seconds in -1_000_000_000_000..1_000_000_000_000i64) {
            let instant = Instant::from_epoch_seconds(seconds);
            let chrono_formatted = instant.to_chrono_internal().to_rfc3339_opts(SecondsFormat::AutoSi, true);
        #[cfg(feature = "chrono-conversions")]
        fn proptest_rfc3339(
            seconds in 0..253_402_300_799i64, // 0 to 9999-12-31T23:59:59
            nanos in 0..1_000_000_000u32
        ) {
            use chrono::DateTime;

            let instant = Instant::from_secs_and_nanos(seconds, nanos);
            let formatted = format(&instant);
            assert_eq!(chrono_formatted, formatted);
            let parsed: Instant = DateTime::parse_from_rfc3339(&formatted).unwrap().into();
            assert_eq!(instant, parsed);
        }
    }
}
+26 −3
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ impl Instant {

    pub fn from_str(s: &str, format: Format) -> Result<Self, DateParseError> {
        match format {
            Format::DateTime => format::iso_8601::parse(s),
            Format::DateTime => format::rfc3339::parse(s),
            Format::HttpDate => format::http_date::parse(s),
            Format::EpochSeconds => <f64>::from_str(s)
                // TODO: Parse base & fraction separately to achieve higher precision
@@ -75,7 +75,7 @@ impl Instant {
    /// Enable parsing multiple dates from the same string
    pub fn read(s: &str, format: Format, delim: char) -> Result<(Self, &str), DateParseError> {
        let (inst, next) = match format {
            Format::DateTime => format::iso_8601::read(s)?,
            Format::DateTime => format::rfc3339::read(s)?,
            Format::HttpDate => format::http_date::read(s)?,
            Format::EpochSeconds => {
                let split_point = s.find(delim).unwrap_or_else(|| s.len());
@@ -134,7 +134,7 @@ impl Instant {

    pub fn fmt(&self, format: Format) -> String {
        match format {
            Format::DateTime => format::iso_8601::format(&self),
            Format::DateTime => format::rfc3339::format(&self),
            Format::EpochSeconds => {
                if self.subsecond_nanos == 0 {
                    format!("{}", self.seconds)
@@ -148,6 +148,20 @@ impl Instant {
    }
}

#[cfg(feature = "chrono-conversions")]
impl From<DateTime<Utc>> for Instant {
    fn from(value: DateTime<Utc>) -> Instant {
        Instant::from_secs_and_nanos(value.timestamp(), value.timestamp_subsec_nanos())
    }
}

#[cfg(feature = "chrono-conversions")]
impl From<DateTime<chrono::FixedOffset>> for Instant {
    fn from(value: DateTime<chrono::FixedOffset>) -> Instant {
        value.with_timezone(&Utc).into()
    }
}

#[derive(Clone, Copy, Eq, PartialEq)]
pub enum Format {
    DateTime,
@@ -217,4 +231,13 @@ mod test {
        let (_, next) = Instant::read(s, Format::HttpDate, ',').expect("valid");
        assert_eq!(next, "Tue, 17 Dec 2019 23:48:18 GMT");
    }

    #[test]
    #[cfg(feature = "chrono-conversions")]
    fn chrono_conversions_round_trip() {
        let instant = Instant::from_secs_and_nanos(1234, 56789);
        let chrono = instant.to_chrono();
        let instant_again: Instant = chrono.into();
        assert_eq!(instant, instant_again);
    }
}