Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collapse Cookie headers in HTTP/2 when doing ESI. #2300

Closed
simonvik opened this issue Apr 3, 2017 · 2 comments
Closed

Collapse Cookie headers in HTTP/2 when doing ESI. #2300

simonvik opened this issue Apr 3, 2017 · 2 comments

Comments

@simonvik
Copy link

@simonvik simonvik commented Apr 3, 2017

#2291 solved the cookie collapsing towards backend but it does not seem to work when doing ESI.

Expected Behavior

That varnish collapse multiple cookie headers into one when doing ESI requests.

Current Behavior

Varnish sends multiple cookie headers to the sub-request.

Steps to Reproduce (for bugs)

varnishtest "ESI Cookie"

server s1 {
        rxreq
        expect req.http.cookie == "Foo=Bar; B=C"
        txresp -body "<html><esi:include src='/esi_test'/>"

        rxreq
        expect req.http.cookie == "Foo=Bar; B=C"
        txresp
} -start

varnish v1 -cliok {param.set feature +http2}


varnish v1 -vcl+backend  {
        sub vcl_backend_response {
                set beresp.do_esi = true;
        }
} -start

client c1 {
        stream 1 {
                txreq -url "/" -hdr cookie "Foo=Bar" -hdr cookie "B=C"
                rxresp
        } -start
} -run

Your Environment

  • Version used: Varnish 5.1.1 (patched to b758894)
  • Operating System and version: Ubuntu 16.04/Arch
@hermunn
Copy link
Member

@hermunn hermunn commented Apr 5, 2017

Backport review: H2 is not in Varnish 4.1, so there is nothing to backport here.

@Dridi
Copy link
Member

@Dridi Dridi commented Apr 10, 2017

Post-mortem: it's one of my rules of thumb to check ESI behavior for anything involving (be)?(req|resp) variables in VCL and I completely forgot it for this change. Thanks @simonvik for catching it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.