Replace chrono with time (address RUSTSEC-2020-0071) #6
Conversation
|
@kevinhoffman is there anything blocking this from being merged and released? Would be happy to help if I can. |
|
Bump? |
|
Any update on this @kevinhoffman? Would be awesome to have this so we can use this crate in projects where we enforce |
|
I want to ➕ what others' have been asking for: any update on getting this merged? |
| @@ -313,8 +308,8 @@ where | |||
| RC: RollingCondition, | |||
| { | |||
| fn write(&mut self, buf: &[u8]) -> io::Result<usize> { | |||
| let now = Local::now(); | |||
| self.write_with_datetime(buf, &now) | |||
| let now = OffsetDateTime::now_local().unwrap_or_else(|_| OffsetDateTime::now_utc()); | |||
There was a problem hiding this comment.
FYI: This change can introduce a memory leak due to time-rs/time#651
There was a problem hiding this comment.
A mach ports leak is better than a segfault, but this is something best addressed via a PR to the num_threads crate.
There was a problem hiding this comment.
This is called quite frequently and every call will leak at least numberOfThreads mach ports. This can easily accumulate to several GB of leaked memory for long-running processes like daemons.
It's also worth pointing out that OffsetDateTime::now_local() will always fail for macOS (unless the process is single threaded). Which makes this equivalent to let now = OffsetDateTime::now_utc(); (on Apple platforms).
It might make sense to consider rotating based on UTC instead of local time in general. This would save quite a few CPU cycles and it's what would happen on Apple platforms with the suggested code anyway.
There was a problem hiding this comment.
I think rotating on UTC is a totally acceptable solution. I'm happy to modify the PR, but I haven't heard anything from the maintainer in over a year so I'm not sure if the crate is still being maintained.
|
Out of curiosity. Since version 0.4.30 chrono no longer depends on time 0.1 (https://github.com/chronotope/chrono/releases/tag/v0.4.30). And before that it didn't use the code that was vulnerable to RUSTSEC-2020-0071 since chrono 4.20. |
This library is unfortunately more susceptible than normal to the env var race conditions in the RUSTSEC, because it uses local time calls extensively.
This PR addresses the vulnerability by replacing
chronowithtime 0.3. This is a semver breaking change, because it touches quite a few public interfaces.