Fix precision loss in compareDoubleInt/compareDoubleUint for values outside float64 safe integer range#1328
Conversation
|
@jnthntatum @l46kok @jcking What are your thoughts on switching to |
|
Isn't this the same problem as #1317? I'd much rather not switch to BigFloat/BigInt, its a pretty heavy weight dep in other stacks and typically performs memory allocation. I am pretty sure you can implement this correctly without using those. |
|
@jcking it's mostly the same, but with a wider type representation. The way in which doubles are rounded depends on compiler flags and platform settings, so I'm not sure if switching to a wider representation above int53 precision would avoid other incongruities relating to floating point. That's my main thought. |
|
Rounding is controlled by IEEE 754 behavior. We should just be able to brute force it with: That should get pretty close. we would have to check the minimum representable value before zero and infinity and see if they hold. But that can be peeked at as well by force casting to int and back and comparing. |
Summary
Fix precision loss in
compareDoubleIntandcompareDoubleUintwhen comparingdoublewithint/uintvalues outside the safe float64 integer range (> 2^53).Problem
The current implementation casts
Int(int64) orUint(uint64) tofloat64before comparison:IEEE 754
float64has a 53-bit mantissa. Any integer with absolute value greater than2^53 = 9007199254740992cannot be represented exactly asfloat64. This means two distinctint64values can map to the samefloat64, causing the comparison to incorrectly return equal.Demonstrated bug
Other affected values:
int(100000000000000001) == double(1e17)→true(should befalse)int(1000000000000000001) == double(1e18)→true(should befalse)Impact
CEL is used as the expression engine in GCP IAM Conditions, Firebase Security Rules, and Kubernetes admission webhooks. A policy rule that compares an integer field to a double literal near these boundary values can produce incorrect authorization decisions.
Fix
Use
math/big.Floatfor exact comparison, eliminating the int→float64 precision loss:The same fix is applied to
compareDoubleUint.The fast-path bounds checks (
< MinInt64,> MaxInt64) remain unchanged and avoid thebig.Floatallocation for values clearly out of range. Thebig.Floatpath is only reached for values in the representable int64 range where precision matters.Testing
All existing comparison tests continue to pass. The previously incorrect cases now return the correct result: