mozilla-services / python-autograph-utils

A library to simplify use of Autograph
Other
5 stars 4 forks source link

Use offset-aware datetimes #9

Closed leplatrem closed 5 years ago

glasserc commented 5 years ago

Why?

leplatrem commented 5 years ago

At least we make it explicit that datetime comparisons are made against the same timezone

1. Always use aware datetime object, i.e. with timezone information. https://julien.danjou.info/python-and-timezones/

glasserc commented 5 years ago

Is that really common practice in Python? I feel like most code I've seen follows instead http://lucumr.pocoo.org/2011/7/15/eppur-si-muove/, which discourages using timezone-aware datetimes at all.

Why would you not want to attach an UTC tzinfo object? First of all because the majority of libraries are written with the assumption of tzinfo == None in mind.

In fact cryptography timestamps have tzinfo == None, as you see here.

I guess this advice is from 2011, so maybe it's outdated? Is the timezone API fixed in Python 3 or something?

leplatrem commented 5 years ago

I guess that we can close this PR because in that particular case we don't return the value, we just use it to compare dates internally. I guess it will use the system timezone for both and comparison will work as excepted.

glasserc commented 5 years ago

Just to clarify, it will use UTC for the current time, and it will use whatever cryptography gives us for the not_valid_before and not_valid_after properties. It seems that will be UTC too: https://github.com/pyca/cryptography/blob/master/src/cryptography/hazmat/backends/openssl/decode_asn1.py#L800. I'm not 100% sure about this because it seems that certs can encode timezones in a GeneralizedTime, but the sample certs in the tests directory all seem to specify GMT (according to openssl x509 -text -noout).