From 7560fbc0415bf69c219b96949f3b2d2839538d56 Mon Sep 17 00:00:00 2001
From: Philippe Scorsolini
Date: Wed, 22 Feb 2023 18:16:32 +0100
Subject: [PATCH] fix: enforce max index value for paths
Signed-off-by: Philippe Scorsolini
---
pkg/fieldpath/paved.go | 37 ++++++++++++++++++++++++++++++++-----
pkg/fieldpath/paved_test.go | 36 +++++++++++++++++++++++++++++++++++-
2 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/pkg/fieldpath/paved.go b/pkg/fieldpath/paved.go
index bcfc90bad..1ef49c275 100644
--- a/pkg/fieldpath/paved.go
+++ b/pkg/fieldpath/paved.go
@@ -25,6 +25,9 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/errors"
)
+// DefaultMaxFieldPathIndex is the max allowed index in a field path.
+const DefaultMaxFieldPathIndex = 1024
+
type errNotFound struct {
error
}
@@ -46,19 +49,39 @@ func IsNotFound(err error) bool {
// A Paved JSON object supports getting and setting values by their field path.
type Paved struct {
- object map[string]any
+ object map[string]any
+ maxFieldPathIndex uint
}
+type PavedOption func(paved *Paved)
+
// PaveObject paves a runtime.Object, making it possible to get and set values
// by field path. o must be a non-nil pointer to an object.
-func PaveObject(o runtime.Object) (*Paved, error) {
+func PaveObject(o runtime.Object, opts ...PavedOption) (*Paved, error) {
u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(o)
- return Pave(u), errors.Wrap(err, "cannot convert object to unstructured data")
+ return Pave(u, opts...), errors.Wrap(err, "cannot convert object to unstructured data")
}
// Pave a JSON object, making it possible to get and set values by field path.
-func Pave(object map[string]any) *Paved {
- return &Paved{object: object}
+func Pave(object map[string]any, opts ...PavedOption) *Paved {
+ p := &Paved{object: object, maxFieldPathIndex: DefaultMaxFieldPathIndex}
+
+ for _, opt := range opts {
+ opt(p)
+ }
+
+ return p
+}
+
+// WithMaxFieldPathIndex returns a PavedOption that sets the max allowed index for field paths, 0 means no limit.
+func WithMaxFieldPathIndex(max uint) PavedOption {
+ return func(paved *Paved) {
+ paved.maxFieldPathIndex = max
+ }
+}
+
+func (p *Paved) maxFieldPathIndexEnabled() bool {
+ return p.maxFieldPathIndex > 0
}
// MarshalJSON to the underlying object.
@@ -358,6 +381,10 @@ 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
diff --git a/pkg/fieldpath/paved_test.go b/pkg/fieldpath/paved_test.go
index 1828b949c..ab0dbd9ee 100644
--- a/pkg/fieldpath/paved_test.go
+++ b/pkg/fieldpath/paved_test.go
@@ -17,6 +17,7 @@ limitations under the License.
package fieldpath
import (
+ "fmt"
"testing"
"github.com/google/go-cmp/cmp"
@@ -593,6 +594,7 @@ func TestSetValue(t *testing.T) {
type args struct {
path string
value any
+ opts []PavedOption
}
type want struct {
object map[string]any
@@ -737,6 +739,38 @@ func TestSetValue(t *testing.T) {
},
},
},
+ "RejectsHighIndexes": {
+ reason: "Paths having indexes above the maximum default value are rejected",
+ data: []byte(`{"data":["a"]}`),
+ args: args{
+ path: fmt.Sprintf("data[%v]", MaxFieldPathIndex+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"),
+ },
+ },
+ "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),
+ value: "c",
+ opts: []PavedOption{},
+ },
+ want: want{
+ object: map[string]any{
+ "data": func() []any {
+ res := make([]any, MaxFieldPathIndex+2)
+ res[0] = "a"
+ res[MaxFieldPathIndex+1] = "c"
+ return res
+ }()},
+ },
+ },
"MapStringString": {
reason: "A map of string to string should be converted to a map of string to any",
data: []byte(`{"metadata":{}}`),
@@ -817,7 +851,7 @@ func TestSetValue(t *testing.T) {
t.Run(name, func(t *testing.T) {
in := make(map[string]any)
_ = json.Unmarshal(tc.data, &in)
- p := Pave(in)
+ p := Pave(in, tc.args.opts...)
err := p.SetValue(tc.args.path, tc.args.value)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {