serde icon indicating copy to clipboard operation
serde copied to clipboard

std::time::SystemTime serializes incorrectly

Open matszczygiel opened this issue 4 years ago • 5 comments

Hello,

std::time::SystemTime serializes incorrectly to {"secs_since_epoch":1634823736,"nanos_since_epoch":647664740} in json format.

The name nanos_since_epoch is misleading, as it should represent the number of sub nanoseconds.

The fix seems to be very simple, just changing the name nanos_since_epoch -> subnanos or something similar.

I am happy to fix this problem with a pull request.

matszczygiel avatar Oct 22 '21 06:10 matszczygiel

I noticed this as well while deserializing timestamps -- one work-around might be to serialize a Duration since the UNIX_EPOCH directly. The field names are not as informative ("secs", and "nanos"), but they are at least accurate.

mlsvrts avatar Oct 31 '21 18:10 mlsvrts

This is perhaps unfortunate naming, but a change of field name in serialization would be a breaking change as the fields as-is may be depended upon downstream or a older version of serde.

avitex avatar Jan 22 '22 16:01 avitex

@avitex Even of it requires breaking change, I don't that this should be simply left as is.

If you send this JSON outside of the Rust (or even serde) ecosystem, anyone who looks at it directly is getting incorrect information. If whoever receives the message takes us at our word, they're going to have a poor time calculating a timestamp from the 'nanos since the epoch,' that we've supplied.

Leaving a field name that is purely wrong can't be the right solution.

mlsvrts avatar Jan 22 '22 17:01 mlsvrts

I'm in agreement that the naming is unfortunate, but I don't agree it is supplying incorrect information. The field data returned is correct. If I was a consumer, I wouldn't be assuming that nanos was the complete number of nanos from epoch if it isn't a larger number than the seconds by a factor of 1,000,000,000.

If the field names were flipped, to since_epoch_secs and since_epoch_nanos this may have been a little less confusing.

avitex avatar Jan 22 '22 17:01 avitex

Maybe the better idea would be to deprecate the Serialize implementation of this type. The format picked is very untypical anyways and it might be better to require users to deal with this through a newtype / with.

mitsuhiko avatar Jan 30 '22 00:01 mitsuhiko