tokio-rs / bytes

Utilities for working with bytes
MIT License
1.86k stars 278 forks source link

1.6.0 yanked? #722

Closed ChristopherRabotin closed 1 month ago

ChristopherRabotin commented 1 month ago

Hi there,

I just noticed that version 1.6.0 was yanked a few hours ago: image

What's the rationale for if I may ask? And is there any chance it could be unyanked? It breaking a few of my dependencies.

Thanks

sfackler commented 1 month ago

Why can't you use 1.6.1? Run cargo update.

ChristopherRabotin commented 1 month ago

I use fixed versions on my crates to have the choice on when to update them.

error: failed to select a version for the requirement `bytes = "=1.6.0"`
candidate versions found which didn't match: 1.6.1, 1.5.0, 1.4.0, ...
location searched: crates.io index
required by package `anise v0.4.0`
    ... which satisfies dependency `anise = "^0.4.0"` of package `nyx-space v2.0.0-rc (/home/runner/work/nyx/nyx)`

According to the Cargo Book:

Crates should only be yanked in exceptional circumstances, for example, an accidental publish, an unintentional SemVer breakages, or a significantly broken and unusable crate. In the case of security vulnerabilities, RustSec is typically a less disruptive mechanism to inform users and encourage them to upgrade, and avoids the possibility of significant downstream disruption irrespective of susceptibility to the vulnerability in question.

The change from 1.6.0 to 1.6.1 seems to not fall in this category, which is why I ask if there is any possibility for version 1.6.0 to be unyanked.

Thanks again

sfackler commented 1 month ago

Crates shouldn't use = version constraints.

ChristopherRabotin commented 1 month ago

I've been bitten before by poor version management, hence the =.

Can't you just unyank 1.6.0? The change in 1.6.1 is very minimal and does not meet the criteria of the Cargo book.

ChristopherRabotin commented 1 month ago

The maintainers also did not yank the previous versions when bug fixes were release.

image

I guess I'm lost as to why the maintainers would yank 1.6.0 for a pretty minor PR and not specify it in the README.

Darksonn commented 1 month ago

I'm sorry, but by using =1.6.0 as a dependency constraint, you have explicitly signed up for this kind of churn in your project. You should use at least a tilde version constraint if crates being yanked is a problem for your workflow.

If you look at the actual cause of the bug, I think it's a pretty serious kind of mistake. It's reading memory of one type as another type. It only barely isn't a security vulnerability worthy of a CVE since the operation is not a write, the memory being read is not out of bounds, the type used for the read is valid for all possible values, and not used for anything more complicated than checking whether the value is equal to one.

ChristopherRabotin commented 1 month ago

I beg to differ. Your argument, if I understand it correctly, is that there almost was a vulnerability. The Cargo Book, as I quoted above, explicitly says that even for a vulnerability, a yank is not the right approach. So it just sounds like you've made an overly eager yank here.

The reason I use fixed version dependencies is because several of my dependencies will update a patch version when they ought to update a minor version. Locking in dependencies means that only in "exceptional circumstances" (to quote the Cargo Book again) will these dependencies no longer be available.

Would it please be possible to un-yank version 1.6.0? That's all I'm asking. Thanks again

weihanglo commented 1 month ago

The Cargo Book also recommends that avoiding constraining SemVer too much. As an alternative, users can commit Cargo.lock into VCS.

Avoid constraining the upper bound of a version to be anything less than the next semver incompatible version (e.g. avoid ">=2.0, <2.4") as other packages in the dependency tree may require a newer version, leading to an unresolvable error (see #9029). Consider whether controlling the version in your Cargo.lock would be more appropriate.

cargo update --precise <yanked> will be stabilized in 1.80 so you can opt-in yanked versiond as you wish.

Darksonn commented 1 month ago

I mean, maybe yanking crates in this situation is the wrong way to go about it. But yanking versions with issues like this has precedent in the bytes crate going back to 2019, whereas the guidance you are quoting from the cargo book is from 2023. In fact, this is the first I hear of it.

As for the 1.2.1 patch release that was not yanked, that was a less serious bug.

ChristopherRabotin commented 1 month ago

Mistakes happen, I totally get it, and I make mistakes all the time. All I was asking was un-yank version 1.6.0, nothing more: it's just a single click on crates.io. This would still allow for dependent crates which specify version bytes = "1.6.0" will be updated automatically.

Among of the many advantages of the Rust ecosystem are reliable crate versions and reproducible builds. Yanking version 1.6.0 for a simple bug fix will break numerous other packages. Should the dozens and dozens of users be expecting that future bug fixes (without vulnerabilities, as said above) will also lead to previous version of bytes being yanked?

I'm not trying to impose any kind of onerous requirement here. I too maintain OSS, and it can be a lot of extra and demanding work. Again, all I am asking for is to unyank version 1.6.0 which was eagerly yanked for a bug fix.

Thank you

sfackler commented 1 month ago

If you want reproducible builds you should check in a lockfile.

seanmonstar commented 1 month ago

I'm sorry this has been a problem for you. Though, I do agree with Alice's assessment that yanking was appropriate. If it helps nudge people off a potentially bad version, then keeping it yanked can be for the greater good.

I hope you can increase the version you depend on your crates. The libraries maintained by us all very strictly follow semver. We put in a lot of effort to curate these crates for many users.

I'd suggest that it would be better to ask those who don't follow semver to do so, or just not depend on those crates.

However, because this has pulled in several of the maintainers already, and they're all extremely busy, I'm inclined to close this down and lock it.