multiple allowed versions of packages that depend on ring

[TotalKrill]

Set a more permissive range to allow for more projects to co-depend on ring.
This way cargo will choose the best version

[jebrosen]

It would be nice if this actually worked, but I'm not confident cargo will handle it properly - see rust-lang/cargo#4902 (comment) and the prior conversation. Nearly every time I've worked with a crate that "supports" multiple otherwise-incompatible versions of a particular dependency something goes wrong.

[TotalKrill]

Yeah maybe, but i feel that until such time there is a real solution to this, I would prefer if we can still compile the project until such a time a problem arises.

I tested running the test-suite for both dependencies, and did not find a problem, and looking briefly at the libraries changed, there doesn't seem to that much of a breaking change between them, mostly dependencies.

Edit: I also do not think that Rocket should not do stuff because of a bug in Cargo, rather that the bug in cargo be fixed

[jebrosen]

If accepting this change breaks existing rocket 0.4 projects by depending on a newer version of ring, then we shouldn't do it. I think cargo nicely resolves dependencies on multiple possible versions of a crate with links=, but I'm not sure it will do it transitively. That is, my concern is that cargo will assume cookie 0.12 regardless of cookie 0.11 being supported, therefore requiring ring 0.14.

[TotalKrill]

I agree, From the very rudimentary tests i made, it does seem like it will choose 0.14 as the preferred one, but if an existing crate has a dependency on ring 0.13, it will use that one instead.

I am unsure how we could prove that it does not break existing builds though, I cant see how it would do that, and if it does, that would be great test to add, to safeguard against future dependency updates.

[jebrosen]

Because of rust-lang/cargo#4978 I think this is actually supposed to work, and it does seem to in my quick tests:

• If any dependency requires only ring 0.13, cargo selects 0.13
• If any dependency requires only ring 0.14, cargo selects 0.14
• If nothing else requires ring, cargo selects 0.14 (the latest)
• If both ring 0.13 and 0.14 are required, cargo errors

This is special behavior for crates that have links=; normally cargo selects the latest version in a range unless you are careful.

I had thought this did not work as desired because of past issues with range dependencies in diesel, but those were a different problem.

---

If this does work properly, no code changes are required within Rocket to support these version ranges so I think this would be safe to include in 0.4 as well. Testing it should not be too hard either -- I think we would need a hello_world-size example in a separate cargo workspace, with a dependency on ring 0.13 and making use of Rocket's features that depend on ring.

@SergioBenitez, did I miss something here?

[SergioBenitez]

I am worried for the same reasons you are, @jebrosen. Historically, Cargo has made incorrect decisions when it comes to minimally satisfying dependencies, which has included selecting two versions of a links dependency when selecting one is necessary and correct.

Assuming this is somehow fixed in Cargo (though I do think we need to test further), I don't believe there are any backwards compatibility concerns with this change.

How can we become confident that Cargo handles this scenario as necessary to prevent any breakage in 0.4?

[jebrosen]

How can we become confident that Cargo handles this scenario as necessary to prevent any breakage in 0.4?

I'll try a local version of my previous test, but actually using Rocket this time. I believe my hypothetical example that depends on rocket 0.4 (with this change) and ring 0.[13,14] would show that it works as intended and would also let us spot any problems in CI. It would need to be a separate workspace (for a separate Cargo.lock), and the main workspace would need to use the other version of ring in order to verify that both versions work.

[SergioBenitez]

@jebrosen What was the result of your tests?

[jebrosen]

Ah, forgot to post back here.

I duplicated the session example and switched it to ring 0.13 explicitly: jebrosen@723feac

Rocket happily compiles with either ring 0.13 or 0.14 depending on what the project itself needs, and a project that (indirectly) requires both ring 0.13 and ring 0.14 gets resolve errors from cargo. The tricky part is integrating this into the test suite, because it must be in a separate workspace to get a separate Cargo.lock.

As for the future:

This change rests on two important things: rustls, cookie, and hyper-sync-rustls did not change any public APIs that rocket uses between the two versions, and that cargo finds a single common version for links dependencies.

Those crates might not stay API-compatible in their ring 0.15-compatible versions, and rocket will have to change ring versions in a breaking release again in the future. In other words, we can't use this approach for every future ring release.

On the other hand if ring switches to versioned symbols, ring will no longer need to be considered part of Rocket's public API and we will be able to "unlock" the version freely at that point.

I don't know what will happen if ring 0.13 is yanked, and I don't know how to test it. My fear is that the error message won't be "0.13 is yanked, cargo is sad" - it might say "rocket needs 0.14, and you need a version that doesn't exist" which would be confusing.

[briansmith]

On the other hand if ring switches to versioned symbols, ring will no longer need to be considered part of Rocket's public API and we will be able to "unlock" the version freely at that point.

Just FYI, I am very aware of this issue and I'm eager to work with you to find a solution to the versioning problem. I made significant progress in replacing C code with Rust code, in large part to help address this issue, but obviously that's not all complete yet.

However, it's also the case that ring 0.16 has a lot of improvements to side channel attack mitigation and other significant improvements and I actively discourage people from using older versions. In the past I tried to use the "yank" mechanism to actively discourage people but it seems like cargo doesn't support using yanked crates well enough for this to be practical for most people, so I'm not planning to yank anything right now. Still, I highly recommend all projects to upgrade to 0.16 and not to use older versions.

That said, I appreciate the effort that has been put into addressing the versioning issue and I'll work with the people who have submitted PRs to allow multiple versions of ring as soon as I have some free time to do so.

[SergioBenitez]

Okay. An updated version was merged in ea9865e.