cedar-policy / rfcs

Apache License 2.0
10 stars 8 forks source link

[RFC 80] `toDate` can overflow #85

Open shaobo-he-aws opened 2 weeks ago

shaobo-he-aws commented 2 weeks ago

In RFC 80 (i.e., the datetime extension), toDate is proposed as a method of datetime. If its meaning is that it should return the date where the instant is, then it could overflow. A counterexample is datetime("1970-01-01").offset(duration("-9223372036854775808ms")).toDate(), which should return the date 106751991168 (9223372036854775808/86400000 + 1) days before the Unix epoch, and this date cannot be represented by a long.

apg commented 1 week ago

I think the answer to this is that we bound duration to be something less than 292 million years. Creating a datetime, via the constructor, is already bounded to years 0-9999 by virtue of it being a 4 digit year requirement. We can say that 10,000 years is the duration limit, and fail on overflow.

emina commented 1 week ago

To retain analyzability, it is important for every long (64-bit integer) to represent a legal datetime and/or duration.

If we don't do that, we are effectively introducing a subtype of integers ("those that represent legal dates / durations"), and this form of subtyping is not something we can express in the encoding.

So, let's allow all valid 64-bit values as durations, and throw overflow errors when calculations overflow.

andrewmwells-amazon commented 1 week ago

I think the answer to this is that we bound duration to be something less than 292 million years. Creating a datetime, via the constructor, is already bounded to years 0-9999 by virtue of it being a 4 digit year requirement. We can say that 10,000 years is the duration limit, and fail on overflow.

Unless I'm mistaken, you can write something like DT.offset("10000y").offset("10000y")... so restricting the constructor doesn't actually restrict the range of valid DTs. I agree with Emina that we should allow all valid 64-bit values for both durations and DateTimes.

apg commented 1 week ago

Stacking .offset("10000y") would cause the datetime to fall out of the 10,000 year range, and be considered "overflow."

I guess expanding the definition of what consitutes overflow to another 292 million years is fine, though. :)

shaobo-he-aws commented 1 week ago

Stacking .offset("10000y") would cause the datetime to fall out of the 10,000 year range, and be considered "overflow."

IIUC, what @emina suggests is that there should not be any restrictions on the datetime other than it must be representable by a long. So we cannot establish a 10000 year range.

apg commented 1 week ago

@shaobo-he-aws: correct. I was responding to the objection that the 10,000 range is easy to circumvent.

By officially supporting this range, we'll be able to market Cedar to new applications -- paleontology (at least up to the Mesozoic era, or so).