Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Commit b081bb7

Browse files
committed
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
1 parent d023bb3 commit b081bb7

File tree

4 files changed

+38
-0
lines changed

4 files changed

+38
-0
lines changed

src/ngx_pagespeed.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,10 @@ ngx_int_t copy_response_headers_to_ngx(
572572
continue;
573573
} else if (STR_EQ_LITERAL(name, "Transfer-Encoding")) {
574574
continue;
575+
} else if (STR_EQ_LITERAL(name, "Vary") && value.len
576+
&& STR_EQ_LITERAL(value, "Accept-Encoding")) {
577+
ps_request_ctx_t* ctx = ps_get_request_context(r);
578+
ctx->psol_vary_accept_only = true;
575579
}
576580

577581
u_char* name_s = ngx_pstrdup(r->pool, &name);
@@ -1282,6 +1286,7 @@ ngx_int_t ps_decline_request(ngx_http_request_t* r) {
12821286
ctx->driver->Cleanup();
12831287
ctx->driver = NULL;
12841288
ctx->location_field_set = false;
1289+
ctx->psol_vary_accept_only = false;
12851290

12861291
// re init ctx
12871292
ctx->html_rewrite = true;
@@ -1852,6 +1857,7 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
18521857
ctx->recorder = NULL;
18531858
ctx->url_string = url_string;
18541859
ctx->location_field_set = false;
1860+
ctx->psol_vary_accept_only = false;
18551861

18561862
// Set up a cleanup handler on the request.
18571863
ngx_http_cleanup_t* cleanup = ngx_http_cleanup_add(r, 0);
@@ -2169,6 +2175,13 @@ ngx_int_t ps_etag_header_filter(ngx_http_request_t* r) {
21692175
break;
21702176
}
21712177
}
2178+
2179+
ps_request_ctx_t* ctx = ps_get_request_context(r);
2180+
#if (NGX_HTTP_GZIP)
2181+
if (ctx && ctx->psol_vary_accept_only) {
2182+
r->gzip_vary = 0;
2183+
}
2184+
#endif
21722185
return ngx_http_ef_next_header_filter(r);
21732186
}
21742187

src/ngx_pagespeed.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ typedef struct {
108108
// we should mirror that when we write it back. nginx may absolutify
109109
// Location: headers that start with '/' without regarding X-Forwarded-Proto.
110110
bool location_field_set;
111+
bool psol_vary_accept_only;
111112
} ps_request_ctx_t;
112113

113114
ps_request_ctx_t* ps_get_request_context(ngx_http_request_t* r);

test/nginx_system_test.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,22 @@ OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1) || tr
12481248
# We ignored the exit code, check if we got a 404 response.
12491249
check_from "$OUT" fgrep -qi '404'
12501250

1251+
start_test Single Vary: Accept-Encoding header in IPRO flow
1252+
URL=http://psol-vary.example.com/mod_pagespeed_example/styles/index_style.css
1253+
OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1)
1254+
# First hit will be recorded and passed on untouched
1255+
MATCHES=$(echo "$OUT" | grep -c "Vary: Accept-Encoding") || true
1256+
check [ $MATCHES -eq 1 ]
1257+
1258+
# Fetch until we get a fully optimized response
1259+
http_proxy=$SECONDARY_HOSTNAME \
1260+
fetch_until $URL "fgrep -c W/\"PSA" 1 --save-headers
1261+
1262+
# Test the optimized response.
1263+
OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1)
1264+
MATCHES=$(echo "$OUT" | grep -c "Vary: Accept-Encoding") || true
1265+
check [ $MATCHES -eq 1 ]
1266+
12511267
start_test Shutting down.
12521268

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

test/pagespeed_test.conf.template

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,14 @@ http {
14711471
pagespeed GlobalAdminDomains
14721472
Allow everything-explicitly-allowed.example.com;
14731473
}
1474+
server {
1475+
listen @@SECONDARY_PORT@@;
1476+
listen [::]:@@SECONDARY_PORT@@;
1477+
server_name psol-vary.example.com;
1478+
pagespeed on;
1479+
pagespeed InPlaceResourceOptimization on;
1480+
pagespeed FileCachePath "@@FILE_CACHE@@";
1481+
}
14741482

14751483
server {
14761484
listen @@PRIMARY_PORT@@;

0 commit comments

Comments
 (0)