Handling & correctly

[airween]

Based on this PR it seems that some engines (libmodsecurity3) allow the & sign with each variables (eg. REQUEST_BODY_LENGTH) even it makes no sense (what about Coraza?). Apache2 reports a weird message: Error creating rule: The & modificator does not apply to non-collection variables. but allows & in front of REQUEST_BODY although it's not a collection either.

We should decide what way do we want to follow: keep the parser as is now or need some modification to make it more strict.

@theseion, @fzipi, @dune73 - what do you think about?

@M4tteoP, @jptosso - how Coraza handles this syntax?

[jptosso]

In coraza it will compile but it will return 1. Because it counts the matched variables and single value variables are a single match

https://github.com/corazawaf/coraza/blob/650f02d4cd8087d1e32ff37fd171fe3ed6b1761f/internal/corazawaf/transaction.go#L603

[theseion]

As @jptosso says, it should return 1. We use it on collections to mean "does this variable exist", so all variables should follow the same semantics.

[airween]

As @jptosso says, it should return 1. We use it on collections to mean "does this variable exist", so all variables should follow the same semantics.

Thanks guys. @theseion do you think we should change this behavior in mod_security2?

[theseion]

Not sure. Maybe with a grace period. Add the new behavior with a compilation flag, log a warning (with deprecation message) when the old behavior is in effect and such a rule is parsed, then, at a later point drop the old behavior and the compilation flag.