[PATCH 2/2] gzip+vary review - Aim for completeness of the Vary+gzip (+ ETag) tests., Remove Vary: Accept-Encoding for do_gunzip

Nils Goroll slink at schokola.de
Wed Aug 27 20:59:22 CEST 2014


As discussed on irc, I have reviewed the tests from
e8e089fc1968ee147fb06c60e3caae568bd11e40 (originally written by Federico) and
tried to improve coverage. In particular, I have also included checks for Etag
and Content-Encoding.

Results are:

* With do_gunzip, we end up with only an uncompressed object in cache and
varnish will never deliver it (re)-compressed. So there will only be one variant
(the plain one) and thus we remove must remove Accept-Encoding from Vary.

If we don't, downstream caches could unncessarily store two copies.

The change to cache_gzip.c implements this using http_RemoveHdrToken from the
previous patch.

One real world use case is to work around a misconfigured backend which, for
example, gzip-compresses jpg images. For this case we really do want to avoid
additional gzip overhead in varnish and the client, so we do want do_gunzip and
no Vary on A-E.

* There is a minor issue with do_gzip and Etags: When using do_gzip and
delivering to a non-gzip client, we restore the original byte stream by
gunzip'ing, so we could deliver the original (strong) Etag we might have
received from the backend - but we weaken the Etag because we have gzipp'ed at
fetch time.

Because fixing this would probably require additonal state in objects just for
this exotic case ("this is an object for which we originally received a strong
etag and gzipped it, so for this case only we may un-weaken the Etag"), I have
simply marked the case XXX in the vtc.

Other than that, while reviewing I noticed bug #1582

With this patch I am quite confident that we can really claim to have got gzip +
Vary + Etags right - finally - and closing #940 should be justified.

Thanks again, Federico
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Aim-for-completeness-of-the-Vary-gzip-ETag-tests.-Re.patch
Type: text/x-patch
Size: 17930 bytes
Desc: not available
URL: <https://www.varnish-cache.org/lists/pipermail/varnish-dev/attachments/20140827/59cf3e8f/attachment.bin>


More information about the varnish-dev mailing list