Skip to content

Commit b8023d4

Browse files
authored
Fixed panic when providing a CLI command with an incorrect JSON input (#3398)
## Changes Fixed panic when providing a CLI command with an incorrect JSON input ## Why CLI should not panic but show a nicer, more actionable error instead. Fixes #3393 ## Tests Added an acceptance test
1 parent bddc4a3 commit b8023d4

File tree

9 files changed

+114
-2
lines changed

9 files changed

+114
-2
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* Update Go SDK to 0.79.0 ([#3376](https://github.com/databricks/cli/pull/3376))
1010

1111
### CLI
12+
* Fixed panic when providing a CLI command with an incorrect JSON input ([#3398](https://github.com/databricks/cli/pull/3398))
1213

1314
### Bundles
1415
* Changed logic for resolving `${resources...}` references. Previously this would be done by terraform at deploy time. Now if it references a field that is present in the config, it will be done by DABs during bundle loading ([#3370](https://github.com/databricks/cli/pull/3370))

acceptance/bundle/variables/file-defaults/output.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141

4242
=== file cannot be parsed
4343
>>> errcode [CLI] bundle validate -o json --target invalid_json
44-
Error: failed to parse variables file [TEST_TMP_DIR]/.databricks/bundle/invalid_json/variable-overrides.json: error decoding JSON at :0:0: invalid character 'o' in literal false (expecting 'a')
44+
Error: failed to parse variables file [TEST_TMP_DIR]/.databricks/bundle/invalid_json/variable-overrides.json: error decoding JSON at [TEST_TMP_DIR]/.databricks/bundle/invalid_json/variable-overrides.json:1:1: invalid character 'o' in literal false (expecting 'a')
4545

4646

4747
Exit code: 1
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Local = true
2+
Cloud = false
3+
4+
[EnvMatrix]
5+
DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"]
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
2+
>>> musterr [CLI] secrets create-scope --scope-backend-type AZURE_KEYVAULT --json "scope": "xxxxxxxx-xxx"
3+
Error: error decoding JSON at (inline):1:8: unexpected additional content
4+
5+
Exit code (musterr): 1
6+
7+
>>> musterr [CLI] secrets create-scope --scope-backend-type AZURE_KEYVAULT --json ["scope"]
8+
Error: expected JSON object, received sequence
9+
10+
Exit code (musterr): 1
11+
12+
>>> [CLI] secrets create-scope --scope-backend-type AZURE_KEYVAULT --json {
13+
"scope": "xxxxxxxx-xxx",
14+
"scope_backend_type": "AZURE_KEYVAULT",
15+
"keyvault_metadata": {
16+
"dns_name": "test",
17+
"resource_id": "123456"
18+
}
19+
}
20+
Warning: unknown field: keyvault_metadata
21+
in (inline):4:4
22+
23+
24+
>>> [CLI] secrets create-scope --scope-backend-type AZURE_KEYVAULT --json {
25+
"scope": "xxxxxxxx-xxx",
26+
"scope_backend_type": "AZURE_KEYVAULT"
27+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
trace musterr $CLI secrets create-scope --scope-backend-type AZURE_KEYVAULT --json '"scope": "xxxxxxxx-xxx"'
2+
3+
trace musterr $CLI secrets create-scope --scope-backend-type AZURE_KEYVAULT --json '["scope"]'
4+
5+
trace $CLI secrets create-scope --scope-backend-type AZURE_KEYVAULT --json '{
6+
"scope": "xxxxxxxx-xxx",
7+
"scope_backend_type": "AZURE_KEYVAULT",
8+
"keyvault_metadata": {
9+
"dns_name": "test",
10+
"resource_id": "123456"
11+
}
12+
}'
13+
14+
trace $CLI secrets create-scope --scope-backend-type AZURE_KEYVAULT --json '{
15+
"scope": "xxxxxxxx-xxx",
16+
"scope_backend_type": "AZURE_KEYVAULT"
17+
}'
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Local = true
2+
Cloud = false
3+
4+
[[Server]]
5+
Pattern = "POST /api/2.0/secrets/scopes/create"
6+
Response.Body = '''
7+
{
8+
"scope": "xxxxxxxx-xxx"
9+
}
10+
'''

libs/dyn/jsonloader/json.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,19 @@ func LoadJSON(data []byte, source string) (dyn.Value, error) {
2323
if err == io.EOF {
2424
err = errors.New("unexpected end of JSON input")
2525
}
26-
return dyn.InvalidValue, fmt.Errorf("error decoding JSON at %s: %v", value.Location(), err)
26+
// Get the current offset for error reporting
27+
errorOffset := decoder.InputOffset()
28+
errorLocation := offset.GetPosition(errorOffset)
29+
return dyn.InvalidValue, fmt.Errorf("error decoding JSON at %s: %v", errorLocation, err)
2730
}
31+
32+
// Check if there are any remaining tokens (should not be valid JSON)
33+
if decoder.More() {
34+
errorOffset := decoder.InputOffset()
35+
errorLocation := offset.GetPosition(errorOffset)
36+
return dyn.InvalidValue, fmt.Errorf("error decoding JSON at %s: unexpected additional content", errorLocation)
37+
}
38+
2839
return value, nil
2940
}
3041

libs/dyn/jsonloader/json_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,23 @@ func TestJsonLoaderEOF(t *testing.T) {
9191
_, err := LoadJSON([]byte(eofData), "path/to/file.json")
9292
assert.ErrorContains(t, err, "unexpected end of JSON input")
9393
}
94+
95+
const mapWithNoBraces = `
96+
"job_id": 123,
97+
"new_settings": {
98+
"name": "xxx",
99+
"wrong": "xxx",
100+
}
101+
`
102+
103+
func TestJsonMapWithoutBraces(t *testing.T) {
104+
_, err := LoadJSON([]byte(mapWithNoBraces), "path/to/file.json")
105+
assert.ErrorContains(t, err, "error decoding JSON at")
106+
}
107+
108+
const validInline = `["job_id", 123]`
109+
110+
func TestJsonValidInline(t *testing.T) {
111+
_, err := LoadJSON([]byte(validInline), "path/to/file.json")
112+
assert.NoError(t, err)
113+
}

libs/flags/json_flag.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,27 @@ func (j *JsonFlag) Unmarshal(v any) diag.Diagnostics {
5757
return diags
5858
}
5959

60+
if !nv.IsValid() {
61+
kind := reflect.TypeOf(v).Kind()
62+
if kind == reflect.Ptr {
63+
kind = reflect.TypeOf(v).Elem().Kind()
64+
}
65+
66+
expectedJsonType := "string"
67+
switch kind {
68+
case reflect.Struct, reflect.Map:
69+
expectedJsonType = "object"
70+
case reflect.Slice:
71+
expectedJsonType = "array"
72+
}
73+
74+
return diags.Append(diag.Diagnostic{
75+
Severity: diag.Error,
76+
Summary: "Invalid command input",
77+
Detail: fmt.Sprintf("expected JSON %s, received %s", expectedJsonType, dv.Kind()),
78+
})
79+
}
80+
6081
// Then marshal the normalized data to the output.
6182
// It will serialize all set data with the correct types.
6283
data, err := json.Marshal(nv.AsAny())

0 commit comments

Comments
 (0)