Discussion:
vcl broken after upgrade from 4.1 to 5.0
Stefan Priebe - Profihost AG
2018-08-07 13:27:10 UTC
Permalink
Hello,

our varnish vcl is broken after upgrading from 4.1 to 5.0 but i can't
find any documentation hint and i also do not find any solution.

The old varnish conf uses std.collect to concatenate alls Set-Cookies
header from the backend.

Then does a lot of "magic" using regsuball.

At the end it uses the following Code to split the header into multiple
headers again:

rebsuball(cookies, ", ", {"
Set-Cookies: "})

{ allows \n / new lines so every new line starts with Set-Cookies: do
have multiple Set-Cookies headers.

This does not seem to work anymore with 5.0 the Set-Cookies Header is
still a one line. I was also not able to find the opposite of
std.collect to split the line again into multiple headers.

Thanks!

Greets,
Stefan
Dridi Boukelmoune
2018-08-07 18:10:07 UTC
Permalink
On Tue, Aug 7, 2018 at 3:27 PM, Stefan Priebe - Profihost AG
Post by Stefan Priebe - Profihost AG
Hello,
our varnish vcl is broken after upgrading from 4.1 to 5.0 but i can't
find any documentation hint and i also do not find any solution.
The old varnish conf uses std.collect to concatenate alls Set-Cookies
header from the backend.
Then does a lot of "magic" using regsuball.
At the end it uses the following Code to split the header into multiple
rebsuball(cookies, ", ", {"
Set-Cookies: "})
{ allows \n / new lines so every new line starts with Set-Cookies: do
have multiple Set-Cookies headers.
Wow, that's like evil clever!
Post by Stefan Priebe - Profihost AG
This does not seem to work anymore with 5.0 the Set-Cookies Header is
still a one line. I was also not able to find the opposite of
std.collect to split the line again into multiple headers.
That's a nice trick, and technically illegal although, well... Good
job for such a hack!

Some time after 4.1 std.collect grew support for an optional separator
other than ", " especially for brain-dead cookie use cases. FYI
cookies are such a mess because they contain a date formatted with a
comma followed by a space, but not quoted, so they can't be split like
other headers such as the Accept* headers.

To add insult to the injury, HTTP/2 requires cookies to be individual
headers nevertheless [1] to help HPACK compression effectiveness.
Since 5.0 is the first release with experimental h2 support, we may
have tightened headers handling and broken your (illegal but oh so
clever) hack.

I don't have time to look [2] closer now, but it would indeed be nice
to have the reciprocal operation to std.collect, so you could use a
safe [3] separator to collect, make your changes and then split using
the same separator.

std.collect(beresp.http.set-cookie, "~SEP~");
/* do stuff to beresp.http.set-cookie */
std.split(beresp.http.set-cookie, "~SEP~");

Dridi

[1] remember, impossible for h1
[2] but I'll find time to review a patch ;-)
[3] if you also control the backend in that regard
Stefan Priebe - Profihost AG
2018-08-08 08:41:36 UTC
Permalink
Hello,
Post by Dridi Boukelmoune
On Tue, Aug 7, 2018 at 3:27 PM, Stefan Priebe - Profihost AG
Post by Stefan Priebe - Profihost AG
Hello,
our varnish vcl is broken after upgrading from 4.1 to 5.0 but i can't
find any documentation hint and i also do not find any solution.
The old varnish conf uses std.collect to concatenate alls Set-Cookies
header from the backend.
Then does a lot of "magic" using regsuball.
At the end it uses the following Code to split the header into multiple
rebsuball(cookies, ", ", {"
Set-Cookies: "})
{ allows \n / new lines so every new line starts with Set-Cookies: do
have multiple Set-Cookies headers.
Wow, that's like evil clever!
Not my code ;-) I'm just the person who should fix it.
Post by Dridi Boukelmoune
Post by Stefan Priebe - Profihost AG
This does not seem to work anymore with 5.0 the Set-Cookies Header is
still a one line. I was also not able to find the opposite of
std.collect to split the line again into multiple headers.
That's a nice trick, and technically illegal although, well... Good
job for such a hack!
Some time after 4.1 std.collect grew support for an optional separator
other than ", " especially for brain-dead cookie use cases. FYI
cookies are such a mess because they contain a date formatted with a
comma followed by a space, but not quoted, so they can't be split like
other headers such as the Accept* headers.
To add insult to the injury, HTTP/2 requires cookies to be individual
headers nevertheless [1] to help HPACK compression effectiveness.
Since 5.0 is the first release with experimental h2 support, we may
have tightened headers handling and broken your (illegal but oh so
clever) hack.
Yes i know but we can't change the backend application so we need to
mess with their cookie stuff inside varnish ;-( and the vendor just says
use varnish 4.1 we don't support 5.X - most probably because the hack
isn't working anymore.
Post by Dridi Boukelmoune
I don't have time to look [2] closer now, but it would indeed be nice
to have the reciprocal operation to std.collect, so you could use a
safe [3] separator to collect, make your changes and then split using
the same separator.
std.collect(beresp.http.set-cookie, "~SEP~");
/* do stuff to beresp.http.set-cookie */
std.split(beresp.http.set-cookie, "~SEP~");
Yes but i'm sorry i'm unable to provide code. Is there any way to use
something like a loop or recursion? In that case i would be able to
modify a single set-cookie header like we need it.

I also wasn't able to solve it by mod_headers.

Thanks!

Greets,
Stefan
Dridi Boukelmoune
2018-08-08 12:14:48 UTC
Permalink
Post by Stefan Priebe - Profihost AG
Yes but i'm sorry i'm unable to provide code. Is there any way to use
something like a loop or recursion? In that case i would be able to
modify a single set-cookie header like we need it.
A "foreach" construct would indeed help for the case of multiple
headers and it has been discussed in the past but to no avail. It's
not trivial to add this to VCL and ensure soundness.
Post by Stefan Priebe - Profihost AG
I also wasn't able to solve it by mod_headers.
I'm not really surprised. This would require a VMOD of its own.

Did you look at alternatives such as vmod-cookie and vmod-cookieplus?

https://github.com/varnish/varnish-modules/blob/master/docs/vmod_cookie.rst#vmod_cookie
https://docs.varnish-software.com/varnish-cache-plus/vmods/cookieplus/#description

Maybe one of those provides enough capabilities for what you need to do.

Dridi
Stefan Priebe - Profihost AG
2018-08-08 13:07:28 UTC
Permalink
Post by Dridi Boukelmoune
Post by Stefan Priebe - Profihost AG
Yes but i'm sorry i'm unable to provide code. Is there any way to use
something like a loop or recursion? In that case i would be able to
modify a single set-cookie header like we need it.
A "foreach" construct would indeed help for the case of multiple
headers and it has been discussed in the past but to no avail. It's
not trivial to add this to VCL and ensure soundness.
Post by Stefan Priebe - Profihost AG
I also wasn't able to solve it by mod_headers.
I'm not really surprised. This would require a VMOD of its own.
Did you look at alternatives such as vmod-cookie and vmod-cookieplus?
https://github.com/varnish/varnish-modules/blob/master/docs/vmod_cookie.rst#vmod_cookie
https://docs.varnish-software.com/varnish-cache-plus/vmods/cookieplus/#description
Maybe one of those provides enough capabilities for what you need to do.
Thanks again. Sadly it seems they do not help.

The Problem is:
1.) there are multiple set-cookies headers from the backend
2.) the vcl than uses multiple regsuball to modify them
3.) the multiple set-cookies headers have to be part of the response in
the same order as before except for those who are empty after the regsuball

;-(

very ugly.

Stefan
Post by Dridi Boukelmoune
Dridi
Loading...