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

filter_except creates garbage in return string when req.url over a certain size #25

Closed
dbeckham opened this issue Jul 13, 2016 · 2 comments

Comments

@dbeckham
Copy link

Compiled and tested against Varnish 4.1.3 installed from the Varnish debian repo:

If req.url is over a certain size, the filter_except method will return a broken string where parts of the url are removed or replaced by parts of the query string.

Example varnish test that fails due to the req.url size:

varnishtest "Test filter_except querystring from req.url in vcl_hash"

server s1 {
        rxreq
        expect req.url ==  "/item/deal/1728366?refcode=xxxxx-xxxx-xxxxxx-xxxxxxx"
        txresp
} -start

varnish v1 -vcl+backend {
        import ${vmod_querystring};

        sub vcl_hash {
                set req.url = querystring.filter_except(req.url,
                                          "q" + querystring.filtersep() + "refcode");
        }
} -start

client c1 {
        txreq -url "/item/deal/1728366?refcode=xxxxx-xxxx-xxxxxx-xxxxxxx"
        rxresp
        expect resp.status == 200

        txreq -url "/item/deal/1728366?refcode=xxxxx-xxxx-xxxxxx-xxxxxxx&_=timestamp"
        rxresp
        expect resp.status == 200
}

varnish v1 -expect n_object == 0
varnish v1 -expect cache_miss == 0
varnish v1 -expect cache_hit == 0

client c1 -run
delay .1

varnish v1 -expect n_object == 1
varnish v1 -expect cache_miss == 1
varnish v1 -expect cache_hit == 1

If you replace the above test url with something smaller like /item/deal/1728366?refcode=xxxxx-xxxx-xxxxxx-xxx, the test will pass.

@dridi
Copy link
Owner

dridi commented Jul 15, 2016

Hello @dbeckham,

Thanks for reporting the bug and thank you for the test case too! However this test case passes for me, even with the latest 4.1.3.

Can you please attach the output of varnishtest -v for both the "too long" and "smaller" URLs?

Also, you may want to peek at the future of this VMOD in #23 since two of the reasons behind this change are the broken semantics of *_except functions and the filtersep function. You could also check whether your use case passes with this branch, feedback appreciated.

@dridi dridi closed this as completed in 8e8a721 Sep 16, 2016
@dridi
Copy link
Owner

dridi commented Sep 16, 2016

I closed this ticket because the related code no longer exists in 1.0, and I could never reproduce it. Please upgrade and open a new ticket if it happens again.

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

No branches or pull requests

2 participants