RFC: ditching the time 0.1 dependency on 0.4.x

[djc]

If we are convinced that our local Duration type API is a strict superset of the time::Duration API (pending an extra round of review), I think the risk of downstream users experiencing actual incompatibility from us dropping the time 0.1 dependency completely is exceedingly limited, and maybe we should just bite the bullet and do it? This would finally get rid of the advisory crap, and would have the added benefit of allowing us to evolve the Duration API on the 0.4.x branch.

@esheppa @pitdicker @jtmoon79 thoughts?

[pitdicker]

Is there any way to know how much real-world breakage this would cause?

[wez]

If you'll forgive me for chiming in; I don't really know anything about the development approach in chrono, or its internals.

Another option to consider is this:

• Branch 0.6-alpha from the current 0.5-alpha branch; this will be where development continues moving forwards
• Branch 0.5 from the current 0.4 branch. On that new 0.5, change the default features to exclude oldtime; that's the only change vs. 0.4. Publish that as 0.5.0. That gives you an intermediate release where the default avoids the legacy, and the semver bump prevents casually running into the problem with a minor 0.4.x update; upholding the semver constraint and not risk breaking anyone in case there was something unforeseen with the duration or other parts of the API.

The story for people complaining about the advisory stuff would then be: "upgrade to 0.5". It should be a non-breaking change in most cases. If that change does break someone, you can engage and get concrete feedback on the breakage and figure out the path forwards for people that might be affected by it.

The only API/semver breakage would be to anyone running with a 0.5 alpha, but those users are already aware that it is alpha and unstable, and they can "simply" migrate to 0.6 alpha. That feels less broadly disruptive than risking the stronger more stable expectation that users have for the 0.4 series.

[pitdicker]

I don't like the suggestion by @wez. But realistically 0.5 is not going to be ready any time soon. The CVE is definitely annoying for chrono and dependent libraries.

Releasing a 0.5 just for just this reason has the drawback that dependents have to do the work to manually update. But It seems like a welcome update to me, and that it is otherwise non-breaking should make it easier as you say.

[djc]

Another option to consider is this:

• Branch 0.6-alpha from the current 0.5-alpha branch; this will be where development continues moving forwards
• Branch 0.5 from the current 0.4 branch. On that new 0.5, change the default features to exclude oldtime; that's the only change vs. 0.4. Publish that as 0.5.0. That gives you an intermediate release where the default avoids the legacy, and the semver bump prevents casually running into the problem with a minor 0.4.x update; upholding the semver constraint and not risk breaking anyone in case there was something unforeseen with the duration or other parts of the API.

Because chrono is used relatively often as a public dependency in downstream crates, I think the ecosystem churn from releasing a 0.5 release for which the only benefit is that it gets rid of the ostensible time 0.1 dependency is not worth it.

Yes, releasing 0.4.x with the local Duration implementation is technically breaking, but it's really only breaking for the subset of downstream users who (a) are still depending on time 0.1 outside chrono and (b) are upgrading their chrono releases.

Is there any way to know how much real-world breakage this would cause?

I'm thinking it might be possible to do some kind of crater run. Not sure the crater infrastructure is available for this kind of thing (outside the Rust project), and I don't personally have the time to spend much time on this, but of course someone could spin up an AWS VM and compile all the library crates (and public GitHub repos) with a patched chrono dependency to see if anything breaks.

[esheppa]

I think the crater run is a great idea as it gives us at least some empirical evidence to back up our argument of minimal breakage. I'll have a look into doing this on a VM

[jtmoon79]

dropping the time 0.1 dependency completely ... maybe we should just bite the bullet and do it?

tl;dr yes

Without reviewing chrono::Duration and time::Duration, dropping time version 0.1 (and the related Security Advisory) is a better path.

Simply, the security advisory nags is a big annoyance and source of confusion for everyone involved. For the engineer without the time to investigate, it appears chrono project isn't concerned about the security advisory headache it's causing. They just see a long-standing nag from github or wherever.

A security advisory can be especially difficult in corporate/government/etc. organizations where policy dictates "no vulnerable software!". For an engineer trying to argue for a special exemption "but we're not using the vulnerable part!", that is often much toil of delicate communication to various managers assuming exemptions are even allowed.

IME, people are much more willing to forgive version upgrade breaks (if there are any in this case) so long as they have clear forewarnings and tips.

So IMO, doing whatever is necessary to move on from this Security Advisory is preferred, including potential code breaks.

[pitdicker]

Last week (before this issue) I had to look at the code for Duration in chrono and time 0.1 side by side. They were identical.

[pitdicker]

Do we have a way back if the release causes too many issues? I don't think so. The version can be yanked, but then the crates that already adjusted their feature flags for this change would brake.

Yes, releasing 0.4.x with the local Duration implementation is technically breaking, but it's really only breaking for the subset of downstream users who (a) are still depending on time 0.1 outside chrono and (b) are upgrading their chrono releases.

The part about upgrading chrono releases is maybe more common than this assumes. How about: some lightly-maintained library in the dependency graph has this dependency on chrono and time 0.1 and cargo resolves it to the latest version of chrono (maybe because of the requirements of another crate)?

---

It is funny, this is another negative sounding comment, while I prefer taking it loosely and making the change in 0.4.x.

[djc]

Do we have a way back if the release causes too many issues? I don't think so. The version can be yanked, but then the crates that already adjusted their feature flags for this change would brake.

There would be no change in Cargo features. The feature will still be there, it will just do nothing -- this is what keeps it almost compatible. As such, yes, the way to backtrack would be to yank the version where we adjusted this, and release a new one that puts back the time 0.1 Duration.

The part about upgrading chrono releases is maybe more common than this assumes. How about: some lightly-maintained library in the dependency graph has this dependency on chrono and time 0.1 and cargo resolves it to the latest version of chrono (maybe because of the requirements of another crate)?

Any code on GitHub that's still using time 0.1 for stuff has had to endure GitHub GHSA warnings about the RustSec advisory. time 0.2.0 was first released 2.5 years ago, so even lightly-maintained software has had ample chance to upgrade to a newer version of time.

[jhpratt]

I think the risk of downstream users experiencing actual incompatibility from us dropping the time 0.1 dependency completely is exceedingly limited,

Based on what? I opposed moving time from a default feature flag to a non-default flag, as that was objectively a breaking change. Now it's proposed to break this even further based on what I can only imagine is a hunch? Merely having the same public API doesn't mean you can swap types at will for what should be obvious reasons.

so even lightly-maintained software has had ample chance to upgrade to a newer version of time.

This assumes that chrono isn't the reason they're still using time 0.1, which is a reasonable assumption. Note that a crater-like run only catches public code, while the code I'm envisioning is presumably more likely to be in corporate environments.

---

Since it seems that you're already closing in on a 0.5 release, I don't see what benefit doing this for 0.4 gives. chrono is widely used. Given that fact, it is quite likely that this is being relied upon by someone. It's not Hyrum's law, either, as the fact that it's the same type (not just the same API) is something the compiler relies upon for coherence checks, among others. The behavior of it being a re-export is also clearly documented.

More generally, I would love it if time and chrono had a shared signed Duration type. I am happy to co-maintain this crate if desired.

If the security vulnerability is the primary motivator, then let's work on a solution that can be backported to 0.1. Making existing code fail to compile (for unclear reasons at that) likely isn't the answer that users want.

[djc]

I think the risk of downstream users experiencing actual incompatibility from us dropping the time 0.1 dependency completely is exceedingly limited,

Based on what? I opposed moving time from a default feature flag to a non-default flag, as that was objectively a breaking change. Now it's proposed to break this even further based on what I can only imagine is a hunch? Merely having the same public API doesn't mean you can swap types at will for what should be obvious reasons.

It seems like a practical solution to me. The language itself sometimes makes technically breaking changes if they've come to the conclusion that practical impact is limited; I'm preparing to make a similar trade-off here. We've inadvertently made small breaking changes before, and it did not turn out to be a big problem. I honestly don't think there's a benefit of following semver constraints by the letter.

Since it seems that you're already closing in on a 0.5 release, I don't see what benefit doing this for 0.4 gives. chrono is widely used. Given that fact, it is quite likely that this is being relied upon by someone. It's not Hyrum's law, either, as the fact that it's the same type (not just the same API) is something the compiler relies upon for coherence checks, among others. The behavior of it being a re-export is also clearly documented.

The benefit is getting rid of the security advisory which is actually invalid (misleading, and still causing people churn and investigation time), and an extra dependency that is actually unnecessary for 3, 4, 5 nines of the current audience.

More generally, I would love it if time and chrono had a shared signed Duration type. I am happy to co-maintain this crate if desired.

I don't think I have much time to spend on this in the near future.

Making existing code fail to compile (for unclear reasons at that) likely isn't the answer that users want.

This is an invalid dichotomy. Yes, making existing code fail to compile isn't great, but most users won't have code that will fail to compile and many of those are experiencing some kind of effect from the extra dependency with the security advisory.

[campbellcole]

Average chrono user here: I would much prefer a breaking change if it meant getting rid of that pesky security advisory. I've read lots of places that it's not actually going to affect me, but it did take a lot of time out of my day to research the advisory and figure that out.

In my mind, a breaking change is a lot less jarring than a notice saying a crate I really can't avoid using has a security vuln. Thanks for all of your hard work 😄

[jhpratt]

Random thought as an aside: it wouldn't be the worst of ideas to improve RUSTSEC such that a crate can declare that it's aware of a vulnerability in a dependency and assets that it is unaffected. It would be useful for situations like this.

[hartwork]

Hi!

My impression is that what version of time users are using themselves does not interfere or affect what version of time chrono is using: I expect one to two entries for time in Cargo.lock: two if versions match, one if they don't. I have seen that in practice with clap and I have verified that in a minimal reproducer also, details below.

Could you help me understand how getting chrono 0.4.x off of vulnerable time 0.1.x could have a breaking effect on users? I would like to understand the related worries better as I don't see any myself yet. Thanks! 🙏

Analysis, click to expand

Here's what I tried:

# cd "$(mktemp -d)"

# cargo new issue-1073

# cd issue-1073/

# cargo add chrono@0.4.26
# cargo add time@0.3.22

# cargo tree --workspace | grep ' time '
│   └── time v0.1.45
└── time v0.3.22

# grep -A1 '"time"' Cargo.lock 
name = "time"
version = "0.1.45"
--
name = "time"
version = "0.3.22"

# cat src/main.rs 
use time::{OffsetDateTime, UtcOffset};

fn main() {
    let offset_date_time = OffsetDateTime::now_utc();
    let utc_offset = UtcOffset::from_hms(1, 2, 3).unwrap();
    let date_time = offset_date_time.checked_to_offset(utc_offset).unwrap();
    println!("date_time is {date_time}, we have time >=0.3.22 beneath.");
}

# cargo run
[..]
date_time is 2023-06-15 17:36:16.891891732 +01:02:03, we have time >=0.3.22 beneath.

Note that OffsetDateTime::checked_to_offset was only added in time 0.3.22.

[djc]

Have a look at the analysis that @pitdicker did in #1095.

[cdesch]

Average chrono user here: I would much prefer a breaking change if it meant getting rid of that pesky security advisory. I've read lots of places that it's not actually going to affect me, but it did take a lot of time out of my day to research the advisory and figure that out.

+1 for this. CI/CD picks up the advisory constantly and it takes more time to acknowledge in downstream workflows, explain on dashboards, and provide explanation/exception for those on the periphery of the development team than it will to fix a breaking change.

[pitdicker]

Released as version 0.4.30. If you have feedback about the switch, please reply in #1268.