-
Notifications
You must be signed in to change notification settings - Fork 181
Added JSON input validation for CLI commands #1771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
58ab2f2
0aca374
b0aeee5
dbb3c2b
3ddf6b5
5f839e3
5a0197e
1280428
7594836
cf2ab73
35f2d58
d25d95b
d4a3346
82d6640
71bc701
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package jsonloader | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
|
|
||
| "github.com/databricks/cli/libs/dyn" | ||
| ) | ||
|
|
||
| func LoadJSON(data []byte) (dyn.Value, error) { | ||
| offsets := BuildLineOffsets(data) | ||
| reader := bytes.NewReader(data) | ||
| decoder := json.NewDecoder(reader) | ||
|
|
||
| // Start decoding from the top-level value | ||
| value, err := decodeValue(decoder, offsets) | ||
| if err != nil { | ||
| if err == io.EOF { | ||
| err = fmt.Errorf("unexpected end of JSON input") | ||
| } | ||
| return dyn.InvalidValue, err | ||
| } | ||
| return value, nil | ||
| } | ||
|
|
||
| func decodeValue(decoder *json.Decoder, offsets []LineOffset) (dyn.Value, error) { | ||
| // Read the next JSON token | ||
| token, err := decoder.Token() | ||
| if err != nil { | ||
| return dyn.InvalidValue, err | ||
| } | ||
|
|
||
| // Get the current byte offset | ||
| offset := decoder.InputOffset() | ||
| location := GetPosition(offset, offsets) | ||
|
|
||
| switch tok := token.(type) { | ||
| case json.Delim: | ||
| if tok == '{' { | ||
|
andrewnester marked this conversation as resolved.
|
||
| // Decode JSON object | ||
| obj := make(map[string]dyn.Value) | ||
| for decoder.More() { | ||
| // Decode the key | ||
| keyToken, err := decoder.Token() | ||
| if err != nil { | ||
| return dyn.InvalidValue, err | ||
| } | ||
| key, ok := keyToken.(string) | ||
| if !ok { | ||
| return dyn.InvalidValue, fmt.Errorf("expected string for object key") | ||
| } | ||
|
|
||
| // Decode the value recursively | ||
| val, err := decodeValue(decoder, offsets) | ||
| if err != nil { | ||
| return dyn.InvalidValue, err | ||
| } | ||
|
|
||
| obj[key] = val | ||
| } | ||
| // Consume the closing '}' | ||
| if _, err := decoder.Token(); err != nil { | ||
| return dyn.InvalidValue, err | ||
| } | ||
| return dyn.NewValue(obj, []dyn.Location{location}), nil | ||
| } else if tok == '[' { | ||
|
andrewnester marked this conversation as resolved.
|
||
| // Decode JSON array | ||
| var arr []dyn.Value | ||
| for decoder.More() { | ||
| val, err := decodeValue(decoder, offsets) | ||
| if err != nil { | ||
| return dyn.InvalidValue, err | ||
| } | ||
| arr = append(arr, val) | ||
| } | ||
| // Consume the closing ']' | ||
| if _, err := decoder.Token(); err != nil { | ||
| return dyn.InvalidValue, err | ||
| } | ||
| return dyn.NewValue(arr, []dyn.Location{location}), nil | ||
| } | ||
| default: | ||
| // Primitive types: string, number, bool, or null | ||
| return dyn.NewValue(tok, []dyn.Location{location}), nil | ||
| } | ||
|
|
||
| return dyn.InvalidValue, fmt.Errorf("unexpected token: %v", token) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package jsonloader | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/databricks/cli/libs/dyn/convert" | ||
| "github.com/databricks/databricks-sdk-go/service/jobs" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| const jsonData = ` | ||
| { | ||
| "job_id": 123, | ||
| "new_settings": { | ||
| "name": "xxx", | ||
| "email_notifications": { | ||
| "on_start": [], | ||
| "on_success": [], | ||
| "on_failure": [] | ||
| }, | ||
| "webhook_notifications": { | ||
| "on_start": [], | ||
| "on_failure": [] | ||
| }, | ||
| "notification_settings": { | ||
| "no_alert_for_skipped_runs": true, | ||
| "no_alert_for_canceled_runs": true | ||
| }, | ||
| "timeout_seconds": 0, | ||
| "max_concurrent_runs": 1, | ||
| "tasks": [ | ||
| { | ||
| "task_key": "xxx", | ||
| "email_notifications": {}, | ||
| "notification_settings": {}, | ||
| "timeout_seconds": 0, | ||
| "max_retries": 0, | ||
| "min_retry_interval_millis": 0, | ||
| "retry_on_timeout": "true" | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ` | ||
|
|
||
| func TestJsonLoader(t *testing.T) { | ||
| v, err := LoadJSON([]byte(jsonData)) | ||
| require.NoError(t, err) | ||
|
|
||
| var r jobs.ResetJob | ||
| err = convert.ToTyped(&r, v) | ||
| require.NoError(t, err) | ||
| } | ||
|
andrewnester marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package jsonloader | ||
|
|
||
| import ( | ||
| "sort" | ||
|
|
||
| "github.com/databricks/cli/libs/dyn" | ||
| ) | ||
|
|
||
| type LineOffset struct { | ||
| Line int | ||
| Start int64 | ||
| } | ||
|
|
||
| // buildLineOffsets scans the input data and records the starting byte offset of each line. | ||
| func BuildLineOffsets(data []byte) []LineOffset { | ||
|
andrewnester marked this conversation as resolved.
Outdated
|
||
| offsets := []LineOffset{{Line: 1, Start: 0}} | ||
| line := 1 | ||
| for i, b := range data { | ||
| if b == '\n' { | ||
| line++ | ||
| offsets = append(offsets, LineOffset{Line: line, Start: int64(i + 1)}) | ||
| } | ||
| } | ||
| return offsets | ||
| } | ||
|
|
||
| // GetPosition maps a byte offset to its corresponding line and column numbers. | ||
| func GetPosition(offset int64, offsets []LineOffset) dyn.Location { | ||
| // Binary search to find the line | ||
| idx := sort.Search(len(offsets), func(i int) bool { | ||
| return offsets[i].Start > offset | ||
| }) - 1 | ||
|
|
||
| if idx < 0 { | ||
| idx = 0 | ||
| } | ||
|
|
||
| lineOffset := offsets[idx] | ||
| return dyn.Location{ | ||
| File: "(inline)", | ||
|
andrewnester marked this conversation as resolved.
Outdated
|
||
| Line: lineOffset.Line, | ||
| Column: int(offset-lineOffset.Start) + 1, | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,10 @@ import ( | |
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
|
|
||
| "github.com/databricks/cli/libs/dyn/convert" | ||
| "github.com/databricks/cli/libs/dyn/jsonloader" | ||
| "github.com/databricks/databricks-sdk-go/marshal" | ||
| ) | ||
|
|
||
| type JsonFlag struct { | ||
|
|
@@ -33,7 +37,34 @@ func (j *JsonFlag) Unmarshal(v any) error { | |
| if j.raw == nil { | ||
| return nil | ||
| } | ||
| return json.Unmarshal(j.raw, v) | ||
|
|
||
| dv, err := jsonloader.LoadJSON(j.raw) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we do the same for the YAML flag? I believe that's the only reason we still depend on Not blocking for this PR, of course. |
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // First normalize the input data. | ||
| // It will convert all the values to the correct types. | ||
| // For example string lterals for booleans and integers will be converted to the correct types. | ||
|
andrewnester marked this conversation as resolved.
Outdated
|
||
| nv, diags := convert.Normalize(v, dv) | ||
| if len(diags) > 0 { | ||
| summary := "" | ||
| for _, diag := range diags { | ||
| summary += fmt.Sprintf("- %s\n", diag.Summary) | ||
| } | ||
| return fmt.Errorf("json input error:\n%v", summary) | ||
|
andrewnester marked this conversation as resolved.
Outdated
|
||
| } | ||
|
andrewnester marked this conversation as resolved.
|
||
|
|
||
| // Then marshal the normalized data to the output. | ||
| // It will serialize all set data with the correct types. | ||
| data, err := json.Marshal(nv.AsAny()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Finally unmarshal the normalized data to the output. | ||
| // It will fill in the ForceSendFields field if the struct contains it. | ||
| return marshal.Unmarshal(data, v) | ||
| } | ||
|
|
||
| func (j *JsonFlag) Type() string { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.