vmod_cookie: cookie.delete does unexpected things with Set-Cookie in vcl_backend_response

[mss]

Abstract

I'm not sure if this is really a code bug, a documentation issue, or a feature request. The manpage for vmod_cookie never mentions that it should work reliably with the Set-Cookie header but it looks like everybody is using it (eg. https://info.varnish-software.com/blog/sticky-session-with-cookies) since Set-Cookie isn't collapsible and some VMOD is needed.

If this is expected behaviour I think the man page should state which functions are safe to use on Set-Cookie and which not.

Example

Take the following stripped down VCL snippet:

sub vcl_backend_response {
    cookie.parse(beresp.http.Set-Cookie);

    cookie.delete("SESSION");
    set beresp.http.Set-Cookie = cookie.get_string();
    if (beresp.http.Set-Cookie == "") {
        unset beresp.http.Set-Cookie;
    }
}

and the following header:

Set-Cookie: SESSION=ffbafc72-a1ca-412d-ad33-1587045d2750; Max-Age=31536000; path=/;

Expected Result

No Set-Cookie header in the response.

Actual Result

I now have a cookie called Max-Age:

Set-Cookie: Max-Age=31536000; path=/;

From an implementation PoV this behaviour does make total sense (how is parse supposed to know if it is a Cookie or a Set-Cookie header it is looking at), it is also totally unexpected.

[gquintard]

I actually don't see the issue :-) Written as it is, you have three cookies, SESSION, Max-Age and path. If you want the last two to be attributes of SESSION, you should use commas, not semi-colons.

Closing until proven wrong.

[mss]

Somebody is confused here and it might indeed be me. Let me re-read the RFC to get up to speed with the Set-Cookie header syntax again and get back to you. All I remember right now is that it was odd and thus the header isn't collapsible.

This is from a real world (Spring Boot) example though:

$ curl -s -I https://db-planet.deutschebahn.com/web/ | grep SESSION                                                                                                                         
Set-Cookie: COYOSESSION=7fab6947-ad07-445a-a8f5-7a2bb0f2ea4c;Max-Age=31536000;path=/;Secure;HttpOnly

[gquintard]

Oooooooooooor, it might be me :-) I remembered that the cookie and set-cookie headers had the same syntax.

Apparently, that would have been too easy...

So, indeed, vmod-cookie shouldn't be used for set-cookie.

[mss]

Ok, I'm glad I didn't discover yet another corner case of cookie handling :smile:

I guess this is a documentation issue then and the vmod_cookie man page should point to vmod_header for setting cookies.

Removing a cookie is actually rather trivial, rewriting one like you do in your blog post quickly gets unwieldy.It would be nice if vmod_cookie could use vmod_header so one could use the syntactic sugar of the former.

[gquintard]

Actually: https://github.com/varnish/varnish-modules/blob/master/src/vmod_cookie.vcc#L13 :-p

But thanks for pointing that out, I'll update the blog post and credit you, if you're okay with it.

[mss]

Yeah, I read that part but thought it was related only to setting new cookies with that helper.

Thanks for the credits when you update the post!

An idea: Maybe vmod_cookie could get two more functions, parse_req(HTTP http) and parse_resp(HTTP http)'. The former would be the same as calling parse(http.Cookie) while the latter has its own logic. Then vmod_cookie could maybe do The Rigth Thing™ depending on context.

[gquintard]

@mss: Just so you know, I corrected the article, the new version should be online shortly. Thanks again for taking the time to correct me.