diff --git a/pql/ast.go b/pql/ast.go index d78ad2828..36d6e35fb 100644 --- a/pql/ast.go +++ b/pql/ast.go @@ -103,7 +103,9 @@ func (q *Query) endConditional() { func (q *Query) addField(field string) { elem := q.lastCallStackElem() - if elem == nil || elem.lastField != "" { + if elem == nil { + panic(fmt.Sprintf("addField called with '%s' while element is nil", field)) + } else if elem.lastField != "" { panic(fmt.Sprintf("addField called with '%s' while field is not empty, it's: %s", field, elem.lastField)) } elem.lastField = field @@ -112,6 +114,15 @@ func (q *Query) addField(field string) { } } +// validateArgField ensures that field does not already +// exist as a key in the Args map before adding the new +// key/value. +func (q *Query) validateArgField(elem *callStackElem) { + if _, exists := elem.call.Args[elem.lastField]; exists { + panic(fmt.Sprintf("multiple instances of argument '%s' provided", elem.lastField)) + } +} + func (q *Query) addVal(val interface{}) { elem := q.lastCallStackElem() if elem == nil || elem.lastField == "" { @@ -123,11 +134,13 @@ func (q *Query) addVal(val interface{}) { return } if elem.lastCond != ILLEGAL { + q.validateArgField(elem) // case 1 elem.call.Args[elem.lastField] = &Condition{ Op: elem.lastCond, Value: val, } } else { + q.validateArgField(elem) // case 2 elem.call.Args[elem.lastField] = val } elem.lastField = "" @@ -162,11 +175,13 @@ func (q *Query) addNumVal(val string) { } return } else if elem.lastCond != ILLEGAL { + q.validateArgField(elem) // case 3 elem.call.Args[elem.lastField] = &Condition{ Op: elem.lastCond, Value: ival, } } else { + q.validateArgField(elem) // case 4 elem.call.Args[elem.lastField] = ival } elem.lastField = "" @@ -175,6 +190,7 @@ func (q *Query) addNumVal(val string) { func (q *Query) startList() { elem := q.lastCallStackElem() + q.validateArgField(elem) // case 5 if elem.lastCond != ILLEGAL { elem.call.Args[elem.lastField] = &Condition{ Op: elem.lastCond, diff --git a/pql/pqlpeg_test.go b/pql/pqlpeg_test.go index 8492d3531..f8a6597b5 100644 --- a/pql/pqlpeg_test.go +++ b/pql/pqlpeg_test.go @@ -676,3 +676,55 @@ func TestPQLDeepEquality(t *testing.T) { }) } } + +func TestPQLPanic(t *testing.T) { + tests := []struct { + name string + call string + }{ + // case 1 + { + name: "StringConditional", + call: "Row(a==foo, a==bar)", + }, + // case 2 + { + name: "StringValue", + call: "Row(a=foo, a=bar)", + }, + // case 3 + { + name: "IntConditional", + call: "Row(a>5, a>6)", + }, + // case 4 + { + name: "IntValue", + call: "Row(a=7, a=8)", + }, + // case 5 + { + name: "List", + call: "Row(a=[7], a=[7,8])", + }, + } + for i, test := range tests { + t.Run(test.name+strconv.Itoa(i), func(t *testing.T) { + var v interface{} + func() { + defer func() { v = recover() }() + + q, err := ParseString(test.call) + if err != nil { + t.Fatalf("parsing query '%s': %v", test.call, err) + } + _ = q + }() + + if !reflect.DeepEqual(v, "multiple instances of argument 'a' provided") { + t.Fatalf("unexpected panic value: %#v", v) + } + + }) + } +}