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

Encoder includes seconds in timezone when encoding #161

Open
rudigiesler opened this issue Jun 1, 2023 · 3 comments
Open

Encoder includes seconds in timezone when encoding #161

rudigiesler opened this issue Jun 1, 2023 · 3 comments
Labels

Comments

@rudigiesler
Copy link
Contributor

rudigiesler commented Jun 1, 2023

This was picked up from the hypothesis testing.

RFC3339 section 5.6 defines the timezone to consist of just hours and minutes, not seconds.

Python timezones support seconds, and when converting to a string using isoformat, includes the seconds.

This is an issue if you try to decode it again, as you'll get an error that it is an invalid datetime string:

>>> from datetime import datetime
>>> dt = datetime.fromisoformat("1912-01-01T00:00:00-00:16:08")
datetime.datetime(1912, 1, 1, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=85432)))
>>> from cbor2 import encoder, decoder
>>> encoder.dumps(dt)
b'\xc0x\x1c1912-01-01T00:00:00-00:16:08'
>>> decoder.loads(b'\xc0x\x1c1912-01-01T00:00:00-00:16:08')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/rudi/pk/cbor2/cbor2/decoder.py", line 668, in loads
    return CBORDecoder(fp, **kwargs).decode()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/rudi/pk/cbor2/cbor2/decoder.py", line 198, in decode
    return self._decode()
           ^^^^^^^^^^^^^^
  File "/Users/rudi/pk/cbor2/cbor2/decoder.py", line 185, in _decode
    return decoder(self, subtype)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/rudi/pk/cbor2/cbor2/decoder.py", line 379, in decode_semantic
    return semantic_decoder(self)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/rudi/pk/cbor2/cbor2/decoder.py", line 445, in decode_datetime_string
    raise CBORDecodeValueError(f"invalid datetime string: {value!r}")
cbor2.types.CBORDecodeValueError: invalid datetime string: '1912-01-01T00:00:00-00:16:08'
@agronholm
Copy link
Owner

I guess we should not allow serialization of such datetimes at all then?

@agronholm
Copy link
Owner

I don't think this problem would ever manifest in real life though, as all time zones have UTC offsets in 30 minute increments.

@agronholm agronholm added the bug label Sep 3, 2023
@rudigiesler
Copy link
Contributor Author

I think raising an exception when trying to serialize such datetimes is a good solution. Silently truncating data on serialization doesn't seem like a good solution, letting the user know that we can't serialise it, and having them truncate it before passing to us, sounds correct to me.

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

No branches or pull requests

2 participants