Switch from chrono to time
#66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
est31
approved these changes
Nov 9, 2021
|
Opened qnighy/yasna.rs#63 since that seems to be taking a while. |
|
Could you rebase this? I drew this out hoping that maybe chrono would fix the issue, but I think now it's |
|
I've rebased, but it still depends on an unreleased change in yasna.rs: qnighy/yasna.rs#61 |
|
There is a yasna release now: qnighy/yasna.rs#64 |
There is unsoundness in `chrono` (https://rustsec.org/advisories/RUSTSEC-2020-0159), and although we don't use the affected APIs, the advisory still fires. `time` ≥3 does not have this unsoundness, and supports almost‡ all the necessary APIs, so we can quite simply swap it out. Since the `chrono` types are used in some APIs, those signatures have had to change, making this a breaking change. The yasna.rs dependency has also been upgraded to a version based on `time` rather than `chrono`. ‡ `time` does not support leap seconds in their representation or parsing. In this commit, we simply ignore leap seconds, which also constitutes a breaking change since values with leap seconds will parse differently (they parse to the previous second).
|
Rebased again with a proper dependency update: https:/est31/rcgen/compare/99c113ad474c7f321028ebc6b7bbfc1af4d61190..b4faf0416d08842cddaf08d4297ea58e832e5003 |
matze
added a commit
to matze/rcgen
that referenced
this pull request
Feb 21, 2022
Despite rustls#66, chrono is still pulled in transitively via x509-parser 0.12. x509-parser 0.13 switched to time as well, so using rcgen with an updated version on't cause `cargo audit` issues.
est31
pushed a commit
that referenced
this pull request
Feb 21, 2022
Despite #66, chrono is still pulled in transitively via x509-parser 0.12. x509-parser 0.13 switched to time as well, so using rcgen with an updated version on't cause `cargo audit` issues.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
yasna.rsas a git dependency, because there's no release with qnighy/yasna.rs#61 yet. I wanted to open the PR anyway to indicate the changes, and perhaps motivate a release foryasna.rs.Since qnighy/yasna.rs#61 is merged, I wanted to make the corresponding updates here as well, to hopefully resolve #65.
The changes are very mechanically once again, and again the main limitation is that
timecannot represent leap seconds. This affects some of the logic and removes a test case.I've checked the output from
main.rsand the examples withopenssl x509 -in certs/cert.pem -noout -textand it's as I'd expect. I've also tried it with a library using QUIC w/ self-signed certs and that also works as expected.