From bade6478fcdd537957f0ce8a2cc5e06a14940ea0 Mon Sep 17 00:00:00 2001
From: Matt Farina <matt@mattfarina.com>
Date: Fri, 15 May 2020 13:23:10 -0400
Subject: [PATCH] Fixing error with strvals parsing

Closes #8140

Signed-off-by: Matt Farina <matt@mattfarina.com>
---
 pkg/strvals/parser.go      | 30 +++++++++++++++++++++-------
 pkg/strvals/parser_test.go | 41 +++++++++++++++++++++++++++++++++++---
 2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/pkg/strvals/parser.go b/pkg/strvals/parser.go
index 03adbd3cb..db2fb60fe 100644
--- a/pkg/strvals/parser.go
+++ b/pkg/strvals/parser.go
@@ -17,6 +17,7 @@ package strvals
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"strconv"
 	"strings"
@@ -230,14 +231,17 @@ 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{}) ([]interface{}, error) {
+	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 +256,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 +269,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 +293,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 +312,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)
 	}
diff --git a/pkg/strvals/parser_test.go b/pkg/strvals/parser_test.go
index 44d317220..fb18980cf 100644
--- a/pkg/strvals/parser_test.go
+++ b/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,17 +52,41 @@ 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 gg := got[tt.add].(int); gg != tt.val {
-			t.Errorf("%s, Expected value %d, got %d", tt.name, tt.val, gg)
+		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)
+			}
 		}
 	}
 }
@@ -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,