Multiple expressions not supported in named matchers

disposedtrolley · January 6, 2022, 12:29am

1. Caddy version (`caddy version`):

v2.4.6 h1:HGkGICFGvyrodcqOOclHKfvJC0qTU7vny/7FhYp9hNw=

2. How I run Caddy:

a. System environment:

macOS 12.1 on amd64. Running the downloaded binary from GitHub releases.

b. Command:

ENVIRONMENT=production ./caddy run --config ~/Desktop/Caddyfile

c. Service/unit/compose file:

N/A

d. My complete Caddyfile or JSON config:

{
	debug
	auto_https off

	http_port 9080
}

http://* {
	@my_multiple_expression_matcher {
		expression "{query.foo} == \"bar\""
		expression "{env.ENVIRONMENT} == \"production\""
	}

	respond @my_multiple_expression_matcher 200 {
		body "Matched all expressions"
	}

	respond 200 {
		body "Matched some or no expressions"
	}
}

3. The problem I’m having:

I have a named matcher (my_multiple_expression_matcher containing two expression matchers. Both should evaluate to true in order to satisfy the entire matcher.

I run Caddy with ENVIRONMENT=production ./caddy run --config ~/Desktop/Caddyfile and hit the server with curl localhost:9080/dkfj. I should receive the body Matched some or no expressions since I haven’t provided the foo=bar query parameter, but instead I receive Matched all expressions.

I believe Caddy only supports a single expression type matcher despite the doco indicating that:

Multiple matchers of the same type may be combined (e.g. multiple path matchers in the same set) using boolean algebra (AND/OR), as described in their respective sections below.

The section on the expression matcher doesn’t indicate that multiple expressions aren’t supported.

I had a crack at debugging Caddy, and I believe the problem is here:

github.com/caddyserver/caddy

modules/caddyhttp/celmatcher.go

ecd5eeab3


      
          	for d.Next() {
          		m.Expr = strings.Join(d.RemainingArgs(), " ")
          	}
          	return nil

d.tokens contains tokens for both expressions, but it tries to parse both into a single statement (m.Expr) which means the second expression overwrites the first one. Here’s a screenshot of the debug session:

You can see that while all expressions have been correctly parsed, only the ENVIRONMENT == "production" expression is retained.

I admit that multiple expressions could be composed into a single compound expression; expression "{env.ENVIRONMENT} == \"production\" && {query.foo} == \"bar\"" in this case, but having them on separate lines helps with the clarity.

I’m happy to explore a potential fix (I understand the expression module is experimental, so I expected some rough edges) or maybe propose some changes to the documentation to make it clear that multiple expressions are not supported.

4. Error messages and/or full log output:

5. What I already tried:

6. Links to relevant resources:

francislavoie · January 6, 2022, 1:05am

This is a limitation of how the Caddyfile matchers are constructed, and what the resulting JSON looks like. Only one matcher of each type can end up in a matcher set, because they’re keyed by the matcher type.

See the JSON docs for the expression matcher:

Since MatchExpression actually get unmarshaled as a string into the Expr field (honestly not sure why it was done this way), for this to work in a backwards compatible way, the existing matcher string would need to be manipulated, probably by transforming the string like this:

(<existing>) && (<second expression>)

Which is kinda weird.

Breaking changes to it are probably acceptable since it’s still marked experimental – it could be changed from a string into a slice of strings (array of strings in JSON). The breaking change would probably only affect JSON users since the Caddyfile would just output different JSON to fix it, I guess. Existing Caddyfiles would remain backwards compatible.

FYI if you’re interested in playing around with the CEL code, there’s this open issue that could use some looking into, if you’re feeling adventurous:

github.com/caddyserver/caddy

Provide all core matchers as CEL functions

opened 04:51PM - 25 May 20 UTC

francislavoie

help wanted feature discussion

CEL is super cool and powerful, makes it easier to write complex conditional log…ic for request matching. We can use placeholders to pull the values from the request and write conditions with it. One thing that would make it even better though, is if all our existing request matchers were added to the CEL context as functions. Consider this Caddyfile (example of doing all the permutations of conditional logic): ``` localhost @both { path /abc/* method GET } respond @both "Both" @neither { not path /abc/* not method GET } respond @neither "Neither" @not_both { not { path /abc/* method GET } } respond @not_both "Not both" @either { not { not path /abc/* not method GET } } respond @either "Either" ``` This is pretty wacky. Especially the "either" case. If we had the matchers as CEL functions, we could do something like this for each of them (not all of these make sense as expressions, but this is just an example): ``` localhost @both expression `path("/abc/*") && method("GET")` respond @both "Both" @neither expression `!path("/abc/*") || !method("GET")` respond @neither "Neither" @not_both expression `!path("/abc/*") && !method("GET")` respond @not_both "Not both" @either expression `path("/abc/*") || method("GET")` respond @either "Either" ``` This reads a lot better, especially if you're comfortable with programming boolean logic.

disposedtrolley · January 6, 2022, 1:20am

Thanks @francislavoie, that makes sense!

I’m definitely out of my depth to make any meaningful contribution to the issue you linked but I’ll take the chance to prime myself on CEL.

I’ll try looking into the two approaches you proposed:

Composing multiple expressions into a single string during parsing.
Changing the JSON structure so the expression value is a string array rather than a string.