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

v1.0.181 introduced a breaking change - was that intentional/can that happen again? #2662

Closed
jost-s opened this issue Dec 11, 2023 · 2 comments

Comments

@jost-s
Copy link

jost-s commented Dec 11, 2023

I've outlined the situation as a response to the PR that introduced a breaking change from v1.0.180 to v1.0.181.

I asked for clarification there but haven't received an answer, that's why I'm creating this issue. Please respond either here or on the PR.

@dtolnay
Copy link
Member

dtolnay commented Dec 11, 2023

Sorry about the breakage. I don't know a lot about messagepack, but I don't think I anticipated that this change would affect it.

I tried reproducing the example from #2505 (comment) using this code:

// [dependencies]
// rmp-serde = "1.1.2"
// serde = { version = "1", features = ["derive"] }

#[derive(serde::Serialize, serde::Deserialize, Debug)]
#[serde(tag = "type", content = "data")]
enum Message {
    A(i32),
}

fn main() {
    let m = Message::A(100);

    let bytes = rmp_serde::to_vec_named(&m).unwrap();
    println!("{:?}", bytes);
    println!("{:?}", rmp_serde::from_slice::<Message>(&bytes).unwrap());

    let bytes = rmp_serde::to_vec(&m).unwrap();
    println!("{:?}", bytes);
    println!("{:?}", rmp_serde::from_slice::<Message>(&bytes).unwrap());
}

And with both cargo update -p serde --precise 1.0.180 and cargo update -p serde --precise 1.0.181 it produces the same output. The to_vec_named output is:

[130, 164, 116, 121, 112, 101, 161, 65, 164, 100, 97, 116, 97, 100]

I was not able to reproduce the other output ([130, 164, 116, 121, 112, 101, 129, 161, 65, 192, 164, 100, 97, 116, 97, 100]). Do I need to configure the serializer in some non-default way to get that?

Regarding whether it can happen again — I think the best way to help it not happen would be by participating in reviewing serde PRs.

@jost-s
Copy link
Author

jost-s commented Dec 11, 2023

Thank you for the swift response and investigation. It turns out that the project I was referring to is using rmp-serde v0.15, and with that version you get different outputs with serde v1.0.180 and v1.0.181.

$ cargo update -p serde --precise 1.0.180
    Updating crates.io index
 Downgrading serde v1.0.181 -> v1.0.180
 Downgrading serde_derive v1.0.181 -> v1.0.180
$ cargo run
[130, 164, 116, 121, 112, 101, 161, 65, 164, 100, 97, 116, 97, 100]
[146, 161, 65, 100]
$ cargo update -p serde --precise 1.0.181
    Updating crates.io index
    Updating serde v1.0.180 -> v1.0.181
    Updating serde_derive v1.0.180 -> v1.0.181
$ cargo run
[130, 164, 116, 121, 112, 101, 129, 0, 192, 164, 100, 97, 116, 97, 100]
[146, 129, 0, 192, 100]

As you say with v1 it produces identical outputs. So the breakage somehow comes from the interplay of serde with that version of rmp-serde.

I've built in a serialization test as a guard in our system that'll prevent unknown serialization deviations. I'll close this issue.

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

No branches or pull requests

2 participants