From 2b4c097d48b6935a7c7aea94da9f09be8d611d60 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Wed, 27 Jan 2016 23:07:25 +0100
Subject: [PATCH] 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 https://github.com/pagespeed/ngx_pagespeed/issues/1064
---
 src/ngx_pagespeed.cc              | 13 +++++++++++++
 src/ngx_pagespeed.h               |  1 +
 test/nginx_system_test.sh         | 16 ++++++++++++++++
 test/pagespeed_test.conf.template |  8 ++++++++
 4 files changed, 38 insertions(+)

diff --git a/src/ngx_pagespeed.cc b/src/ngx_pagespeed.cc
index b62e9b3aa..0e298b7ab 100644
--- a/src/ngx_pagespeed.cc
+++ b/src/ngx_pagespeed.cc
@@ -576,6 +576,10 @@ ngx_int_t copy_response_headers_to_ngx(
       continue;
     } else if (STR_EQ_LITERAL(name, "Transfer-Encoding")) {
       continue;
+    } else if (STR_EQ_LITERAL(name, "Vary") && value.len
+        && STR_EQ_LITERAL(value, "Accept-Encoding")) {
+      ps_request_ctx_t* ctx = ps_get_request_context(r);
+      ctx->psol_vary_accept_only = true;
     }
 
     ngx_table_elt_t* header = static_cast<ngx_table_elt_t*>(
@@ -1270,6 +1274,7 @@ ngx_int_t ps_decline_request(ngx_http_request_t* r) {
   ctx->driver->Cleanup();
   ctx->driver = NULL;
   ctx->location_field_set = false;
+  ctx->psol_vary_accept_only = false;
 
   // re init ctx
   ctx->html_rewrite = true;
@@ -1840,6 +1845,7 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
     ctx->recorder = NULL;
     ctx->url_string = url_string;
     ctx->location_field_set = false;
+    ctx->psol_vary_accept_only = false;
 
     // Set up a cleanup handler on the request.
     ngx_http_cleanup_t* cleanup = ngx_http_cleanup_add(r, 0);
@@ -2157,6 +2163,13 @@ ngx_int_t ps_etag_header_filter(ngx_http_request_t* r) {
       break;
     }
   }
+
+  ps_request_ctx_t* ctx = ps_get_request_context(r);
+#if (NGX_HTTP_GZIP)
+  if (ctx && ctx->psol_vary_accept_only) {
+    r->gzip_vary = 0;
+  }
+#endif
   return ngx_http_ef_next_header_filter(r);
 }
 
diff --git a/src/ngx_pagespeed.h b/src/ngx_pagespeed.h
index cae895a8a..a531bc3ed 100644
--- a/src/ngx_pagespeed.h
+++ b/src/ngx_pagespeed.h
@@ -108,6 +108,7 @@ typedef struct {
   // we should mirror that when we write it back. nginx may absolutify
   // Location: headers that start with '/' without regarding X-Forwarded-Proto.
   bool location_field_set;
+  bool psol_vary_accept_only;
 } ps_request_ctx_t;
 
 ps_request_ctx_t* ps_get_request_context(ngx_http_request_t* r);
diff --git a/test/nginx_system_test.sh b/test/nginx_system_test.sh
index 67238e640..0acff6f3c 100644
--- a/test/nginx_system_test.sh
+++ b/test/nginx_system_test.sh
@@ -1249,6 +1249,22 @@ OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1) || tr
 # We ignored the exit code, check if we got a 404 response.
 check_from "$OUT" fgrep -qi '404'
 
+start_test Single Vary: Accept-Encoding header in IPRO flow
+URL=http://psol-vary.example.com/mod_pagespeed_example/styles/index_style.css
+OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1)
+# First hit will be recorded and passed on untouched
+MATCHES=$(echo "$OUT" | grep -c "Vary: Accept-Encoding") || true
+check [ $MATCHES -eq 1 ]
+
+# Fetch until we get a fully optimized response
+http_proxy=$SECONDARY_HOSTNAME \
+  fetch_until $URL "fgrep -c W/\"PSA" 1 --save-headers
+
+# Test the optimized response.
+OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1)
+MATCHES=$(echo "$OUT" | grep -c "Vary: Accept-Encoding") || true
+check [ $MATCHES -eq 1 ]
+
 start_test Shutting down.
 
 # Fire up some heavy load if ab is available to test a stressed shutdown
diff --git a/test/pagespeed_test.conf.template b/test/pagespeed_test.conf.template
index 7f9045c0c..8c56fe349 100644
--- a/test/pagespeed_test.conf.template
+++ b/test/pagespeed_test.conf.template
@@ -1471,6 +1471,14 @@ http {
     pagespeed GlobalAdminDomains
      Allow everything-explicitly-allowed.example.com;
   }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name psol-vary.example.com;
+    pagespeed on;
+    pagespeed InPlaceResourceOptimization on;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
 
   server {
     listen @@PRIMARY_PORT@@;