Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
vary-header: Emit a single vary header in the IPRO flow
Browse files Browse the repository at this point in the history
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
  • Loading branch information
oschaaf committed Jan 28, 2016
1 parent d023bb3 commit b081bb7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/ngx_pagespeed.cc
Expand Up @@ -572,6 +572,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;
}

u_char* name_s = ngx_pstrdup(r->pool, &name);
Expand Down Expand Up @@ -1282,6 +1286,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;
Expand Down Expand Up @@ -1852,6 +1857,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);
Expand Down Expand Up @@ -2169,6 +2175,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);
}

Expand Down
1 change: 1 addition & 0 deletions src/ngx_pagespeed.h
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions test/nginx_system_test.sh
Expand Up @@ -1248,6 +1248,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
Expand Down
8 changes: 8 additions & 0 deletions test/pagespeed_test.conf.template
Expand Up @@ -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@@;
Expand Down

0 comments on commit b081bb7

Please sign in to comment.