Merge pull request #8141 from mattfarina/fix-strvals-parsing

Fixing error with strvals parsing

pkg/strvals/parser.go

@@ -17,6 +17,7 @@ package strvals
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"strconv"
 	"strings"
@@ -149,7 +150,12 @@ func runeSet(r []rune) map[rune]bool {
 	return s
 }
 
-func (t *parser) key(data map[string]interface{}) error {
+func (t *parser) key(data map[string]interface{}) (reterr error) {
+	defer func() {
+		if r := recover(); r != nil {
+			reterr = fmt.Errorf("unable to parse key: %s", r)
+		}
+	}()
 	stop := runeSet([]rune{'=', '[', ',', '.'})
 	for {
 		switch k, last, err := runesUntil(t.sc, stop); {
@@ -230,14 +236,26 @@ func set(data map[string]interface{}, key string, val interface{}) {
 	data[key] = val
 }
 
-func setIndex(list []interface{}, index int, val interface{}) []interface{} {
+func setIndex(list []interface{}, index int, val interface{}) (l2 []interface{}, err error) {
+	// There are possible index values that are out of range on a target system
+	// causing a panic. This will catch the panic and return an error instead.
+	// The value of the index that causes a panic varies from system to system.
+	defer func() {
+		if r := recover(); r != nil {
+			err = fmt.Errorf("error processing index %d: %s", index, r)
+		}
+	}()
+
+	if index < 0 {
+		return list, fmt.Errorf("negative %d index not allowed", index)
+	}
 	if len(list) <= index {
 		newlist := make([]interface{}, index+1)
 		copy(newlist, list)
 		list = newlist
 	}
 	list[index] = val
-	return list
+	return list, nil
 }
 
 func (t *parser) keyIndex() (int, error) {
@@ -252,6 +270,9 @@ func (t *parser) keyIndex() (int, error) {
 
 }
 func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) {
+	if i < 0 {
+		return list, fmt.Errorf("negative %d index not allowed", i)
+	}
 	stop := runeSet([]rune{'[', '.', '='})
 	switch k, last, err := runesUntil(t.sc, stop); {
 	case len(k) > 0:
@@ -262,16 +283,19 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) {
 		vl, e := t.valList()
 		switch e {
 		case nil:
-			return setIndex(list, i, vl), nil
+			return setIndex(list, i, vl)
 		case io.EOF:
-			return setIndex(list, i, ""), err
+			return setIndex(list, i, "")
 		case ErrNotList:
 			rs, e := t.val()
 			if e != nil && e != io.EOF {
 				return list, e
 			}
 			v, e := t.reader(rs)
-			return setIndex(list, i, v), e
+			if e != nil {
+				return list, e
+			}
+			return setIndex(list, i, v)
 		default:
 			return list, e
 		}
@@ -283,7 +307,10 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) {
 		}
 		// Now we need to get the value after the ].
 		list2, err := t.listItem(list, i)
-		return setIndex(list, i, list2), err
+		if err != nil {
+			return list, err
+		}
+		return setIndex(list, i, list2)
 	case last == '.':
 		// We have a nested object. Send to t.key
 		inner := map[string]interface{}{}
@@ -299,7 +326,10 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) {
 
 		// Recurse
 		e := t.key(inner)
-		return setIndex(list, i, inner), e
+		if e != nil {
+			return list, e
+		}
+		return setIndex(list, i, inner)
 	default:
 		return nil, errors.Errorf("parse error: unexpected token %v", last)
 	}

pkg/strvals/parser_test.go

@@ -28,6 +28,7 @@ func TestSetIndex(t *testing.T) {
 		expect  []interface{}
 		add     int
 		val     int
+		err     bool
 	}{
 		{
 			name:    "short",
@@ -35,6 +36,7 @@ func TestSetIndex(t *testing.T) {
 			expect:  []interface{}{0, 1, 2},
 			add:     2,
 			val:     2,
+			err:     false,
 		},
 		{
 			name:    "equal",
@@ -42,6 +44,7 @@ func TestSetIndex(t *testing.T) {
 			expect:  []interface{}{0, 2},
 			add:     1,
 			val:     2,
+			err:     false,
 		},
 		{
 			name:    "long",
@@ -49,19 +52,43 @@ func TestSetIndex(t *testing.T) {
 			expect:  []interface{}{0, 1, 2, 4, 4, 5},
 			add:     3,
 			val:     4,
+			err:     false,
+		},
+		{
+			name:    "negative",
+			initial: []interface{}{0, 1, 2, 3, 4, 5},
+			expect:  []interface{}{0, 1, 2, 3, 4, 5},
+			add:     -1,
+			val:     4,
+			err:     true,
 		},
 	}
 
 	for _, tt := range tests {
-		got := setIndex(tt.initial, tt.add, tt.val)
+		got, err := setIndex(tt.initial, tt.add, tt.val)
+
+		if err != nil && tt.err == false {
+			t.Fatalf("%s: Expected no error but error returned", tt.name)
+		} else if err == nil && tt.err == true {
+			t.Fatalf("%s: Expected error but no error returned", tt.name)
+		}
+
 		if len(got) != len(tt.expect) {
 			t.Fatalf("%s: Expected length %d, got %d", tt.name, len(tt.expect), len(got))
 		}
 
+		if !tt.err {
 			if gg := got[tt.add].(int); gg != tt.val {
 				t.Errorf("%s, Expected value %d, got %d", tt.name, tt.val, gg)
 			}
 		}
+
+		for k, v := range got {
+			if v != tt.expect[k] {
+				t.Errorf("%s, Expected value %d, got %d", tt.name, tt.expect[k], v)
+			}
+		}
+	}
 }
 
 func TestParseSet(t *testing.T) {
@@ -271,6 +298,10 @@ func TestParseSet(t *testing.T) {
 				},
 			},
 		},
+		{
+			str: "list[0].foo=bar,list[-30].hello=world",
+			err: true,
+		},
 		{
 			str:    "list[0]=foo,list[1]=bar",
 			expect: map[string]interface{}{"list": []string{"foo", "bar"}},
@@ -283,6 +314,10 @@ func TestParseSet(t *testing.T) {
 			str:    "list[0]=foo,list[3]=bar",
 			expect: map[string]interface{}{"list": []interface{}{"foo", nil, nil, "bar"}},
 		},
+		{
+			str: "list[0]=foo,list[-20]=bar",
+			err: true,
+		},
 		{
 			str: "illegal[0]name.foo=bar",
 			err: true,
@@ -327,6 +362,10 @@ func TestParseSet(t *testing.T) {
 				},
 			},
 		},
+		{
+			str: "]={}].",
+			err: true,
+		},
 	}
 
 	for _, tt := range tests {