caddyhttp: Reject repeated expression matchers in a matcher set (#8183)

* caddyhttp: Combine repeated expression matchers in a matcher set

The Caddyfile parser groups repeated matchers of the same type and hands
all their segments to the module, but the expression matcher only read
the first segment, so every later `expression` line in a named matcher
or `not` block was dropped without an error. Read every segment and AND
the expressions, like the other matchers in a set.

* caddyhttp: Put each joined expression on its own line

A trailing // comment in one expression no longer swallows the closing
parenthesis and the expressions joined after it.

* caddyhttp: Reject repeated expression matchers instead of joining them

A second expression line in a named matcher or a not block is now a
Caddyfile error that points to combining them with && in one expression,
instead of being dropped or joined with an implied operator.

(cherry picked from commit d4165a4eb1)

modules/caddyhttp/celmatcher.go

@@ -243,25 +243,29 @@ func (m *MatchExpression) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	// CEL expression (e.g. strings) and we should retain that
 	if d.CountRemainingArgs() > 1 {
 		m.Expr = strings.Join(d.RemainingArgsRaw(), " ")
-		return nil
+	} else {
+		// there should at least be one arg
+		if !d.NextArg() {
+			return d.ArgErr()
+		}
+
+		// if there's only one token, then we can safely grab the
+		// cleaned token (no quotes) and use that as the expression
+		// because there's no valid CEL expression that is only a
+		// quoted string; commonly quotes are used in Caddyfile to
+		// define the expression
+		m.Expr = d.Val()
+
+		// use the named matcher's name, to fill regexp
+		// matchers names by default
+		m.Name = d.GetContextString(caddyfile.MatcherNameCtxKey)
 	}
 
-	// there should at least be one arg
-	if !d.NextArg() {
-		return d.ArgErr()
+	// a matcher set holds only one expression matcher
+	if d.Next() {
+		return d.Err("repeated expression matchers are not supported, combine them into one expression with &&")
 	}
 
-	// if there's only one token, then we can safely grab the
-	// cleaned token (no quotes) and use that as the expression
-	// because there's no valid CEL expression that is only a
-	// quoted string; commonly quotes are used in Caddyfile to
-	// define the expression
-	m.Expr = d.Val()
-
-	// use the named matcher's name, to fill regexp
-	// matchers names by default
-	m.Name = d.GetContextString(caddyfile.MatcherNameCtxKey)
-
 	return nil
 }
 

modules/caddyhttp/celmatcher_test.go

@@ -21,9 +21,11 @@ import (
 	"encoding/pem"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 )
 
 var (
@@ -605,3 +607,20 @@ func TestMatchExpressionProvision(t *testing.T) {
 		})
 	}
 }
+
+func TestMatchExpressionUnmarshalCaddyfileRepeated(t *testing.T) {
+	for _, input := range []string{`
+		expression {http.request.method} == 'GET'
+		expression {http.request.uri.path}.startsWith('/public/')
+	`, `
+		expression "{http.request.method} == 'GET'"
+		expression {http.request.uri.path}.startsWith('/public/')
+	`} {
+		d := caddyfile.NewTestDispenser(input)
+		m := new(MatchExpression)
+		err := m.UnmarshalCaddyfile(d)
+		if err == nil || !strings.Contains(err.Error(), "repeated expression matchers are not supported") {
+			t.Errorf("UnmarshalCaddyfile() error = %v, want repeated expression error (expression: %s)", err, m.Expr)
+		}
+	}
+}