vars: Make nil values act as empty string instead of "<nil>"

[francislavoie]

This is a silly little bug.

I was looking into a possible config to fix an issue for Authelia, and while testing I noticed that vars and vars_regexp matchers were turning placeholders that return no value (nil) into literally the string "<nil>" because of the default: case doing fmt.Sprintf("%v", vv).

This means that it was impossible to match against nil placeholders by checking for empty string, because the value would always be non-empty. So this fixes a matchers like this:

@foo not vars {rp.header.Foo} ""
@bar vars_regexp bar {rp.header.Bar} ^(.+)$

These matchers when inside a proxy handle_response should not match when the response had a non-empty header value (first one because of not negating empty string, and the regexp requiring at least one character), but instead it did match because the actual string was <nil>.

[mholt]

I suppose this is better... thanks. I'm not 100% convinced it will be for everyone, though, as in some cases I could see a nil value wanting to be explicitly printed as nil, as opposed to empty, but, let's see how it goes 👍

[christophcemper]

I disagree with the assertion that an empty string and nil can be aliased @mholt @francislavoie

"" is simple not the same as nil/null.

Just like in databases, those are two very different "values"

1/ a definitive, distinct value ("")
2/ a mathematically endless number of possibilities (null, nil)

Simply equaling those two has never ended well, based on my experience - especially in a late stage with existing data, config, etc.

Haven't finished research the full impact, but this is one major difference in handling data between a stable 2.7.6 and a crashing 2.8.4. we found so far when trying to understand those 343 changed files with 17,585 additions and 6,030 deletions leading to our repeated unreproducible production crash

[christophcemper]

On the root issue

This means that it was impossible to match against nil placeholders by checking for empty string, because the value would >always be non-empty.

I don't understand why that was a requirement.
Could you not have checked against "<nil>" to solve this requirement?

We have not had this use case, so may be missing something,
but how is this change backwards-compatible with anyone who did check for "" that way in the past?

[francislavoie]

Using a special constant string for nil is not better than using the known zero value of a string. It simply moves the goalpost.

If you really need to match for exactly nil, then you can use the expression matcher which lets you get access to the actual underlying type (because CEL maps Go types to its internal type system). The vars and vars_regexp matchers are intended to work on strings (because that's what users have access to at the config level), not Go types. For example:

@nil-var `{vars.some-var} == nil`
respond @nil-var "Yep it's nil"

[christophcemper]

Using a special constant string for nil is not better than using the known zero value of a string.

The current solution equals/aliases them, when they are not equal.

Having 2 separate strings "" and "" would actually still be better
(yet not fully representing the math speciality of infinite nil-results)

If you really need to match for exactly nil...

Interesting, that's a new, thanks!

[francislavoie]

The current solution equals/aliases them, when they are not equal.

And using a special string like "<nil>" would similarly overload a legitimate string with that value that wasn't the type nil. It's not better. It also leaks internal implementation details (nil is a Go-ism, most other languages use null for example, and the <> is an arbitrary choice by Go's string formatter)

[mholt]

@christophcemper

"" is simple not the same as nil/null.

I agree, but I think the context of this issue is that printing a nil value as a string of "<nil>" is less helpful than a literal empty string. When performing type-weak operations in Caddy config (and many other scripting languages), you would expect to compare against "" not "<nil>".

As Francis mentioned, you can use CEL for proper type-safe comparison. But the built-in matcher configs that only get string inputs have no good way to represent nil other than "" (surely you wouldn't write "<nil>" to represent nil).