Skip to content

Commit 983a9b0

Browse files
authored
Fix int64 precision loss in JSON loader (#5494)
## Why Found during a full-repo review of the CLI. The JSON loader decodes every number as float64, which has a 53-bit mantissa. Integer values above 2^53 silently lose precision, and job, run, and pipeline IDs routinely exceed that. For example, `--json '{"job_id": 123456789012345678}'` unmarshals to `123456789012345680` and targets the wrong resource. The corruption happens at load time, so the precision guard in `convert.Normalize` never sees the original value. ## Changes Before, all JSON numbers became float64; now integer literals are loaded as int64 and keep their exact value. The loader in `libs/dyn/jsonloader/json.go` enables `decoder.UseNumber()` and handles the `json.Number` token: integer literals parse as int64, while literals with a fraction or exponent (`2.0`, `1e3`) stay float64. Integer literals that overflow int64 also fall back to float64, matching the previous decoder behavior. This affects everything loading through `jsonloader`: the `--json` flag on generated commands and bundle variable files. Both paths normalize values against the target type afterwards, and `convert.Normalize` already converts int to float (and back) with precision checks, so well-formed inputs are unaffected apart from no longer being corrupted. ## Test plan - [x] New unit tests in `libs/dyn/jsonloader`: large positive and negative int64 values, max int64, floats with fraction or exponent, int64 overflow fallback, out-of-range error, and a mixed object converted via `convert.ToTyped` asserting the exact job ID - [x] `go test ./libs/dyn/...` - [x] `go test ./libs/flags ./bundle/config/mutator` (the two `LoadJSON` consumers) - [x] Acceptance tests covering JSON inputs pass unchanged: `bundle/variables/file-defaults`, `cmd/workspace/apps`, `cmd/workspace/database/update-database-instance` - [x] `./task fmt-q`, `./task lint-q`, `./task checks` This pull request and its description were written by Isaac.
1 parent 58c0974 commit 983a9b0

2 files changed

Lines changed: 61 additions & 0 deletions

File tree

libs/dyn/jsonloader/json.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ func LoadJSON(data []byte, source string) (dyn.Value, error) {
1717
reader := bytes.NewReader(data)
1818
decoder := json.NewDecoder(reader)
1919

20+
// Use json.Number to avoid losing precision on int64 values above 2^53 (e.g. job and run IDs).
21+
decoder.UseNumber()
22+
2023
// Start decoding from the top-level value
2124
value, err := decodeValue(decoder, &offset)
2225
if err != nil {
@@ -107,6 +110,16 @@ func decodeValue(decoder *json.Decoder, o *Offset) (dyn.Value, error) {
107110
}
108111
return dyn.NewValue(arr, []dyn.Location{location}), nil
109112
}
113+
case json.Number:
114+
// Integers that overflow int64 fall back to float64, matching the decoder's behavior without UseNumber.
115+
if i64, err := tok.Int64(); err == nil {
116+
return dyn.NewValue(i64, []dyn.Location{location}), nil
117+
}
118+
f64, err := tok.Float64()
119+
if err != nil {
120+
return invalidValueWithLocation(decoder, o), fmt.Errorf("invalid number %q: %w", tok.String(), err)
121+
}
122+
return dyn.NewValue(f64, []dyn.Location{location}), nil
110123
default:
111124
return dyn.NewValue(tok, []dyn.Location{location}), nil
112125
}

libs/dyn/jsonloader/json_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/databricks/cli/libs/dyn/convert"
77
"github.com/databricks/databricks-sdk-go/service/jobs"
88
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
910
)
1011

1112
const jsonData = `
@@ -111,3 +112,50 @@ func TestJsonValidInline(t *testing.T) {
111112
_, err := LoadJSON([]byte(validInline), "path/to/file.json")
112113
assert.NoError(t, err)
113114
}
115+
116+
func TestJsonLoaderNumbers(t *testing.T) {
117+
for _, tc := range []struct {
118+
input string
119+
expected any
120+
}{
121+
{`123`, int64(123)},
122+
{`-1`, int64(-1)},
123+
{`123456789012345678`, int64(123456789012345678)},
124+
{`-123456789012345678`, int64(-123456789012345678)},
125+
{`9223372036854775807`, int64(9223372036854775807)},
126+
{`2.0`, 2.0},
127+
{`2.5`, 2.5},
128+
{`1e3`, 1000.0},
129+
{`18446744073709551615`, 1.8446744073709552e+19},
130+
} {
131+
v, err := LoadJSON([]byte(tc.input), "(inline)")
132+
assert.NoError(t, err, tc.input)
133+
assert.Equal(t, tc.expected, v.AsAny(), tc.input)
134+
}
135+
}
136+
137+
func TestJsonLoaderNumberOutOfRange(t *testing.T) {
138+
_, err := LoadJSON([]byte(`1e400`), "(inline)")
139+
assert.ErrorContains(t, err, "value out of range")
140+
}
141+
142+
const mixedNumbersData = `
143+
{
144+
"job_id": 123456789012345678,
145+
"new_settings": {
146+
"name": "xxx",
147+
"timeout_seconds": 100
148+
}
149+
}
150+
`
151+
152+
func TestJsonLoaderMixedNumbersToTyped(t *testing.T) {
153+
v, err := LoadJSON([]byte(mixedNumbersData), "(inline)")
154+
require.NoError(t, err)
155+
156+
var r jobs.ResetJob
157+
err = convert.ToTyped(&r, v)
158+
require.NoError(t, err)
159+
assert.Equal(t, int64(123456789012345678), r.JobId)
160+
assert.Equal(t, 100, r.NewSettings.TimeoutSeconds)
161+
}

0 commit comments

Comments
 (0)