trunk-tracking: update from r3924 to r3934

There aren't actually any changes required from r3924 to r3934 but the admin
work from previous versions wasn't completely ported before.

This change:
 * adds support for /pagespeed_admin and /pagespeed_global_admin
 * removes NgxPagespeedConsoleAsyncFetch using NgxBaseFetch instead
 * adds options StatisticsPath, GlobalStatisticsPath, ConsolePath,
   MessagesPath, AdminPath, and GlobalAdminPath which allow site owners to
   control what path these are served under
 * changes /ngx_pagespeed_statistics etc from default-on to default-off,
   requiring explicit path configuration to function
 * does not fix the problem where trunk-tracking fails tests on 1.5 post #653

config

@@ -109,6 +109,10 @@ case "$NGX_GCC_VER" in
   ;;
 esac
 
+if [ "$WNO_ERROR" = "YES" ]; then
+    CFLAGS="$CFLAGS -Wno-error"
+fi
+
 pagespeed_include="\
   $mod_pagespeed_dir \
   $mod_pagespeed_dir/third_party/chromium/src \

src/ngx_fetch.cc

@@ -72,11 +72,11 @@ namespace net_instaweb {
         done_(false),
         content_length_(-1),
         content_length_known_(false) {
-            ngx_memzero(&url_, sizeof(url_));
-            log_ = log;
-            pool_ = NULL;
-            timeout_event_ = NULL;
-            connection_ = NULL;
+    ngx_memzero(&url_, sizeof(url_));
+    log_ = log;
+    pool_ = NULL;
+    timeout_event_ = NULL;
+    connection_ = NULL;
   }
 
   NgxFetch::~NgxFetch() {

src/ngx_pagespeed.cc

@@ -290,7 +290,7 @@ ngx_int_t copy_response_headers_to_ngx(
           StringCaseEqual(name_gs, "Last-Modified") ||
           StringCaseEqual(name_gs, "Cache-Control")) {
         continue;
-      }      
+      }
     } else if (preserve_caching_headers == kPreserveOnlyCacheControl) {
       // Retain the original Cache-Control header, but send the recomputed
       // values for all other cache-related headers.
@@ -299,7 +299,7 @@ ngx_int_t copy_response_headers_to_ngx(
           StringCaseEqual(name_gs, "Date") ||
           StringCaseEqual(name_gs, "Last-Modified")) {
         continue;
-      }      
+      }
     } // else we don't preserve any headers
 
     ngx_str_t name, value;
@@ -441,8 +441,11 @@ enum Response {
   kPagespeedDisabled,
   kBeacon,
   kStatistics,
+  kGlobalStatistics,
   kConsole,
   kMessages,
+  kAdmin,
+  kGlobalAdmin,
   kPagespeedSubrequest,
   kNotHeadOrGet,
   kErrorResponse,
@@ -1567,16 +1570,26 @@ RequestRouting::Response ps_route_request(ngx_http_request_t* r,
   } else if (url.PathSansLeaf() ==
              NgxRewriteDriverFactory::kStaticAssetPrefix) {
     return RequestRouting::kStaticContent;
-  } else if (url.PathSansQuery() == "/ngx_pagespeed_statistics" ||
-             url.PathSansQuery() == "/ngx_pagespeed_global_statistics" ) {
-    return RequestRouting::kStatistics;
-  } else if (url.PathSansQuery() == "/pagespeed_console") {
-    return RequestRouting::kConsole;
-  } else if (url.PathSansQuery() == "/ngx_pagespeed_message") {
-    return RequestRouting::kMessages;
   }
 
-  RewriteOptions* global_options = cfg_s->server_context->global_options();
+  NgxRewriteOptions* global_options = cfg_s->server_context->config();
+
+  StringPiece path = url.PathSansQuery();
+  if (path == global_options->statistics_path()) {
+    return RequestRouting::kStatistics;
+  } else if (path == global_options->global_statistics_path()) {
+    return RequestRouting::kGlobalStatistics;
+  } else if (path == global_options->console_path()) {
+    return RequestRouting::kConsole;
+  } else if (path == global_options->messages_path()) {
+    return RequestRouting::kMessages;
+  } else if (!global_options->admin_path().empty() &&
+             path.starts_with(global_options->admin_path())) {
+    return RequestRouting::kAdmin;
+  } else if (!global_options->global_admin_path().empty() &&
+             path.starts_with(global_options->global_admin_path())) {
+    return RequestRouting::kGlobalAdmin;
+  }
 
   const GoogleString* beacon_url;
   if (ps_is_https(r)) {
@@ -1592,7 +1605,9 @@ RequestRouting::Response ps_route_request(ngx_http_request_t* r,
   return RequestRouting::kResource;
 }
 
-ngx_int_t ps_resource_handler(ngx_http_request_t* r, bool html_rewrite) {
+ngx_int_t ps_resource_handler(ngx_http_request_t* r,
+                              bool html_rewrite,
+                              RequestRouting::Response response_category) {
   if (r != r->main) {
     return NGX_DECLINED;
   }
@@ -1653,6 +1668,12 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r, bool html_rewrite) {
 
   bool pagespeed_resource =
       !html_rewrite && cfg_s->server_context->IsPagespeedResource(url);
+  bool is_an_admin_handler =
+      response_category == RequestRouting::kStatistics ||
+      response_category == RequestRouting::kGlobalStatistics ||
+      response_category == RequestRouting::kConsole ||
+      response_category == RequestRouting::kAdmin ||
+      response_category == RequestRouting::kGlobalAdmin;
 
   if (html_rewrite) {
     ps_release_base_fetch(ctx);
@@ -1678,7 +1699,7 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r, bool html_rewrite) {
       // Downstream cache integration is not enabled. Disable original
       // Cache-Control headers.
       ctx->preserve_caching_headers = kDontPreserveHeaders;
-    } else if (!pagespeed_resource) {
+    } else if (!pagespeed_resource && !is_an_admin_handler) {
       ctx->preserve_caching_headers = kPreserveOnlyCacheControl;
       // Downstream cache integration is enabled. If a rebeaconing key has been
       // configured and there is a ShouldBeacon header with the correct key,
@@ -1732,6 +1753,42 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r, bool html_rewrite) {
         custom_options.release() /* null if there aren't custom options */,
         false /* using_spdy */, cfg_s->server_context, ctx->base_fetch);
     return ps_async_wait_response(r);
+  } else if (is_an_admin_handler) {
+    QueryParams query_params;
+    query_params.Parse(url.Query());
+
+    PosixTimer timer;
+    int64 now_ms = timer.NowMs();
+    ctx->base_fetch->response_headers()->SetDateAndCaching(
+        now_ms, 0 /* max-age */, ", no-cache");
+
+    if (response_category == RequestRouting::kStatistics ||
+        response_category == RequestRouting::kGlobalStatistics) {
+      cfg_s->server_context->StatisticsPage(
+          response_category == RequestRouting::kGlobalStatistics,
+          query_params,
+          cfg_s->server_context->config(),
+          ctx->base_fetch);
+    } else if (response_category == RequestRouting::kConsole) {
+      cfg_s->server_context->ConsoleHandler(
+          *cfg_s->server_context->config(),
+          SystemServerContext::kStatistics,
+          query_params,
+          ctx->base_fetch);
+    } else if (response_category == RequestRouting::kAdmin ||
+               response_category == RequestRouting::kGlobalAdmin) {
+      cfg_s->server_context->AdminPage(
+          response_category == RequestRouting::kGlobalAdmin,
+          url,
+          query_params,
+          custom_options == NULL ? cfg_s->server_context->config()
+                                 : custom_options.get(),
+          ctx->base_fetch);
+    } else {
+      CHECK(false);
+    }
+
+    return ps_async_wait_response(r);
   }
 
   if (html_rewrite) {
@@ -2038,7 +2095,8 @@ ngx_int_t ps_html_rewrite_header_filter(ngx_http_request_t* r) {
     return ngx_http_next_header_filter(r);
   }
 
-  ngx_int_t rc = ps_resource_handler(r, true /* html rewrite */);
+  ngx_int_t rc = ps_resource_handler(r, true /* html rewrite */,
+                                     RequestRouting::kResource);
   if (rc != NGX_OK) {
     ctx->html_rewrite = false;
     return ngx_http_next_header_filter(r);
@@ -2320,27 +2378,6 @@ ngx_int_t send_out_headers_and_body(
   return ngx_http_output_filter(r, out);
 }
 
-namespace {
-
-// TODO(jefftk): This class is a temporary shim to just support console fetch,
-// but we eventually want to convert everything in ps_simple_handler to use
-// something akin to NgxBaseFetch (which sends results direct to nginx).  This
-// is work in progress (but well along) in the mod_pagespeed world.
-class NgxPagespeedConsoleAsyncFetch : public AsyncFetchUsingWriter {
- public:
-  NgxPagespeedConsoleAsyncFetch(const RequestContextPtr& request_context,
-                                Writer* writer)
-      : AsyncFetchUsingWriter(request_context, writer) { }
-  virtual void HandleDone(bool status) { }
-  virtual void HandleHeadersComplete() { }
-
-  void FlushToNgx(const GoogleString& output, ngx_http_request_t* r) {
-    send_out_headers_and_body(r, *response_headers(), output);
-  }
-};
-
-}  // namespace
-
 ngx_int_t ps_simple_handler(ngx_http_request_t* r,
                             NgxServerContext* server_context,
                             RequestRouting::Response response_category) {
@@ -2376,34 +2413,6 @@ ngx_int_t ps_simple_handler(ngx_http_request_t* r,
       file_contents.CopyToString(&output);
       break;
     }
-    case RequestRouting::kStatistics: {
-      bool is_global_request =
-          StringCaseStartsWith(
-              request_uri_path, "/ngx_pagespeed_global_statistics");
-      ps_srv_conf_t* cfg_s = ps_get_srv_config(r);
-      RequestContextPtr request_context(
-          cfg_s->server_context->NewRequestContext(r));
-      NgxPagespeedConsoleAsyncFetch fetch(request_context, &writer);
-      server_context->StatisticsPage(
-          is_global_request,
-          query_params,
-          cfg_s->server_context->global_options(),
-          &fetch);
-      fetch.FlushToNgx(output, r);
-      return NGX_OK;
-    }
-    case RequestRouting::kConsole: {
-      ps_srv_conf_t* cfg_s = ps_get_srv_config(r);
-      RequestContextPtr request_context(
-          cfg_s->server_context->NewRequestContext(r));
-      NgxPagespeedConsoleAsyncFetch fetch(request_context, &writer);
-      server_context->ConsoleHandler(*server_context->config(),
-                                     SystemServerContext::kStatistics,
-                                     query_params,
-                                     &fetch);
-      fetch.FlushToNgx(output, r);
-      return NGX_OK;
-    }
     case RequestRouting::kMessages: {
       GoogleString log;
       StringWriter log_writer(&log);
@@ -2643,12 +2652,16 @@ ngx_int_t ps_content_handler(ngx_http_request_t* r) {
     case RequestRouting::kBeacon:
       return ps_beacon_handler(r);
     case RequestRouting::kStaticContent:
-    case RequestRouting::kStatistics:
-    case RequestRouting::kConsole:
     case RequestRouting::kMessages:
       return ps_simple_handler(r, cfg_s->server_context, response_category);
+    case RequestRouting::kStatistics:
+    case RequestRouting::kGlobalStatistics:
+    case RequestRouting::kConsole:
+    case RequestRouting::kAdmin:
+    case RequestRouting::kGlobalAdmin:
     case RequestRouting::kResource:
-      return ps_resource_handler(r, false /* html rewrite */);
+      return ps_resource_handler(
+          r, false /* html rewrite */, response_category);
   }
 
   CHECK(0);

src/ngx_rewrite_options.cc

@@ -37,6 +37,13 @@ namespace net_instaweb {
 
 namespace {
 
+const char kStatisticsPath[] = "StatisticsPath";
+const char kGlobalStatisticsPath[] = "GlobalStatisticsPath";
+const char kConsolePath[] = "ConsolePath";
+const char kMessagesPath[] = "MessagesPath";
+const char kAdminPath[] = "AdminPath";
+const char kGlobalAdminPath[] = "GlobalAdminPath";
+
 // These options are copied from mod_instaweb.cc, where APACHE_CONFIG_OPTIONX
 // indicates that they can not be set at the directory/location level. They set
 // options in the RewriteDriverFactory, so they do not appear in RewriteOptions.
@@ -92,7 +99,26 @@ void NgxRewriteOptions::Init() {
 }
 
 void NgxRewriteOptions::AddProperties() {
-  // Nothing ngx-specific for now.
+  // Nginx-specific options.
+  add_ngx_option(
+      "", &NgxRewriteOptions::statistics_path_, "nsp", kStatisticsPath,
+      kProcessScope, "Set the statistics path. Ex: /ngx_pagespeed_statistics");
+  add_ngx_option(
+      "", &NgxRewriteOptions::global_statistics_path_, "ngsp",
+      kGlobalStatisticsPath, kProcessScope,
+      "Set the global statistics path. Ex: /ngx_pagespeed_global_statistics");
+  add_ngx_option(
+      "", &NgxRewriteOptions::console_path_, "ncp", kConsolePath, kProcessScope,
+      "Set the console path. Ex: /pagespeed_console");
+  add_ngx_option(
+      "", &NgxRewriteOptions::messages_path_, "nmp", kMessagesPath,
+      kProcessScope, "Set the messages path.  Ex: /ngx_pagespeed_message");
+  add_ngx_option(
+      "", &NgxRewriteOptions::admin_path_, "nap", kAdminPath,
+      kProcessScope, "Set the admin path.  Ex: /pagespeed_admin");
+  add_ngx_option(
+      "", &NgxRewriteOptions::global_admin_path_, "ngap", kGlobalAdminPath,
+      kProcessScope, "Set the global admin path.  Ex: /pagespeed_global_admin");
 
   MergeSubclassProperties(ngx_properties_);
 

src/ngx_rewrite_options.h

@@ -68,6 +68,24 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   static const NgxRewriteOptions* DynamicCast(const RewriteOptions* instance);
   static NgxRewriteOptions* DynamicCast(RewriteOptions* instance);
 
+  const GoogleString& statistics_path() const {
+    return statistics_path_.value();
+  }
+  const GoogleString& global_statistics_path() const {
+    return global_statistics_path_.value();
+  }
+  const GoogleString& console_path() const {
+    return console_path_.value();
+  }
+  const GoogleString& messages_path() const {
+    return messages_path_.value();
+  }
+  const GoogleString& admin_path() const {
+    return admin_path_.value();
+  }
+  const GoogleString& global_admin_path() const {
+    return global_admin_path_.value();
+  }
 
  private:
   // Helper methods for ParseAndSetOptions().  Each can:
@@ -109,10 +127,20 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   static void add_ngx_option(typename OptionClass::ValueType default_value,
                              OptionClass NgxRewriteOptions::*offset,
                              const char* id,
-                             StringPiece option_name) {
-    AddProperty(default_value, offset, id, option_name, ngx_properties_);
+                             StringPiece option_name,
+                             OptionScope scope,
+                             const char* help) {
+    AddProperty(default_value, offset, id, option_name, scope, help,
+                ngx_properties_);
   }
 
+  Option<GoogleString> statistics_path_;
+  Option<GoogleString> global_statistics_path_;
+  Option<GoogleString> console_path_;
+  Option<GoogleString> messages_path_;
+  Option<GoogleString> admin_path_;
+  Option<GoogleString> global_admin_path_;
+
   // Helper for ParseAndSetOptions.  Returns whether the two directives equal,
   // ignoring case.
   bool IsDirective(StringPiece config_directive, StringPiece compare_directive);

test/nginx_system_test.sh

@@ -1123,44 +1123,43 @@ http_proxy=$SECONDARY_HOSTNAME fetch_until $PROXIED_IMAGE \
 
 WGET_ARGS=""
 
-# TODO(jud): Enable this test when support for the admin page has been added.
-# start_test ShowCache without URL gets a form, inputs, preloaded UA.
-# ADMIN_CACHE=$PRIMARY_SERVER/pagespeed_admin/cache
-# OUT=$($WGET_DUMP $ADMIN_CACHE)
-# check_from "$OUT" fgrep -q "<form "
-# check_from "$OUT" fgrep -q "<input "
-# check_from "$OUT" fgrep -q "Cache-Control: max-age=0, no-cache"
-# # Preloaded user_agent value field leading with "Mozilla" set in
-# # ../automatic/system_test_helpers.sh to help test a "normal" flow.
-# check_from "$OUT" fgrep -q 'name=user_agent value="Mozilla'
-#
-# start_test ShowCache with bogus URL gives a 404
-# wget $PRIMARY_SERVER/pagespeed_cache?url=bogus_format >& /dev/null
-# check [ $? = 8 ]
-#
-# start_test ShowCache with valid, present URL, with unique options.
-# options="PageSpeedImageInlineMaxBytes=6765"
-# fetch_until -save $EXAMPLE_ROOT/rewrite_images.html?$options \
-#     'grep -c Puzzle\.jpg\.pagespeed\.ic\.' 1
-# URL_TAIL=$(grep Puzzle $FETCH_UNTIL_OUTFILE | cut -d \" -f 2)
-# SHOW_CACHE_URL=$EXAMPLE_ROOT/$URL_TAIL
-# SHOW_CACHE_QUERY=$ADMIN_CACHE?url=$SHOW_CACHE_URL\&$options
-# OUT=$($WGET_DUMP $SHOW_CACHE_QUERY)
-# check_from "$OUT" fgrep -q cache_ok:true
-# check_from "$OUT" fgrep -q mod_pagespeed_example/images/Puzzle.jpg
-#
-# function show_cache_after_flush() {
-#   start_test ShowCache with same URL and matching options misses after flush
-#   OUT=$($WGET_DUMP $SHOW_CACHE_QUERY)
-#   check_from "$OUT" fgrep -q cache_ok:false
-# }
-#
-# on_cache_flush show_cache_after_flush
-#
-# start_test ShowCache with same URL but new options misses.
-# options="PageSpeedImageInlineMaxBytes=6766"
-# OUT=$($WGET_DUMP $ADMIN_CACHE?url=$SHOW_CACHE_URL\&$options)
-# check_from "$OUT" fgrep -q cache_ok:false
+start_test ShowCache without URL gets a form, inputs, preloaded UA.
+ADMIN_CACHE=$PRIMARY_SERVER/pagespeed_admin/cache
+OUT=$($WGET_DUMP $ADMIN_CACHE)
+check_from "$OUT" fgrep -q "<form "
+check_from "$OUT" fgrep -q "<input "
+check_from "$OUT" fgrep -q "Cache-Control: max-age=0, no-cache"
+# Preloaded user_agent value field leading with "Mozilla" set in
+# ../automatic/system_test_helpers.sh to help test a "normal" flow.
+check_from "$OUT" fgrep -q 'name=user_agent value="Mozilla'
+
+start_test ShowCache with bogus URL gives a 404
+wget $PRIMARY_SERVER/pagespeed_cache?url=bogus_format >& /dev/null
+check [ $? = 8 ]
+
+start_test ShowCache with valid, present URL, with unique options.
+options="PageSpeedImageInlineMaxBytes=6765"
+fetch_until -save $EXAMPLE_ROOT/rewrite_images.html?$options \
+    'grep -c Puzzle\.jpg\.pagespeed\.ic\.' 1
+URL_TAIL=$(grep Puzzle $FETCH_UNTIL_OUTFILE | cut -d \" -f 2)
+SHOW_CACHE_URL=$EXAMPLE_ROOT/$URL_TAIL
+SHOW_CACHE_QUERY=$ADMIN_CACHE?url=$SHOW_CACHE_URL\&$options
+OUT=$($WGET_DUMP $SHOW_CACHE_QUERY)
+check_from "$OUT" fgrep -q cache_ok:true
+check_from "$OUT" fgrep -q mod_pagespeed_example/images/Puzzle.jpg
+
+function show_cache_after_flush() {
+  start_test ShowCache with same URL and matching options misses after flush
+  OUT=$($WGET_DUMP $SHOW_CACHE_QUERY)
+  check_from "$OUT" fgrep -q cache_ok:false
+}
+
+on_cache_flush show_cache_after_flush
+
+start_test ShowCache with same URL but new options misses.
+options="PageSpeedImageInlineMaxBytes=6766"
+OUT=$($WGET_DUMP $ADMIN_CACHE?url=$SHOW_CACHE_URL\&$options)
+check_from "$OUT" fgrep -q cache_ok:false
 
 # This is dependent upon having a /ngx_pagespeed_beacon handler.
 test_filter add_instrumentation beacons load.
@@ -2368,7 +2367,7 @@ start_test IPRO flow uses cache as expected.
 STATS=$OUTDIR/blocking_rewrite_stats
 IPRO_ROOT=http://ipro.example.com/mod_pagespeed_test/ipro
 URL=$IPRO_ROOT/test_image_dont_reuse2.png
-IPRO_STATS_URL=http://ipro.example.com/ngx_pagespeed_statistics?PageSpeed=off
+IPRO_STATS_URL=http://ipro.example.com/ngx_pagespeed_statistics
 
 # Initial stats.
 http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP $IPRO_STATS_URL > $STATS.0

test/pagespeed_test.conf.template

@@ -27,6 +27,13 @@ http {
   proxy_cache_path "@@PROXY_CACHE@@"  levels=1:2   keys_zone=htmlcache:60m inactive=90m  max_size=50m;
   proxy_temp_path "@@TMP_PROXY_CACHE@@";
 
+  pagespeed StatisticsPath /ngx_pagespeed_statistics;
+  pagespeed GlobalStatisticsPath /ngx_pagespeed_global_statistics;
+  pagespeed ConsolePath /pagespeed_console;
+  pagespeed MessagesPath /ngx_pagespeed_message;
+  pagespeed AdminPath /pagespeed_admin;
+  pagespeed GlobalAdminPath /pagespeed_global_admin;
+
   root "@@SERVER_ROOT@@";
 
   # Block 5a: Decide on Cache-Control header value to use for outgoing
@@ -234,6 +241,45 @@ http {
     }
   }
 
+  server {
+    listen @@SECONDARY_PORT@@;
+    server_name if-in-server.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+
+    pagespeed RewriteLevel PassThrough;
+    set $inline_javascript "No";
+
+    if ($http_x_custom_header_inline_js) {
+      # TODO(jefftk): Turn on NGX_HTTP_SIF_CONF and figure out how to get
+      # pagespeed directives inside of a server location block to be respected,
+      # then uncomment the following line and duplicate the if-in-location test
+      # for if-in-server.
+      #pagespeed EnableFilters inline_javascript;
+      set $inline_javascript "Yes";
+    }
+
+    add_header "X-Inline-Javascript" $inline_javascript;
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    server_name if-in-location.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+
+
+    location / {
+      set $inline_javascript "No";
+      pagespeed RewriteLevel PassThrough;
+
+      if ($http_x_custom_header_inline_js) {
+        pagespeed EnableFilters inline_javascript;
+        set $inline_javascript "Yes";
+      }
+
+      add_header "X-Inline-Javascript" $inline_javascript;
+    }
+  }
+
   server {
     listen @@SECONDARY_PORT@@;
     server_name mpd.example.com;
@@ -872,6 +918,11 @@ http {
     server_name  localhost;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
+    location ~ ^/pagespeed_admin {
+      allow 127.0.0.1;
+      deny all;
+    }
+
     location ~ "\.pagespeed\.([a-z]\.)?[a-z]{2}\.[^.]{10}\.[^.]+" {
       add_header "" "";
     }