hoodie / icalendar-rs

📆 icalendar library, in Rust of course - for fun
Apache License 2.0
126 stars 34 forks source link

fix: disable oldtime feature of chrono #54

Closed figsoda closed 2 years ago

figsoda commented 2 years ago

on main:

Crate:     time
Version:   0.1.44
Title:     Potential segfault in the time crate
Date:      2020-11-18
ID:        RUSTSEC-2020-0071
URL:       https://rustsec.org/advisories/RUSTSEC-2020-0071
Solution:  Upgrade to >=0.2.23
Dependency tree:
time 0.1.44
└── chrono 0.4.22
    └── icalendar 0.13.1

error: 1 vulnerability found!

This removes the oldtime feature of chrono and fixes cargo audit

hoodie commented 2 years ago

thanks, I remember a different issue one time where somebody suggested just to get rid of chrono instead and opt for time directly, what do you think about that?

figsoda commented 2 years ago

It would be a breaking change if we were to remove chrono. chrono was pretty much unmaintained for a period of time but dedvelopment seems to have continued, maybe that's why it was suggested. We can also put chrono behind a feature and have another feature for time support, maybe something like

default = ["chrono-support"]
chrono-support = ["dep:chrono"]
time-support = ["dep:time"]

in Cargo.toml I think these are a little out of scope for this pr and maybe could be discussed in a dedicated issue

hoodie commented 2 years ago

thanks for bringing this up, I was also curious why this issue still hasn't been addressed, even though the dev on chrono has been taken over by somebody new a while ago already. From what I can tell so far this seems to rather non-ish issue according to one of the maintainers of chrono:

https://github.com/chronotope/chrono/issues/553#issuecomment-1075914745

so since this would be a rather significant change I'd rather not tare out chrono for now 😀

hoodie commented 2 years ago

alright, I had to read up on the issue a little. It turns out they actually recommend turning off the oldtime feature altogether and only kept on by default for backward compatibility. seems we don't actually need it (... well let's who'll complain that they got broke next hihi)

🫶 thanks for the contribution!!!