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

Multiple Vary headers emitted #1064

Closed
oschaaf opened this Issue Dec 9, 2015 · 35 comments

Comments

Projects
None yet
8 participants
@oschaaf
Copy link
Member

oschaaf commented Dec 9, 2015

On trunk-tracking, we somehow end up emitting three Vary: Accept-Encoding headers.

$ curl -I -X GET http://192.168.137.10:8090/mod_pagespeed_example/styles/index_style.css
HTTP/1.1 200 OK
Server: nginx/1.8.0
Content-Type: text/css
Connection: keep-alive
Vary: Accept-Encoding
ETag: "5666b969-2c7"
Accept-Ranges: bytes
Date: Wed, 09 Dec 2015 15:51:53 GMT
Expires: Wed, 09 Dec 2015 15:56:53 GMT
Cache-Control: max-age=300
X-Original-Content-Length: 711
Vary: Accept-Encoding
Vary: Accept-Encoding
Content-Length: 711

nginx.conf:

server {
    listen       8090;
    server_name  localhost;
    pagespeed on;
    pagespeed FileCachePath /tmp/ps;
    pagespeed RewriteLevel PassThrough;
    pagespeed CssPreserveUrls on;
    pagespeed EnableFilters prioritize_critical_css,debug,extend_cache;

    location / {
        root   /var/www/;
        index  index.html index.htm;
    }
}
@deweydb

This comment has been minimized.

Copy link

deweydb commented Dec 10, 2015

Not sure if related, but i was having this issue when i had: gzip_vary on; in my http block, instead of the server block.

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Dec 11, 2015

@jeffkaufman is this worthy of being on the work-prioritization list?

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 5, 2016

Added

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 26, 2016

Reminder, please.. would love to have this fixed too (request of one of my clients). 😃

@Niwreg

This comment has been minimized.

Copy link

Niwreg commented Jan 26, 2016

Same issue here. I'll follow the thread in case of updates :)

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Jan 27, 2016

Fix for this is in progress at https://github.com/pagespeed/ngx_pagespeed/tree/oschaaf-trunk-tracking-issue-1064, working on tests to prevent regression.
I have yet to reproduce the situation again where we emit three Vary: Accept-Encoding headers which somehow seems hard now, but this fix gets rid of a reproducible double Vary: Accept-Encoding situation.
If anyone is up for trying the patch, here it is: https://github.com/pagespeed/ngx_pagespeed/compare/trunk-tracking...oschaaf-trunk-tracking-issue-1064.patch

oschaaf added a commit that referenced this issue Jan 28, 2016

vary-header: Emit a single vary header in the IPRO flow
The report from some time ago mentioned three Vary: headers,
but I can now only reproduce two using trunk-tracking plus the
original repro-configuration.

This fix unflags r->gzip_vary as set by the gzip module when PSOL
hands us Vary: Accept-Encoding, to make sure that nginx's core
header filter doesn't append another one.

Fixes #1064
@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 28, 2016

@oschaaf thanks, I applied the patch successfully (without errors) and recompiled Nginx with it.
Unfortunately, still the issue.
The double Vary headers appear after accessing the URL for the 2nd time.

First time:

$ curl -I https://www.jennifersauer.nl/wp-includes/js/jquery/jquery.js,qver=1.11.4.pagespeed.jm.zixJPNMRNN.js
HTTP/1.1 200 OK
Content-Type: application/javascript
Connection: keep-alive
Keep-Alive: timeout=300
Server: nginx
Content-Length: 95977
Last-Modified: Tue, 05 Jan 2016 19:30:28 GMT
Vary: Accept-Encoding
Access-Control-Allow-Origin: *
Accept-Ranges: bytes
Date: Fri, 29 Jan 2016 15:45:43 GMT
Expires: Fri, 29 Jan 2016 15:50:43 GMT
Cache-Control: max-age=300,private
X-Page-Speed: 1.10.33.2-7600

Second time:

$ curl -I https://www.jennifersauer.nl/wp-includes/js/jquery/jquery.js,qver=1.11.4.pagespeed.jm.zixJPNMRNN.js
HTTP/1.1 200 OK
Content-Type: application/javascript
Connection: keep-alive
Keep-Alive: timeout=300
Server: nginx
Access-Control-Allow-Origin: *
Accept-Ranges: bytes
Date: Fri, 29 Jan 2016 15:45:43 GMT
Expires: Sat, 28 Jan 2017 15:45:43 GMT
Cache-Control: max-age=31536000
ETag: W/"0"
Last-Modified: Fri, 29 Jan 2016 15:45:43 GMT
X-Original-Content-Length: 95889
Vary: Accept-Encoding
Vary: Accept-Encoding
Content-Length: 95889
X-Page-Speed: 1.10.33.2-7600

Just try it yourself, change qver=1.11.4 to qver=1.11.5 or 6 or what you want, it's just a variable. The double Vary headers appear after the 2nd access.

It's only with rewritten sources, normal sources are fine:

$ curl -I "https://www.jennifersauer.nl/wp-includes/js/wp-emoji-release.min.js?ver=4.4.2"
HTTP/1.1 200 OK
Server: nginx
Date: Thu, 28 Jan 2016 23:14:10 GMT
Content-Type: application/javascript
Content-Length: 33713
Last-Modified: Thu, 07 Jan 2016 07:19:03 GMT
Connection: keep-alive
Keep-Alive: timeout=300
Vary: Accept-Encoding
ETag: "568e1167-83b1"
Expires: Thu, 31 Dec 2037 23:55:55 GMT
Cache-Control: max-age=315360000
Access-Control-Allow-Origin: *
Cache-Control: public, must-revalidate, proxy-revalidate
Accept-Ranges: bytes

I've purged all the caches (pagespeed via memcached) and gave the whole VPS a reboot to make sure not battling with the caches, but still no avail. Let me know if you need more information.

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Jan 29, 2016

It took me a little while, but I found the old directory where I could reproduce writing out three Vary:Accept-Encoding response headers.

Applying #1105 there, the number of Vary: Accept-Encoding headers goes down from three to two on the optimized IPRO and .pagespeed. responses for me.

The two remaining Vary: Accept-Encoding headers originate from PSOL in both cases, and it seems this duplication has already been fixed on mod_pagespeed's master. With #1105 applied and build against the current state of mps master I can't reproduce the problem: a single Vary: Accept-Encoding gets emitted every time.

What is surprising is that your website continues to emit three Vary:Accept-Encoding headers with #1105 applied: I would at least have expected one of them to be removed.. Which makes me wonder: how did you clear PageSpeed's caches? The "official" way or by directly purging the memcached instance(s)?

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 29, 2016

@oschaaf yes, strange isn't it? I cleared the cache by deleting the files inside /var/cache/ngx_pagespeed and I rebooted the server (so memcached would be 100% fresh and empty too).
There are no Wordpress cache plugins installed (except the nginx helper plugin). I also cleared the nginx fastcgi cache just to be sure, by clearing /var/run/nginx-cache.

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Jan 29, 2016

@HansVanEijsden: Are you sure you fired up the patched nginx binary? Is it possible that the old (unpatched) nginx server was/is still running? I can't yet come up with a reason that would explain how it could be that the patch has zero effect, but there may be something in your configuration or module list that bars the patch from working correctly (though even if the patched worked, the problem would persist but in reduced form, because ngx_pagespeed would also need to be build against a newer PSOL version to eliminate the problem completely).

If you are sure we're looking at a patched nginx instance, what does nginx -V look like? Could you PM your configuration files?

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 30, 2016

@oschaaf unfortunately.. yes, I'm sure about that.
To be sure, I've done a complete re-install of ngx_pagespeed and nginx.

I installed ngx_pagespeed following the instructions in #1099 and I also applied the cherry-pick.
Then, I applied your patch:

hans@vps:~/release-1.10.33.2-beta$ patch -p1 < trunk-tracking...oschaaf-trunk-tracking-issue-1064.patch
patching file src/ngx_pagespeed.cc
Hunk #1 succeeded at 571 (offset -1 lines).
Hunk #2 succeeded at 1274 (offset -12 lines).
Hunk #3 succeeded at 1845 (offset -12 lines).
Hunk #4 succeeded at 2163 (offset -12 lines).
patching file src/ngx_pagespeed.h
patching file test/nginx_system_test.sh
Hunk #1 succeeded at 1241 with fuzz 2 (offset -7 lines).
patching file test/pagespeed_test.conf.template
Hunk #1 succeeded at 1463 (offset -8 lines).

After re-installing nginx I gave the whole server a reboot, to make really really sure no old binaries to be active.
Here's my nginx -V:

$ /opt/nginx19/sbin/nginx -V
nginx version: nginx/1.9.10
built by gcc 4.9.2 (Debian 4.9.2-10)
built with OpenSSL 1.0.2f 28 Jan 2016
TLS SNI support enabled
configure arguments: --add-module=/home/hans/release-1.10.33.2-beta --prefix=/opt/nginx19 --user=www-data --group=www-data --with-http_ssl_module --with-http_v2_module --with-openssl=/usr/local/src/openssl-1.0.2f --with-openssl-opt='enable-ec_nistp_64_gcc_128 threads' --with-md5=/usr/local/src/openssl-1.0.2f--with-md5-asm --with-sha1=/usr/local/src/openssl-1.0.2f --with-sha1-asm --with-pcre-jit --with-file-aio --with-http_flv_module --with-http_geoip_module --with-http_gzip_static_module --with-http_gunzip_module --with-http_mp4_module --with-http_realip_module --with-http_stub_status_module --with-threads --with-ipv6 --add-module=/usr/local/src/nginx-rtmp-module --add-module=/usr/local/src/ngx_cache_purge-2.3 --add-module=/usr/local/src/ngx_http_substitutions_filter_module --with-cc-opt='-DTCP_FASTOPEN=23 -O3 -march=native'

$ uname -a
Linux vps 4.3.0-0.bpo.1-amd64 #1 SMP Debian 4.3.3-7~bpo8+1 (2016-01-19) x86_64 GNU/Linux

I sent you my config files by mail. 😃

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Jan 30, 2016

Thanks @HansVanEijsden
I PM'd you another patch, let me know if that results in a single Vary: Accept-Encoding header being emitted consistently.
If it does, that means that #1105 is probably all we need for the nps side to resolve this.

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Jan 30, 2016

@oschaaf I own you a beer.
The patch you PM'ed works! 😄 😄

@nirocfz

This comment has been minimized.

Copy link

nirocfz commented Feb 4, 2016

There are two Vary: Accept-Encoding headers after I applied #1105, how to make it a single Vary: Accept-Encoding?

@JoyceBabu

This comment has been minimized.

Copy link

JoyceBabu commented Feb 4, 2016

I am also seeing seeing two Vary headers after applying #1105.

@oschaaf Can you please share the patch for removing the extra header?

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Feb 4, 2016

@nirocfz

This comment has been minimized.

Copy link

nirocfz commented Feb 4, 2016

@oschaaf That's great!

@JoyceBabu

This comment has been minimized.

Copy link

JoyceBabu commented Feb 4, 2016

I tried to apply it to release 1.10.33.4 with commits b081bb7 (#1105) and 059dd20(#1099) cherry picked. It failed with

patching file src/ngx_pagespeed.cc
Hunk #1 succeeded at 493 (offset 1 line).
Hunk #2 FAILED at 569.
Hunk #3 FAILED at 1267.
Hunk #4 FAILED at 1837.
Hunk #5 FAILED at 2154.
@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Feb 4, 2016

@JoyceBabu the gist I posted should apply cleanly when b081bb7 (#1105) is not cherry-picked.

@JoyceBabu

This comment has been minimized.

Copy link

JoyceBabu commented Feb 4, 2016

Thank you @oschaaf. I have successfully applied the patch, and now there is only one Vary header.

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Feb 18, 2016

After updating to 1.10.33.5 the issue still persists. I had to re-apply this patch to make it work.

@centminmod

This comment has been minimized.

Copy link

centminmod commented Feb 19, 2016

I just switched to using downstream proxy cache and 1.10.33.5 as a dynamic module and seeing 3 sets of Vary: Accept-Encoding headers

and yes only with rewritten pagespeed resources as well

curl -I http://centminmod.com/css/A.localfonts.css+font-awesome.min.css+bootstrap.min.css+hover-dropdown-menu.css+icons-set8.css+animate.min.css+style.css+responsive.css+color.css,Mcc.CSme7w21FI.css.pagespeed.cf.L0MXByuJj_.css
HTTP/1.1 200 OK
Content-Type: text/css
Connection: keep-alive
Vary: Accept-Encoding
Server: nginx centminmod
X-Powered-By: centminmod proxy
X-Cache-Status: MISS
Accept-Ranges: bytes
Date: Fri, 19 Feb 2016 17:42:59 GMT
Expires: Sat, 18 Feb 2017 17:42:59 GMT
Cache-Control: max-age=31536000
ETag: W/"0"
Last-Modified: Fri, 19 Feb 2016 17:42:59 GMT
X-Original-Content-Length: 352288
Vary: Accept-Encoding
Vary: Accept-Encoding
Content-Length: 352288
X-Page-Speed: 1.10.33.5-0

using CentOS 6.7 and 7.2 for my site at centminmod.com and I also have ngx_brotli and other modules enabled

nginx -V
nginx version: nginx/1.9.11
built by gcc 4.8.5 20150623 (Red Hat 4.8.5-4) (GCC)
built with OpenSSL 1.0.2f 28 Jan 2016
TLS SNI support enabled
configure arguments: --with-ld-opt='-ljemalloc -Wl,-z,relro -Wl,-rpath,/usr/local/lib' --with-cc-opt='-m64 -mtune=native -mfpmath=sse -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2' --sbin-path=/usr/local/sbin/nginx --conf-path=/usr/local/nginx/conf/nginx.conf --with-http_stub_status_module --with-http_secure_link_module --with-openssl-opt=enable-tlsext --add-module=../nginx-module-vts --with-libatomic --with-threads --with-stream=dynamic --with-stream_ssl_module --with-http_gzip_static_module --add-module=../ngx_brotli --add-dynamic-module=../ngx_pagespeed-release-1.10.33.5-beta --with-http_sub_module --with-http_addition_module --with-http_image_filter_module=dynamic --with-http_geoip_module=dynamic --with-http_realip_module --add-module=../nginx-accesskey-2.0.3 --add-module=../nginx-http-concat-master --add-module=../ngx-fancyindex-0.3.6 --add-module=../ngx_cache_purge-2.3 --add-module=../ngx_devel_kit-0.2.19 --add-module=../set-misc-nginx-module-0.29 --add-module=../echo-nginx-module-0.58 --add-module=../redis2-nginx-module-0.12 --add-module=../ngx_http_redis-0.3.7 --add-module=../lua-nginx-module-0.10.1rc1 --add-module=../lua-upstream-nginx-module-0.04 --add-module=../lua-upstream-cache-nginx-module-0.1.1 --add-module=../nginx_upstream_check_module-0.3.0 --add-module=../openresty-memc-nginx-module-4f6f78f --add-module=../openresty-srcache-nginx-module-ffa9ab7 --add-module=../headers-more-nginx-module-0.29 --with-pcre=../pcre-8.38 --with-pcre-jit --with-http_ssl_module --with-http_v2_module --with-openssl=../openssl-1.0.2f

Not sure if related, but i was having this issue when i had: gzip_vary on; in my http block, instead of the server block.

I also have gzip_vary on only in http block and not each vhost block

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Feb 19, 2016

For fixing this we would need to land #1105 (pending review) plus a commit on the mps side but I am not sure which one..

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Feb 19, 2016

-ps- the patch link I posted in a gist above (https://gist.github.com/oschaaf/c16c7458a57db4e94323) is similar to #1105 but adds a temporary hack to make sure ngx_pagespeed only emits a single vary header when psol hands it more then one.

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Feb 19, 2016

I don't know if it's by purpose, but I receive Pingdom errors about Vary headers (I applied the patch), because sometimes there are 2 different Vary headers at the same time: "Vary: Accept-Encoding" and "Vary: User-Agent".

$ curl -I https://www.jennifersauer.nl/wp-content/plugins/cyclone-slider-2/templates/thumbnails/style.css
HTTP/1.1 200 OK
Content-Type: text/css
Connection: keep-alive
Keep-Alive: timeout=300
Server: nginx
Access-Control-Allow-Origin: *
Accept-Ranges: bytes
X-Original-Content-Length: 4269
Vary: Accept-Encoding
Content-Length: 4269
ETag: W/"PSA-aj-RzSpPY1grB"
Date: Fri, 19 Feb 2016 18:36:15 GMT
Expires: Fri, 17 Feb 2017 02:01:32 GMT
Cache-Control: max-age=31389916
Vary: User-Agent

Turning Pagespeed off fixes it:

$ curl -I https://www.jennifersauer.nl/wp-content/plugins/cyclone-slider-2/templates/thumbnails/style.css?ModPagespeed=Off
HTTP/1.1 200 OK
Server: nginx
Date: Fri, 19 Feb 2016 18:37:06 GMT
Content-Type: text/css
Content-Length: 5685
Last-Modified: Tue, 05 Jan 2016 19:30:40 GMT
Connection: keep-alive
Keep-Alive: timeout=300
Vary: Accept-Encoding
ETag: "568c19e0-1635"
Expires: Thu, 31 Dec 2037 23:55:55 GMT
Cache-Control: max-age=315360000
Access-Control-Allow-Origin: *
Cache-Control: public, must-revalidate, proxy-revalidate
Accept-Ranges: bytes

The correct one would be: "Vary: Accept-Encoding, User-Agent".

@oschaaf

This comment has been minimized.

Copy link
Member Author

oschaaf commented Feb 19, 2016

@HansVanEijsden As far as I know at the http level

Vary: Accept-Encoding, User-Agent

means the same as

Vary: Accept-Encoding
Vary: User-Agent

From https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)].
It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma.

@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Feb 19, 2016

@oschaaf thanks! I will pass that on to Pingdom.

@deweydb

This comment has been minimized.

Copy link

deweydb commented Mar 4, 2016

Sorry for the super noob question but how do i apply this patch without it failing?

    root@localhost:/root/nginx/nginx_modules/ngx_pagespeed# git apply -v multiple-vary-hans-hotfix.patch
    multiple-vary-hans-hotfix.patch:20: trailing whitespace.
          // TODO(oschaaf): temporary hack to prevent cascading that PSOL emits 
    Checking patch src/ngx_pagespeed.cc...
    error: while searching for:
      for (i = 0 ; i < pagespeed_headers.NumAttributes() ; i++) {
        const GoogleString& name_gs = pagespeed_headers.Name(i);
        const GoogleString& value_gs = pagespeed_headers.Value(i);

        if (preserve_caching_headers == kPreserveAllCachingHeaders) {
          if (StringCaseEqual(name_gs, "ETag") ||
              StringCaseEqual(name_gs, "Expires") ||

    error: patch failed: src/ngx_pagespeed.cc:492
    error: src/ngx_pagespeed.cc: patch does not apply
    Checking patch src/ngx_pagespeed.h...
    Checking patch test/nginx_system_test.sh...
    error: while searching for:
    MATCHES=$(echo "$OUT" | grep -c "Cache-Control: override") || true
    check [ $MATCHES -eq 1 ]

    start_test Shutting down.

    # Fire up some heavy load if ab is available to test a stressed shutdown

    error: patch failed: test/nginx_system_test.sh:1241
    error: test/nginx_system_test.sh: patch does not apply
    Checking patch test/pagespeed_test.conf.template...
    Hunk #1 succeeded at 1471 (offset 8 lines).
@HansVanEijsden

This comment has been minimized.

Copy link

HansVanEijsden commented Mar 4, 2016

@deweydb no worries, there are no stupid questions, only stupid answers.

I do it like this:
First I go to the source folder (cd ngx_pagespeed-release-1.10.33.6-beta) and then I download the patch there (wget https://gist.githubusercontent.com/oschaaf/c16c7458a57db4e94323/raw/a659f3809b22a89b49cd647d5466ab8f8fa66dad/multiple-vary-hans-hotfix.patch).
Then I apply the patch (patch -p1 < multiple-vary-hans-hotfix.patch).

Good luck!

@jeffkaufman jeffkaufman changed the title Multiple Vary headers emitted? Multiple Vary headers emitted Mar 8, 2016

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Mar 8, 2016

I just reproduced this on www.jefftk.com:

    server {
      listen 80;
      server_name ipro-gzip-proxy;

      pagespeed on;

      location / {
        proxy_pass http://www.lilywise.com;
      }
    }

And then:

http_proxy=www.jefftk.com:80 curl -D- ipro-gzip-proxy/foo.js
HTTP/1.1 200 OK
Server: nginx/1.8.0
Date: Tue, 08 Mar 2016 14:56:29 GMT
Content-Type: application/javascript
Content-Length: 400
Connection: keep-alive
Vary: Accept-Encoding
Last-Modified: Tue, 08 Mar 2016 14:55:31 GMT
Vary: Accept-Encoding
ETag: "56dee7e3-190"
Accept-Ranges: bytes

document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
jefftk@jefftk02 ~/clients/vary-accept $ http_proxy=www.jefftk.com:80 curl -D- ipro-gzip-proxy/foo.js
HTTP/1.1 200 OK
Server: nginx/1.8.0
Content-Type: application/javascript
Connection: keep-alive
Vary: Accept-Encoding
Last-Modified: Tue, 08 Mar 2016 14:55:31 GMT
Vary: Accept-Encoding
ETag: "56dee7e3-190"
Accept-Ranges: bytes
Date: Tue, 08 Mar 2016 14:56:29 GMT
Expires: Tue, 08 Mar 2016 15:01:29 GMT
Cache-Control: max-age=300
X-Original-Content-Length: 400
Vary: Accept-Encoding
Vary: Accept-Encoding
Content-Length: 400

document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
document.write   (   "Hello World"   );
jefftk@jefftk02 ~/clients/vary-accept $ http_proxy=www.jefftk.com:80 curl -D- ipro-gzip-proxy/foo.js
HTTP/1.1 200 OK
Server: nginx/1.8.0
Content-Type: application/javascript
Connection: keep-alive
Vary: Accept-Encoding
Accept-Ranges: bytes
X-Original-Content-Length: 300
Vary: Accept-Encoding
Vary: Accept-Encoding
Content-Length: 300
ETag: W/"PSA-aj-Sb56YPzIH8"
Date: Tue, 08 Mar 2016 14:56:32 GMT
Expires: Tue, 08 Mar 2016 15:01:29 GMT
Cache-Control: max-age=296

document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");document.write("Hello World");

(This is after I merged #1105 but I didn't rebuild jefftk.com so that bug is still there.)

We see:

1st response: one vary header.
2nd response: four vary headers.
3rd and later requests: three vary headers.

The 1st response is correct, and for the later responses not having #1105 merged means one extra one is expected. So there's two unexplained vary headers on the 2nd response, and one unexplained on later responses.

Trying to fix this now.

(Thanks for @morlovich for having a good sense of where to look!)

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Mar 8, 2016

Even though I have this replicated on jefftk.com, I'm still having trouble replicating it in a dev environment. Still working on it.

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Mar 8, 2016

Talking to @oschaaf , neither of us have been able to reproduce this with current PSOL, just on the 1.10 branch. It looks like this will be resolved the rest of the way when we next cut a branch (1.12 if it exists, otherwise 2.0) and it's not worth putting additional time into for now.

@jeffkaufman jeffkaufman closed this Mar 8, 2016

jeffkaufman added a commit that referenced this issue Mar 30, 2016

vary-header: Emit a single vary header in the IPRO flow
The report from some time ago mentioned three Vary: headers,
but I can now only reproduce two using trunk-tracking plus the
original repro-configuration.

This fix unflags r->gzip_vary as set by the gzip module when PSOL
hands us Vary: Accept-Encoding, to make sure that nginx's core
header filter doesn't append another one.

Fixes #1064

jeffkaufman added a commit that referenced this issue Mar 30, 2016

vary-header: Emit a single vary header in the IPRO flow
The report from some time ago mentioned three Vary: headers,
but I can now only reproduce two using trunk-tracking plus the
original repro-configuration.

This fix unflags r->gzip_vary as set by the gzip module when PSOL
hands us Vary: Accept-Encoding, to make sure that nginx's core
header filter doesn't append another one.

Fixes #1064
@centminmod

This comment has been minimized.

Copy link

centminmod commented Apr 1, 2016

I thought this was fixed in 1.11.33.0 ?

from proxy downstream front end

curl -I http://centminmod.com/js/jquery.min.js+bootstrap.min.js.pagespeed.jc.Cd39AMnoIp.js
HTTP/1.1 200 OK
Date: Fri, 01 Apr 2016 20:43:19 GMT
Content-Type: application/javascript; charset=utf-8
Content-Length: 124101
Connection: keep-alive
Vary: Accept-Encoding
Expires: Sat, 01 Apr 2017 20:42:09 GMT
ETag: W/"0"
Last-Modified: Fri, 01 Apr 2016 20:42:09 GMT
X-Original-Content-Length: 124101
Vary: Accept-Encoding
Vary: Accept-Encoding
Server: nginx centminmod
X-Powered-By: centminmod
Cache-Control: max-age=31536000
PS-CapabilityList: LargeScreen.SkipUADependentOptimizations
X-Pcache: BYPASS

from proxy downstream backend

HTTP/1.1 200 OK
Content-Type: application/javascript
Transfer-Encoding: chunked
Connection: keep-alive
Server: nginx centminmod
X-Powered-By: centminmod
Date: Fri, 01 Apr 2016 20:50:06 GMT
Expires: Sat, 01 Apr 2017 20:50:06 GMT
Cache-Control: max-age=31536000
ETag: W/"0"
Last-Modified: Fri, 01 Apr 2016 20:50:06 GMT
X-Original-Content-Length: 124101
Vary: Accept-Encoding
Vary: Accept-Encoding
X-Page-Speed: 1.11.33.0-0
Content-Encoding: gzip

nginx configuration

nginx version: nginx/1.9.13
built by gcc 4.8.5 20150623 (Red Hat 4.8.5-4) (GCC)
built with LibreSSL 2.3.3
TLS SNI support enabled
configure arguments: --with-ld-opt='-lrt -ljemalloc -Wl,-z,relro -Wl,-rpath,/usr/local/lib' --with-cc-opt='-m64 -mtune=native -mfpmath=sse -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2' --sbin-path=/usr/local/sbin/nginx --conf-path=/usr/local/nginx/conf/nginx.conf --with-http_stub_status_module --with-http_secure_link_module --with-openssl-opt=enable-tlsext --add-module=../nginx-module-vts --with-libatomic --with-threads --with-stream=dynamic --with-stream_ssl_module --with-http_gzip_static_module --add-dynamic-module=../ngx_brotli --add-dynamic-module=../ngx_pagespeed-release-1.11.33.0-beta --with-http_sub_module --with-http_addition_module --with-http_image_filter_module=dynamic --with-http_geoip_module --with-http_realip_module --add-module=../ngx-fancyindex-0.3.6 --add-module=../ngx_cache_purge-2.3 --add-module=../ngx_devel_kit-0.3.0rc1 --add-module=../set-misc-nginx-module-0.30 --add-module=../echo-nginx-module-0.58 --add-module=../redis2-nginx-module-0.12 --add-module=../ngx_http_redis-0.3.7 --add-module=../lua-nginx-module-0.10.2 --add-module=../nginx_upstream_check_module-0.3.0 --add-module=../openresty-memc-nginx-module-4f6f78f --add-module=../srcache-nginx-module-0.30 --add-module=../headers-more-nginx-module-0.29 --with-pcre=../pcre-8.38 --with-pcre-jit --with-http_ssl_module --with-http_v2_module --with-openssl=../libressl-2.3.3

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Apr 4, 2016

Sorry @centminmod , only half of this fix is in 1.11. Specifically, b081bb7 resolved this for most users, but for some users we have unreleased changes on master to clean up the rest of the duplication. That will be in 1.12 (if it's needed) or 2.0.

@centminmod

This comment has been minimized.

Copy link

centminmod commented Apr 5, 2016

@jeffkaufman thanks for the clarification.. looking forward to 1.12/2.0 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment