Merge branch 'jmarantz-config-with-response-headers-test' of github.com:pagespeed/ngx_pagespeed into jmarantz-config-with-response-headers-test

Rename QueryParams::Parse to ParseFromUrl. Add a wrapper that takes an untrusted string.

README.md

@@ -49,9 +49,9 @@ recompiling Tengine](https://github.com/pagespeed/ngx_pagespeed/wiki/Using-ngx_p
    ```bash
    $ cd ~
    $ # check http://nginx.org/en/download.html for the latest version
-   $ wget http://nginx.org/download/nginx-1.4.6.tar.gz
-   $ tar -xvzf nginx-1.4.6.tar.gz
-   $ cd nginx-1.4.6/
+   $ wget http://nginx.org/download/nginx-1.6.0.tar.gz
+   $ tar -xvzf nginx-1.6.0.tar.gz
+   $ cd nginx-1.6.0/
    $ ./configure --add-module=$HOME/ngx_pagespeed-1.7.30.4-beta
    $ make
    $ sudo make install

config

@@ -173,7 +173,8 @@ if [ $ngx_found = yes ]; then
     $ps_src/ngx_rewrite_driver_factory.h \
     $ps_src/ngx_rewrite_options.h \
     $ps_src/ngx_server_context.h \
-    $ps_src/ngx_url_async_fetcher.h"
+    $ps_src/ngx_url_async_fetcher.h \
+    $psol_library_binaries"
   NGX_ADDON_SRCS="$NGX_ADDON_SRCS \
     $ps_src/log_message_handler.cc \
     $ps_src/ngx_base_fetch.cc \

src/ngx_pagespeed.cc

@@ -1277,17 +1277,16 @@ ps_loc_conf_t* ps_get_loc_config(ngx_http_request_t* r) {
 // Wrapper around GetQueryOptions()
 RewriteOptions* ps_determine_request_options(
     ngx_http_request_t* r,
+    const RewriteOptions* domain_options, /* may be null */
     RequestHeaders* request_headers,
     ResponseHeaders* response_headers,
     ps_srv_conf_t* cfg_s,
     GoogleUrl* url) {
-  // Stripping ModPagespeed query params before the property cache lookup to
-  // make cache key consistent for both lookup and storing in cache.
-  //
   // Sets option from request headers and url.
   RewriteQuery rewrite_query;
   if (!cfg_s->server_context->GetQueryOptions(
-          url, request_headers, response_headers, &rewrite_query)) {
+          domain_options, url, request_headers, response_headers,
+          &rewrite_query)) {
     // Failed to parse query params or request headers.  Treat this as if there
     // were no query params given.
     ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
@@ -1375,7 +1374,7 @@ bool ps_determine_options(ngx_http_request_t* r,
   // Request-specific options, nearly always null.  If set they need to be
   // rebased on the directory options or the global options.
   RewriteOptions* request_options = ps_determine_request_options(
-      r, request_headers, response_headers, cfg_s, url);
+      r, directory_options, request_headers, response_headers, cfg_s, url);
   bool have_request_options = request_options != NULL;
 
   // Because the caller takes ownership of any options we return, the only
@@ -1798,7 +1797,7 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
     return ps_async_wait_response(r);
   } else if (is_an_admin_handler) {
     QueryParams query_params;
-    query_params.Parse(url.Query());
+    query_params.ParseFromUrl(url);
 
     PosixTimer timer;
     int64 now_ms = timer.NowMs();
@@ -2395,7 +2394,7 @@ ngx_int_t ps_simple_handler(ngx_http_request_t* r,
   GoogleUrl url(url_string);
   QueryParams query_params;
   if (url.IsWebValid()) {
-    query_params.Parse(url.Query());
+    query_params.ParseFromUrl(url);
   }
 
   GoogleString output;

test/nginx_system_test.sh

@@ -1324,10 +1324,10 @@ check touch "$SECONDARY_CACHE/cache.flush"
 check touch "$IPRO_CACHE/cache.flush"
 sleep 1
 
-CSS_FILE="$SERVER_ROOT/mod_pagespeed_test/update.css"
+CSS_FILE="$SERVER_ROOT/mod_pagespeed_test/cache_flush/update.css"
 echo ".class myclass { color: $COLOR0; }" > "$CSS_FILE"
 
-URL_PATH="mod_pagespeed_test/cache_flush_test.html"
+URL_PATH="mod_pagespeed_test/cache_flush/cache_flush_test.html"
 
 URL="$SECONDARY_HOSTNAME/$URL_PATH"
 CACHE_A="--header=Host:cache_a.example.com"
@@ -1997,13 +1997,13 @@ OUT1=$(http_proxy=$SECONDARY_HOSTNAME \
           $WGET_DUMP --header 'X-PSA-Blocking-Rewrite: psatest' $URL)
 check_not_from "$OUT1" egrep -q 'pagespeed\.criticalCssBeaconInit'
 check_from "$OUT1" grep -q "Cache-Control: private, max-age=3000"
+
 # 2. We get an instrumented page if the correct key is present.
-OUT2=$(http_proxy=$SECONDARY_HOSTNAME \
-          $WGET_DUMP $WGET_ARGS \
-          --header 'X-PSA-Blocking-Rewrite: psatest'\
-          --header="PS-ShouldBeacon: random_rebeaconing_key" $URL)
-check_from "$OUT2" grep -q "Cache-Control: max-age=0, no-cache"
-check_from "$OUT2" egrep -q "pagespeed\.criticalCssBeaconInit"
+http_proxy=$SECONDARY_HOSTNAME \
+  fetch_until -save $URL 'grep -c criticalCssBeaconInit' 2 \
+  "--header=PS-ShouldBeacon:random_rebeaconing_key --save-headers"
+check grep -q "Cache-Control: max-age=0, no-cache" $FETCH_UNTIL_OUTFILE
+
 # 3. We do not get an instrumented page if the wrong key is present.
 WGET_ARGS="--header=\"PS-ShouldBeacon: wrong_rebeaconing_key\""
 OUT3=$(http_proxy=$SECONDARY_HOSTNAME \
@@ -2342,6 +2342,77 @@ test_decent_browsers "New Opera" \
 WGETRC=$OLD_WGETRC
 WGET_ARGS=""
 
+test_filter collapse_whitespace
+start_test Cookie options on: by default comments not removed, whitespace is
+HOST_NAME="http://options-by-cookies-enabled.example.com"
+URL="$HOST_NAME/mod_pagespeed_test/forbidden.html"
+echo wget $URL
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $URL)"
+check_from     "$OUT" grep -q '<!--'
+check_not_from "$OUT" grep -q '  '
+start_test Cookie options on: set option by cookie takes effect
+echo wget --header=Cookie:PageSpeedFilters=+remove_comments $URL
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP --no-cookies \
+       --header=Cookie:PageSpeedFilters=+remove_comments $URL)"
+check_not_from "$OUT" grep -q '<!--'
+check_not_from "$OUT" grep -q '  '
+start_test Cookie options off: by default comments nor whitespace removed
+HOST_NAME="http://options-by-cookies-disabled.example.com"
+URL="$HOST_NAME/mod_pagespeed_test/forbidden.html"
+echo wget $URL
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $URL)"
+check_from "$OUT" grep -q '<!--'
+check_from "$OUT" grep -q '  '
+start_test Cookie options off: set option by cookie has no effect
+echo wget --header=Cookie:PageSpeedFilters=+remove_comments $URL
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP --no-cookies \
+       --header=Cookie:PageSpeedFilters=+remove_comments $URL)"
+check_from "$OUT" grep -q '<!--'
+check_from "$OUT" grep -q '  '
+WGET_ARGS=""
+
+start_test Request Option Override : Correct values are passed
+HOST_NAME="http://request-option-override.example.com"
+OPTS="?ModPagespeed=on"
+OPTS+="&ModPagespeedFilters=+collapse_whitespace,+remove_comments"
+OPTS+="&PageSpeedRequestOptionOverride=abc"
+URL="$HOST_NAME/mod_pagespeed_test/forbidden.html$OPTS"
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $URL)"
+echo wget $URL
+check_not_from "$OUT" grep -q '<!--'
+
+start_test Request Option Override : Incorrect values are passed
+HOST_NAME="http://request-option-override.example.com"
+OPTS="?ModPagespeed=on"
+OPTS+="&ModPagespeedFilters=+collapse_whitespace,+remove_comments"
+OPTS+="&PageSpeedRequestOptionOverride=notabc"
+URL="$HOST_NAME/mod_pagespeed_test/forbidden.html$OPTS"
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $URL)"
+echo wget $URL
+check_from "$OUT" grep -q '<!--'
+
+start_test Request Option Override : Correct values are passed as headers
+HOST_NAME="http://request-option-override.example.com"
+OPTS="--header=ModPagespeed:on"
+OPTS+=" --header=ModPagespeedFilters:+collapse_whitespace,+remove_comments"
+OPTS+=" --header=PageSpeedRequestOptionOverride:abc"
+URL="$HOST_NAME/mod_pagespeed_test/forbidden.html"
+echo wget $OPTS $URL
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $OPTS $URL)"
+check_not_from "$OUT" grep -q '<!--'
+
+start_test Request Option Override : Incorrect values are passed as headers
+HOST_NAME="http://request-option-override.example.com"
+OPTS="--header=ModPagespeed:on"
+OPTS+=" --header=ModPagespeedFilters:+collapse_whitespace,+remove_comments"
+OPTS+=" --header=PageSpeedRequestOptionOverride:notabc"
+URL="$HOST_NAME/mod_pagespeed_test/forbidden.html"
+echo wget $OPTS $URL
+OUT="$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $OPTS $URL)"
+check_from "$OUT" grep -q '<!--'
+
+start_test JS gzip headers
+
 JS_URL="$HOSTNAME/pagespeed_static/js_defer.$HASH.js"
 JS_HEADERS=$($WGET -O /dev/null -q -S --header='Accept-Encoding: gzip' \
   $JS_URL 2>&1)
@@ -2551,6 +2622,16 @@ check_from "$OUT" egrep -iq $'^Cache-Control: .*\r$'
 check_from "$OUT" egrep -iq $'^Expires: .*\r$'
 check_from "$OUT" egrep -iq $'^Last-Modified: .*\r$'
 
+# Verify that we can control pagespeed settings via a response
+# header passed from an origin to a reverse proxy.
+start_test Honor response header direcives from origin
+URL="http://rproxy.rmcomments.example.com/"
+URL+="mod_pagespeed_example/remove_comments.html"
+echo http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $URL ...
+OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $URL)
+check_from "$OUT" fgrep -q "remove_comments example"
+check_not_from "$OUT" fgrep -q "This comment will be removed"
+
 # Test handling of large HTML files. We first test with a cold cache, and verify
 # that we bail out of parsing and insert a script redirecting to
 # ?PageSpeed=off. This should also insert an entry into the property cache so

test/pagespeed_test.conf.template

@@ -351,6 +351,29 @@ http {
     pagespeed DownstreamCachePurgeLocationPrefix "http://localhost:@@SECONDARY_PORT@@/purge";
   }
 
+  server {
+    listen @@SECONDARY_PORT@@;
+    server_name origin.rmcomments.example.com;
+    pagespeed off;
+    add_header PageSpeedFilters remove_comments;
+  }
+  server {
+    # Set up a reverse proxy (rproxy.) and origin (origin.) as vhosts for
+    # showing that we can configure PageSpeed via response headers.
+    listen @@SECONDARY_PORT@@;
+    server_name rproxy.rmcomments.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    pagespeed RewriteLevel PassThrough;
+    pagespeed DisableFilters add_instrumentation,remove_comments;
+    # Note that we don't enable remove_comments here; that setting
+    # comes from the response headers from origin.rmcomments.example.com
+    location / {
+      proxy_pass http://localhost:@@SECONDARY_PORT@@;
+      proxy_set_header X-Forwarded-For $remote_addr;
+      proxy_set_header Host origin.rmcomments.example.com;
+    }
+  }
+
   server {
     listen @@SECONDARY_PORT@@;
     server_name renderedimagebeacon.example.com;
@@ -624,6 +647,18 @@ http {
     pagespeed CacheFlushPollIntervalSec 1;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters inline_css;
+
+    # Make a non-empty subdirectory config to make sure that
+    # cache.flush updates get transmitted to nested configurations.
+    #
+    # TODO(jmarantz): This test currently fails on ngx_pagespeed, meaning we
+    # don't handle cache flushing correctly here.  Uncomment the config below to
+    # expose the error.
+    #location /mod_pagespeed_test/cache_flush/ {
+    #  pagespeed RewriteLevel PassThrough;
+    #  pagespeed EnableFilters inline_css;
+    #  pagespeed DisableFilters add_instrumentation;
+    #}
   }
 
   server {
@@ -862,6 +897,34 @@ http {
     add_header "Date" "Date: Fri, 16 Oct 2009 23:05:07 GMT";
   }
 
+  server {
+    listen @@SECONDARY_PORT@@;
+    server_name options-by-cookies-enabled.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@_optionsbycookieson";
+    pagespeed AllowOptionsToBeSetByCookies true;
+    pagespeed RewriteLevel PassThrough;
+    pagespeed EnableFilters collapse_whitespace;
+    pagespeed DisableFilters remove_comments,add_instrumentation;
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    server_name options-by-cookies-disabled.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@_optionsbycookiesoff";
+    pagespeed AllowOptionsToBeSetByCookies false;
+    pagespeed DisableFilters add_instrumentation;
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    server_name request-option-override.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RequestOptionOverride abc;
+    pagespeed RewriteLevel Passthrough;
+    pagespeed EnableFilters collapse_whitespace;
+    pagespeed DisableFilters remove_comments,add_instrumentation;
+  }
+
   # Proxy + IPRO a gzip'd file for testing Issue 896.
   server {
     listen @@SECONDARY_PORT@@;