[BUG] (simulator) Editing header values using colons

[bungoume]

Describe the problem It seems that the result of rewriting the header value using colons differs from Fiddler's output. Could you please review the following case?

VCL code that cause the problem / reproduceable https://fiddle.fastly.dev/fiddle/da790883

// @scope: recv
// @suite: SET VARS VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "V";
    assert.equal(req.http.VARS, "VALUE=V");
}

// @scope: recv
// @suite: SET NOT-INITIALIZED VARS VALUE
sub test_recv {
    set req.http.VARS:VALUE = "V";
    assert.equal(req.http.VARS, "VALUE=V");
}

// @scope: recv
// @suite: SET MULTIPLE VARS VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "V";
    set req.http.VARS:VALUE2 = "V2";
    assert.equal(req.http.VARS, "VALUE=V, VALUE2=V2");
}

// @scope: recv
// @suite: SET EMPTY VARS VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "";
    assert.equal(req.http.VARS, "VALUE");
}

// @scope: recv
// @suite: SET MULTIPLE EMPTY VARS VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "";
    set req.http.VARS:VALUE2 = "";
    assert.equal(req.http.VARS, "VALUE, VALUE2");
}

// @scope: recv
// @suite: UNSET VARS ALL VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "V";
    unset req.http.VARS:VALUE;
    assert.is_notset(req.http.VARS);
}

// @scope: recv
// @suite: UNSET VARS VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "V";
    set req.http.VARS:VALUE2 = "V2";
    unset req.http.VARS:VALUE;
    assert.equal(req.http.VARS, "VALUE2=V2");
}

// @scope: recv
// @suite: OVERRIDE VARS VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "V";
    set req.http.VARS:VALUE = "O";
    assert.equal(req.http.VARS, "VALUE=O");
}

// @scope: recv
// @suite: SET NULL VALUE
sub test_recv {
    set req.http.VARS = "";
    set req.http.VARS:VALUE = "V";
    set req.http.VARS:VALUE = req.http.NULL;
    assert.equal(req.http.VARS, "VALUE");
}

Additional context https://github.com/bungoume/falco-vcl-empty-test/blob/main/tests/header_vars.test.vcl

[ysugimoto]

@bungoume I'm not figuring out the expected behavior without any details. Could you put more detail - expected and actual results on testing/interpreter please?

In my understanding, falco and Faslty's header does not use semicolon for multiple header setting.

It will help us:

1. Screenshot of unit test result (particularly, failing case)
2. Difference between falco and Fastly behavior (I could not understand in fiddle result)

[ysugimoto]

Ah i'm guessing you'd say about colon, not a semicolon like VARS:FOO. Is it correct?

[bungoume]

colon, not a semicolon

Yes, I'm sorry, I made a mistake in writing. I will correct the title.

[richardmarshall]

This is happening because of this line in setRequestHeaderValue (and the same issue occurs in setResponseHeaderValue): https://github.com/ysugimoto/falco/blob/c3ea5168b4f2cff0c4388b09954dd7ec6c378c76/interpreter/variable/header.go#L77

Since Add is used if the same header and field key is already set a duplicate entry is created instead of overriding the existing field value. When the field is read back the first of the two values is read and it appears as if the set did nothing.

Getting started on a fix for this, @ysugimoto you're welcome to assign the issue to me if you'd like. I should have time in the next few days to get this fixed.

[ysugimoto]

@richardmarshall Thanks, but I'm now working on the large change that relates to NotSet treating. It includes a defining falco particular HTTP header struct like golang http.Header to be fixed #235 and #237. Bear with me 🙏