Skip to content

Commit 5f44021

Browse files
authored
Check arg count when validating optFieldSelect (#1168)
Add a check for correct call shape before indexing into args in checkOptSelect. Manually constructed ASTs could lead to a panic.
1 parent 5de96a5 commit 5f44021

File tree

3 files changed

+55
-0
lines changed

3 files changed

+55
-0
lines changed

checker/checker.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,17 @@ func (c *checker) checkSelect(e ast.Expr) {
145145
func (c *checker) checkOptSelect(e ast.Expr) {
146146
// Collect metadata related to the opt select call packaged by the parser.
147147
call := e.AsCall()
148+
if len(call.Args()) != 2 || call.IsMemberFunction() {
149+
t := ""
150+
if call.IsMemberFunction() {
151+
t = " member call with"
152+
}
153+
c.errors.notAnOptionalFieldSelectionCall(e.ID(), c.location(e),
154+
fmt.Sprintf(
155+
"incorrect signature.%s argument count: %d%s", t, len(call.Args())))
156+
return
157+
}
158+
148159
operand := call.Args()[0]
149160
field := call.Args()[1]
150161
fieldName, isString := maybeUnwrapString(field)

checker/checker_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2595,6 +2595,46 @@ func TestCheckErrorData(t *testing.T) {
25952595
}
25962596
}
25972597

2598+
func TestCheckInvalidOptSelectMember(t *testing.T) {
2599+
fac := ast.NewExprFactory()
2600+
target := fac.NewStruct(1, "Foo", nil)
2601+
arg1 := fac.NewStruct(2, "Foo", nil)
2602+
arg2 := fac.NewLiteral(3, types.String("field"))
2603+
call := fac.NewMemberCall(4, "_?._", target, arg1, arg2)
2604+
2605+
// This is not valid syntax, just for illustration purposes.
2606+
src := common.NewTextSource("Foo{}._?._(Foo{}, 'field')")
2607+
parsed := ast.NewAST(call, ast.NewSourceInfo(src))
2608+
reg := newTestRegistry(t)
2609+
env, err := NewEnv(containers.DefaultContainer, reg)
2610+
if err != nil {
2611+
t.Fatalf("NewEnv(cont, reg) failed: %v", err)
2612+
}
2613+
_, iss := Check(parsed, src, env)
2614+
if !strings.Contains(iss.ToDisplayString(), "incorrect signature. member call") {
2615+
t.Errorf("got %s, wanted 'incorrect signature. member call'", iss.ToDisplayString())
2616+
}
2617+
}
2618+
2619+
func TestCheckInvalidOptSelectMissingArg(t *testing.T) {
2620+
fac := ast.NewExprFactory()
2621+
arg1 := fac.NewStruct(1, "Foo", nil)
2622+
call := fac.NewCall(2, "_?._", arg1)
2623+
2624+
// This is not valid syntax, just for illustration purposes.
2625+
src := common.NewTextSource("_?._(Foo{})")
2626+
parsed := ast.NewAST(call, ast.NewSourceInfo(src))
2627+
reg := newTestRegistry(t)
2628+
env, err := NewEnv(containers.DefaultContainer, reg)
2629+
if err != nil {
2630+
t.Fatalf("NewEnv(cont, reg) failed: %v", err)
2631+
}
2632+
_, iss := Check(parsed, src, env)
2633+
if !strings.Contains(iss.ToDisplayString(), "incorrect signature. argument count: 1") {
2634+
t.Errorf("got %s, wanted 'incorrect signature. argument count: 1'", iss.ToDisplayString())
2635+
}
2636+
}
2637+
25982638
func TestCheckInvalidLiteral(t *testing.T) {
25992639
fac := ast.NewExprFactory()
26002640
durLiteral := fac.NewLiteral(1, types.Duration{Duration: time.Second})

checker/errors.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ func (e *typeErrors) notAComprehensionRange(id int64, l common.Location, t *type
4545
FormatCELType(t))
4646
}
4747

48+
func (e *typeErrors) notAnOptionalFieldSelectionCall(id int64, l common.Location, err string) {
49+
e.errs.ReportErrorAtID(id, l, "unsupported optional field selection: %s", err)
50+
}
51+
4852
func (e *typeErrors) notAnOptionalFieldSelection(id int64, l common.Location, field ast.Expr) {
4953
e.errs.ReportErrorAtID(id, l, "unsupported optional field selection: %v", field)
5054
}

0 commit comments

Comments
 (0)