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

Pin serde to <1.0.172 #359

Merged
merged 1 commit into from
Aug 19, 2023
Merged

Pin serde to <1.0.172 #359

merged 1 commit into from
Aug 19, 2023

Conversation

newpavlov
Copy link
Member

serde v1.0.172 and later include pre-compiled binaries which is a security hazard. So until the decision gets reverted, I believe it's worth to pin upper version of serde. This approach may cause issues if a different crate in someone's dependency tree will depend on a post-1.0.172 version of serde, but I think this issue is small enough when compared to the security concerns. Also, a number of other crates in the ecosystem follow this approach, so we are not alone.

More information and discussion about the serde change can be found in serde-rs/serde#2538.

Copy link
Member

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sad, but makes sense

@tarcieri
Copy link
Member

tarcieri commented Aug 19, 2023

Isn't the offending binary in the serde_derive crate?

Edit: oh, I see, serde pins serde_derive with =

@newpavlov
Copy link
Member Author

serde uses pinned version of serde_derive, i.e. serde v1.0.172 uses strictly only serde_derive v1.0.172

@newpavlov newpavlov force-pushed the pin_serde branch 3 times, most recently from c3b2384 to 5713330 Compare August 19, 2023 14:36
@newpavlov
Copy link
Member Author

@dignifiedquire
I will merge this PR, but will not do a release and will leave it up to your discretion.

@newpavlov newpavlov merged commit 3e17a67 into master Aug 19, 2023
9 checks passed
@newpavlov newpavlov deleted the pin_serde branch August 19, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants