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

Regression in 0.3.10 from 0.3.9 parsing optional rfc3339 dates #479

Closed
lu-zero opened this issue Jun 20, 2022 · 3 comments
Closed

Regression in 0.3.10 from 0.3.9 parsing optional rfc3339 dates #479

lu-zero opened this issue Jun 20, 2022 · 3 comments
Assignees
Labels
A-well-known-format-description Area: well known format descriptions C-bug Category: bug in current code

Comments

@lu-zero
Copy link

lu-zero commented Jun 20, 2022

This test:

    #[test]
    fn date() {
        const A: &str = r#"
            {
                "date": "2022-05-01T10:20:42.123Z"
            }
        "#;

        const B: &str = r#"
            {
                "date": "2022-05-01T10:20:42.123Z",
                "maybedate": "2022-05-01T10:20:42.123Z"
            }
        "#;

        #[derive(Debug, Deserialize, Serialize)]
        struct S {
            #[serde(with = "time::serde::rfc3339")]
            date: OffsetDateTime,
            #[serde(with = "time::serde::rfc3339::option", default)]
            maybedate: Option<OffsetDateTime>,
        }

        let a: S = serde_json::from_str(A).unwrap();
        let b: S = serde_json::from_str(B).unwrap();

        println!("{a:?}, {b:?}");
    }

passes on 0.3.9 and fails with 0.3.10 with:

thread 'thing::test::date' panicked at 'called `Result::unwrap()` on an `Err` value: Error("invalid type: Option value, expected an RFC3339-formatted `OffsetDateTime`", line: 4, column: 29)',
@jhpratt jhpratt self-assigned this Jun 20, 2022
@jhpratt jhpratt added C-bug Category: bug in current code A-well-known-format-description Area: well known format descriptions labels Jun 20, 2022
@jhpratt
Copy link
Member

jhpratt commented Jun 20, 2022

Confirmed. I'll take a look into this later today.

@jhpratt
Copy link
Member

jhpratt commented Jun 20, 2022

This was introduced in f9398b9, which itself was a fix to serde-rs/serde#466. It probably would have been caught if serde-rs/test#2 was resolved. The ability to test serde implementations without pulling in various serializers/deserializers as dependencies is limited as a result of the serde issue.

I have a possible fix (reverting one line of that commit), but I need to introduce a new test dependency and write a few tests to ensure that nothing regresses. I'd also like to avoid removing the ability to have non-self-describing formats work with Option<T> (which is what serde-rs/serde#466 is about), but I will if it's necessary to avoid this bug.

@jhpratt
Copy link
Member

jhpratt commented Jun 22, 2022

I have reverted the relevant line and added a regression test. The partial reversion has just been released as part of 0.3.11.

@jhpratt jhpratt closed this as completed Jun 22, 2022
lu-zero added a commit to sifis-home/wot-td that referenced this issue Jun 22, 2022
time-0.3.10 introduced a bug.

See: time-rs/time#479
lu-zero added a commit to sifis-home/wot-td that referenced this issue Jun 22, 2022
time-0.3.10 introduced a bug.

See: time-rs/time#479
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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