-
-
Save oschaaf/c16c7458a57db4e94323 to your computer and use it in GitHub Desktop.
Multiple Vary headers hotfix Hans
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/src/ngx_pagespeed.cc b/src/ngx_pagespeed.cc | |
index 1f8f2e2..4fc26ed 100644 | |
--- a/src/ngx_pagespeed.cc | |
+++ b/src/ngx_pagespeed.cc | |
@@ -492,7 +492,6 @@ ngx_int_t copy_response_headers_to_ngx( | |
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") || | |
@@ -570,6 +569,16 @@ 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); | |
+ // TODO(oschaaf): temporary hack to prevent cascading that PSOL emits | |
+ // multiple Vary: Accept-Encoding headers. Remove this once the PSOL fix | |
+ // is released. | |
+ if ( ctx->psol_vary_accept_only ) { | |
+ continue; | |
+ } | |
+ ctx->psol_vary_accept_only = true; | |
} | |
u_char* name_s = ngx_pstrdup(r->pool, &name); | |
@@ -1268,6 +1277,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; | |
@@ -1838,6 +1848,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); | |
@@ -2155,6 +2166,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 cae895a..a531bc3 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 ff52a7c..5bd8984 100644 | |
--- a/test/nginx_system_test.sh | |
+++ b/test/nginx_system_test.sh | |
@@ -1241,6 +1241,22 @@ check_from "$OUT" fgrep -qi '404' | |
MATCHES=$(echo "$OUT" | grep -c "Cache-Control: override") || true | |
check [ $MATCHES -eq 1 ] | |
+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 d05f92a..35a7616 100644 | |
--- a/test/pagespeed_test.conf.template | |
+++ b/test/pagespeed_test.conf.template | |
@@ -1463,6 +1463,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@@; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment