Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes chrono and usages of DateTime<Utc> and replaces them withtime::OffsetDateTime #249

Merged
merged 7 commits into from
Nov 23, 2021

Conversation

clintfred
Copy link
Contributor

@clintfred clintfred commented Nov 19, 2021

Closes #244 #245

This change should not have any semantic meaning and is being done to address RUSTSEC-2020-0071 (see #244) and RUSTSEC-20200159 (see #245).

This is a breaking API change because some result types include create/updated timestamps. Consumers of the library will need to add a direct dependency on time. Note that chrono already uses time internally, so this change should not bloat transitive dependencies.

For those needing serialization support of OffsetDateTime timestamps, the time crate offers this support with the serde feature flag.

One small point of friction here is rfc3339 support. time has this support on a branch that is blocked on a rust feature hitting stable, so I copied in a few lines of code from that branch to support deserializing rfc3339 timestamps.

To fully fix the RUSTSEC advisories, it was necessary to upgrade one of our dependencies as well.
See Keats/jsonwebtoken#222

For now, this PR depends on https://github.com/IronCoreLabs/jsonwebtoken.

…`time::OffsetDateTime`.

This change should not have any semantic meaning and is being done to address RUSTSEC-2020-0071 (see #244) and RUSTSEC-20200159 (see #245).

This is a breaking API change because some result types include create/updated timestamps. Consumers of the library will need to add a direct dependency on `time`. Note that `chrono` already uses `time` internally, so this change should not bloat transitive dependencies.

For those needing serialization support of  `OffsetDateTime` timestamps, the `time` crate offers this support with the `serde` feature flag.

One small point of friction here is rfc3339 support. `time` has this support on [a branch](time-rs/time#387) that is blocked on a rust feature hitting stable, so I copied in a few lines of code from that branch to support deserializing rfc3339 timestamps.
Cargo.toml Outdated Show resolved Hide resolved
src/internal/group_api/requests.rs Outdated Show resolved Hide resolved
src/internal/serde_rfc3339.rs Outdated Show resolved Hide resolved
@@ -1,5 +1,8 @@
# Changelog

## 0.27.0 (Unreleased)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start putting dates on the changelog entries.
Also, any reason to not release this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily disagreeing on adding the changelog dates, but the canonical information for release-dates is available from crates.io via

curl 'https://crates.io/api/v1/crates/ironoxide' 

As for release - I think it's fine to release this whenever, but we should merge this PR first and then follow https://github.com/IronCoreLabs/ironoxide/blob/main/RELEASING.md
If we want to start adding dates in CHANGELOG.md, adding it to the release checklist would be the right place.

Copy link
Member

@coltfred coltfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good to me. I did bump the base64 dep to 0.13 in jsonwebtoken to match ours. That's what they're doing on v8 of the real release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RUSTSEC-2020-0071: Potential segfault in the time crate
4 participants