Skip to content

Commit

Permalink
fix: properly validate max index
Browse files Browse the repository at this point in the history
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
  • Loading branch information
phisco committed Mar 8, 2023
1 parent 53508a9 commit 1ee580d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 11 deletions.
20 changes: 16 additions & 4 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 @@ -370,6 +371,10 @@ func (p *Paved) setValue(s Segments, value any) error {
return errors.Wrap(err, "cannot unmarshal value from JSON")
}

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

var in any = p.object
for i, current := range s {
final := i == len(s)-1
Expand All @@ -381,10 +386,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 @@ -483,6 +484,17 @@ func (p *Paved) SetValue(path string, value any) error {
return p.setValue(segments, value)
}

func (p *Paved) validateSegments(s Segments) error {
if p.maxFieldPathIndexEnabled() {
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

0 comments on commit 1ee580d

Please sign in to comment.