chrono feature to allow library users to either use time or chrono dependency

[bm-w]

The chrono feature brings in the chrono crate as a dependency & uses it to reimplement all date & time functionality. In essence, it uses chrono::DateTime<chrono::FixedOffset> in Expiration::DateTime rather than time::OffsetDateTime (FixedOffset seems to best match OffsetDateTime’s offset field), and chrono::Duration rather than time::Duration.

The time feature (which becomes a default feature) represents the library’s previous behavior, implementing date & time functionality using the time crate. Gating it behind a feature lets users opt out of using that crate, and into using the chrono crate instead.

In order to let the library continue working with default-features = false, all date & time functionality is gated behind either of these features. So too is some other functionality that relies on date & time functionality, such as “removal“ cookies. In order to avoid adding a lot of noise to doctests those too are gated behind the time feature.

This is a breaking change, given that it effectively removes all date & time functionality when neither feature is enabled, e.g. when default-features = false. Given that the cookies crate did not previously have default features, it seems plausible that few were actually disabling those. When time is enabled (which in many cases it will be as a default feature), none of the changes are breaking.

All regular tests pass, but I have not had the opportunity to run the fuzzers yet.

---

I went ahead an implemented this as I understood that it might be welcome from a comment by @SergioBenitez in #109. In another comment, @jhpratt express concern that the features wouldn’t be additive, but IIUC strictly either feature on its own is additive — they’re just mutually exclusive, for which the Cargo Book carves out an exception.

On a final note, also discussed in #109, removing the use of time::Duration (or chrono::Duration for that matter) in favor of std::time::Duration would clean this PR up a bit: the max_age stuff wouldn’t need to be feature-gated.

[SergioBenitez]

There's no way this can work. The features need to be truly additive - there is no exception for crates that are used as transitive dependencies. Imagine a crate Foo that depends on crates A and B where A depends on Cookie with chrono and B depends on cookie with time. Foo will fail to compile, and there's no recourse.

The only way to add support for chrono and/or time is to create independent cookie time types that wrap either of the types and allow conversions to/from the types. When a feature is enabled, the respective conversions are implemented. Thus you can use both features at the same time.

I'm not sure if a T: Into<cookie::Duration> bound would work, however. I'd be worried about the impls that appear based on features messing up inference. But we can try and test.

[bm-w]

Understood and agreed.

I could try and see if I can make your suggestion work. However, if we’re already going the way of independent cookie time types, I think we should go all the way and remove all types from time crate from the public API. We’d still need either the time or chrono crate for parsing†, and we could gate conversions behind time & chrono features (modulo your concerns about inference).

†As long as we have it handy we can and should use one of those crates for formatting as well, but we wouldn’t strictly need the crate, since cookie only ever formats string using parsing::FMT1 which we could could conceivably do ourselves. Moot point, though.

[SergioBenitez]

I could try and see if I can make your suggestion work. However, if we’re already going the way of independent cookie time types, I think we should go all the way and remove all types from time crate from the public API. We’d still need either the time or chrono crate for parsing†, and we could gate conversions behind time & chrono features (modulo your concerns about inference).

I agree, though I still think we need to provide conversion impls and pick a crate, any crate (could even be std::time) to do the parsing for us. I don't think we want to get into the business of maintaining (and debugging...) time parsing or manipulation.

In short:

• If we could back our internal types on std::time, that'd be great (though we might not be able to due to limitation of those types). And then by default we'd introduce no new dependencies.
• Provide two features: time and chrono, which enable conversions from the relevant types to our internal type.
• Test whether we can safely have polymorphic functions like fn max_age<T: Into<Duration>>(self, value: T), where Duration is the internal type and any of time::Duration and crono::Duration and cookie::Duration work as a value. Here, "safely" means that enabling a feature cannot prevent existing code from working. The danger is that enabling a feature enables a new impl which might mess with inference. We need to test/read.

[jhpratt]

For what it's worth, I did offer to have a shared Duration type between time and chrono specifically for cases like this. The offer was rejected.

[SergioBenitez]

It's not just Duration, unfortunately. We also need proper dates for expirations, and parsing of said dates.

[SergioBenitez]

Closing in favor of moving forward with #217.