(NULL == "") == true ?

[nigoroll]

I am pretty sure this has been discussed before, but I stumbled over it again and would like to revisit because I experienced it to be very un-pola:

We print a C NULL string as "", but a comparison with "" yields false.

[gquintard]

agreed, it's fine for C devs, but it's confusing for the rest of the population. Changing that is a major breaking change though, all the VCLs would need to at least be reviewed and possibly updated

[dridi]

As long as a BOOL evaluation of a HEADER can be used to test existence, I have no problem with HEADER == STRING to be degraded to STRING == STRING (and turning a non existent header into a string would result in an empty string, just like an assignment would).

[gquintard]

I think I'd prefer for the HEADER to be cast into a STRING and then into a BOOL, with "" being false-ish, and have a std.header_exists(HEADER) function to actually test presence.

[nigoroll]

The case here was with a vmod function returning NULL, but I think @gquintard 's suggestion has some merit to it: NULL could signify "undefined/unset", but still evaluate to "" if used as a string, then the following could hold true:

• req.http.inexistent-header == false
• vmod.returns_null() == false
• (req.http.inexistent-header == "") == true
• (vmod.returns_null() == "") == true

[nigoroll]

Summary of bugwash discussion:

The current proposal basically boils down to

• for type conversions/folding, preserve NULL: Typea(NULL) => Typeb(NULL)
• for string contexts, convert NULL to "": String(NULL) => ""
• for boolean contexts, convert NULL to false: BOOL(NULL) => false

So far, the rules do not require an exception for comparisons, the examples can be implemented with these rules only (and the existing string comparison).

Some aspects relating to String(NULL) => "" were discussed:

• If NULL handling was to be transitive, set would need to have unset semantics such that !req.http.a == !req.http.inexistent-header holds for

set req.http.a = req.http.inexistent-header;

• But as long as we want to preserve String(NULL) => "", such that in the example above, header a will be set to the empty string (as it is now), we will need to define BOOL("") == true such that BOOL(req.http.empty-header) == true and BOOL(req.http.inexistent-header) == false. This is the case with currecnt code.

It was argued that the empty string should evaluate to false such that BOOL(x) == BOOL(String(x)).

[dridi]

I tried a light survey of the current situation:

varnishtest cmp

varnish v1 -vcl {
	import debug;

	backend be none;

	sub vcl_recv {
		return (synth(200));
	}

	sub vcl_synth {
		set resp.http.noheader-eq-empty = req.http.noheader == "";
		set resp.http.empty-eq-noheader = "" == req.http.noheader;

		set resp.http.nostring-eq-empty = debug.null_string() == "";
		set resp.http.empty-eq-nostring = "" == debug.null_string();

		#set resp.http.nostrands-eq-empty = debug.null_strands() == "";
		#set resp.http.empty-eq-nostrands = "" == debug.null_strands();

		#set resp.http.nobackend-eq-empty = req.backend_hint == "";
		#set resp.http.empty-eq-nobackend = "" == req.backend_hint;

		#set resp.http.nobackend-eq-false = req.backend_hint == false;
		#set resp.http.false-eq-nobackend = false == req.backend_hint;

		if (req.backend_hint) {
			set resp.http.backend_hint = "true";
		} else {
			set resp.http.backend_hint = "false";
		}

		set resp.http.not-empty = !"";
		set resp.http.not-noheader = !req.http.noheader;
		set resp.http.not-nostring = !debug.null_string();
		set resp.http.not-nobackend = !req.backend_hint;
	}
} -start

client c1 {
	txreq
	rxresp

	expect resp.http.noheader-eq-empty == false
	expect resp.http.empty-eq-noheader == false

	expect resp.http.nostring-eq-empty == false
	expect resp.http.empty-eq-nostring == false

	#expect resp.http.nostrands-eq-empty == <panic>
	#expect resp.http.empty-eq-nostrands == <panic>

	#expect resp.http.nobackend-eq-empty == true
	#expect resp.http.empty-eq-nobackend == true

	#expect resp.http.nobackend-eq-false == <???>
	#expect resp.http.false-eq-nobackend == <???>

	expect resp.http.backend_hint == false

	expect resp.http.not-empty == false
	expect resp.http.not-noheader == true
	expect resp.http.not-nostring == true
	expect resp.http.not-nobackend == true
} -run

It relies on non missing vmod_debug trivial functions, and it looks like today we can't even generalize BOOL(x) == BOOL(String(x)), because BOOL(x) automatic conversion will not happen when prompted by a boolean operator.

VCL string comparison lines commented out currently don't compile without a dummy "" + x concatenation, but the commented out results reflect the outcome when that dummy concatenation is present.

So maybe the first step should be to fill the gap and allow TYPE cmp_operator STRING|BOOL to compile, if we consider it a gap.

edit: the initial test case completely broken.

[dridi]

A suggestion I'm making (but not implementing!) in #4309 is to turn none into a keyword (implemented) such that we could in the future use none for explicit null (or "lack of value") checks:

if (req.http.foo) { }
if (req.http.foo != none) { }

Both if statements being equivalent in the spirit of my suggestion.

[gquintard]

testing nullness this way work, but my (and Nils' I believe) is that "" is trueish, would we change that once we can req.http.foo != none?