mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
topdown: fix precision loss for integers larger than 64 bits in arithmetic and aggregates (#8887)
## Description Fixes #6281. `plus`, `minus`, `multiply`, `sum` and `product` corrupt integers that need more than 64 bits of precision. They route the operands through `builtins.NumberToFloat` (a `big.Float` carrying the default mantissa), so the value is rounded before the operation is applied: ```rego 18446744073709551617 + 1 # 18446744073709551616 (should be ...618) 18446744073709551617 * 2 # 36893488147419103230 (should be ...234) sum([18446744073709551617, 1]) # 18446744073709551616 (should be ...618) product([18446744073709551617, 2]) # 36893488147419103230 (should be ...234) ``` This is the same defect class that #8857 fixed for `format_int`. There is a root cause underneath it that is worth calling out separately, because it is the reason this fails silently rather than erroring: ```go func NumberToInt(n ast.Number) (*big.Int, error) { f := NumberToFloat(n) // rounds here r, accuracy := f.Int(nil) if accuracy != big.Exact { // cannot fire: the rounded float IS an exact integer return nil, errors.New("illegal value") } return r, nil } ``` The accuracy check exists to catch inexact conversions, but the rounding has already happened inside `NumberToFloat`, and the rounded value is itself an integer, so the check passes. `NumberToInt` returns the wrong `big.Int` and reports no error: ``` NumberToInt(18446744073709551617) = 18446744073709551616 NumberToInt(123456789012345678901234567890) = 123456789012345678899921813504 ``` `NumberToInt` also backs `BigIntOperand`, so the corruption is reachable from the `bits.*` builtins as well. ## Fix - `NumberToInt` parses integer literals exactly with `big.Int`, and falls back to `big.Rat` for fractional and exponent forms, so `1e30` stays exact and a genuinely fractional value is rejected rather than silently truncated. - `plus`/`minus`/`multiply` apply the operation on exact `big.Int`s when both operands are integers, mirroring how `format_int` was fixed. - `sum`/`product` accumulate on `big.Int` when every element is an integer, and fall back to the existing float accumulator otherwise (which also preserves the existing operand-type errors). Float, mixed int/float, and small-int behaviour is unchanged, including the existing interned-small-int fast paths. `0.1 + 0.2` still yields `0.3`. Division and modulo are untouched. The thread raises open questions about their expected semantics for big integers, so they felt out of scope here. ## Test Two golden cases: `arithmetic/bignum exact` and `aggregates/bignum exact`, covering >2^64 values through each operation, a 30-digit value, negatives, and the small-int / float / mixed cases that must not change. Results are rendered with `sprintf` in the golden cases because the case loader parses expected numbers as `float64`, which cannot represent these values. Asserting on the numbers directly fails on the loader rather than on the builtin. `format_int`'s golden case sidesteps the same problem by returning strings. Both cases fail on `main` and pass with this change. `go test ./v1/topdown/` passes with no regressions; the failing tests I do see on Windows (`TestCertSelectionLogic`, and several in `v1/rego`) fail identically on a clean checkout of `main`, so they are pre-existing and unrelated. Added WASM exceptions for both cases, as #8857 did, since WASM cannot represent integers larger than 64 bits (#3711). --------- Signed-off-by: Atishyy27 <atishayjain2704@gmail.com> Co-authored-by: Atishyy27 <atishayjain2704@gmail.com>
This commit is contained in:
@@ -2,3 +2,5 @@
|
||||
"data/toplevel integer": "https://github.com/open-policy-agent/opa/issues/3711"
|
||||
"data/nested integer": "https://github.com/open-policy-agent/opa/issues/3711"
|
||||
"strings/format_int: bignum exact (>64-bit integer, all bases + negative)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtin."
|
||||
"arithmetic/bignum exact (>64-bit integers through plus, minus, multiply)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtins."
|
||||
"aggregates/bignum exact (>64-bit integers through sum and product)": "WASM cannot represent integers larger than 64 bits (see https://github.com/open-policy-agent/opa/issues/3711); this change fixes the Go topdown builtins."
|
||||
|
||||
Reference in New Issue
Block a user