Skip to content
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

fix: properly validate max index #389

Merged
merged 1 commit into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 30 additions & 9 deletions pkg/fieldpath/paved.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type Paved struct {
maxFieldPathIndex uint
}

// PavedOption can be used to configure a Paved behavior.
type PavedOption func(paved *Paved)

// PaveObject paves a runtime.Object, making it possible to get and set values
Expand Down Expand Up @@ -361,13 +362,13 @@ func (p *Paved) setValue(s Segments, value any) error {
// any per https://golang.org/pkg/encoding/json/#Unmarshal. We
// marshal our value to JSON and unmarshal it into an any to ensure
// it meets these criteria before setting it within p.object.
var v any
j, err := json.Marshal(value)
v, err := toValidJSON(value)
if err != nil {
return errors.Wrap(err, "cannot marshal value to JSON")
return err
}
if err := json.Unmarshal(j, &v); err != nil {
return errors.Wrap(err, "cannot unmarshal value from JSON")

if err := p.validateSegments(s); err != nil {
return err
}

var in any = p.object
Expand All @@ -381,10 +382,6 @@ func (p *Paved) setValue(s Segments, value any) error {
return errors.Errorf("%s is not an array", s[:i])
}

if p.maxFieldPathIndexEnabled() && current.Index > p.maxFieldPathIndex {
return errors.Errorf("index %d is greater than max allowed index %d", current.Index, p.maxFieldPathIndex)
}

if final {
array[current.Index] = v
return nil
Expand Down Expand Up @@ -412,6 +409,18 @@ func (p *Paved) setValue(s Segments, value any) error {
return nil
}

func toValidJSON(value any) (any, error) {
var v any
j, err := json.Marshal(value)
if err != nil {
return nil, errors.Wrap(err, "cannot marshal value to JSON")
}
if err := json.Unmarshal(j, &v); err != nil {
return nil, errors.Wrap(err, "cannot unmarshal value from JSON")
}
return v, nil
}

func prepareElement(array []any, current, next Segment) {
// If this segment is not the final one and doesn't exist we need to
// create it for our next segment.
Expand Down Expand Up @@ -483,6 +492,18 @@ func (p *Paved) SetValue(path string, value any) error {
return p.setValue(segments, value)
}

func (p *Paved) validateSegments(s Segments) error {
if !p.maxFieldPathIndexEnabled() {
return nil
}
for _, segment := range s {
if segment.Type == SegmentIndex && segment.Index > p.maxFieldPathIndex {
return errors.Errorf("index %v is greater than max allowed index %d", segment.Index, p.maxFieldPathIndex)
}
}
return nil
}

// SetString value at the supplied field path.
func (p *Paved) SetString(path, value string) error {
return p.SetValue(path, value)
Expand Down
14 changes: 7 additions & 7 deletions pkg/fieldpath/paved_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -743,30 +743,30 @@ func TestSetValue(t *testing.T) {
reason: "Paths having indexes above the maximum default value are rejected",
data: []byte(`{"data":["a"]}`),
args: args{
path: fmt.Sprintf("data[%v]", MaxFieldPathIndex+1),
path: fmt.Sprintf("data[%v]", DefaultMaxFieldPathIndex+1),
value: "c",
},
want: want{
object: map[string]any{
"data": []any{"a"}},
err: errors.Wrap(errors.Errorf("found index above max (%[1]v > %[2]v): data[%[1]v]",
MaxFieldPathIndex+1, MaxFieldPathIndex), "invalid segments"),
err: errors.Errorf("index %v is greater than max allowed index %v",
DefaultMaxFieldPathIndex+1, DefaultMaxFieldPathIndex),
},
},
"NotRejectsHighIndexesIfNoDefaultOptions": {
reason: "Paths having indexes above the maximum default value are not rejected if default disabled",
data: []byte(`{"data":["a"]}`),
args: args{
path: fmt.Sprintf("data[%v]", MaxFieldPathIndex+1),
path: fmt.Sprintf("data[%v]", DefaultMaxFieldPathIndex+1),
value: "c",
opts: []PavedOption{},
opts: []PavedOption{WithMaxFieldPathIndex(0)},
},
want: want{
object: map[string]any{
"data": func() []any {
res := make([]any, MaxFieldPathIndex+2)
res := make([]any, DefaultMaxFieldPathIndex+2)
res[0] = "a"
res[MaxFieldPathIndex+1] = "c"
res[DefaultMaxFieldPathIndex+1] = "c"
return res
}()},
},
Expand Down