Fix shutdown when ngx_pagespeed is completely disabled.

Fixes https://github.com/pagespeed/ngx_pagespeed/issues/1120

config

@@ -29,8 +29,8 @@ if [ "$mod_pagespeed_dir" = "unset" ] ; then
     echo "   You need to separately download the pagespeed library:"
     echo ""
     echo "     $ cd /path/to/ngx_pagespeed"
-    echo "     $ wget https://dl.google.com/dl/page-speed/psol/1.9.32.1.tar.gz"
-    echo "     $ tar -xzvf 1.9.32.1.tar.gz # expands to psol/"
+    echo "     $ wget https://dl.google.com/dl/page-speed/psol/1.10.33.5.tar.gz"
+    echo "     $ tar -xzvf 1.10.33.5.tar.gz # expands to psol/"
     echo ""
     echo "   Or see the installation instructions:"
     echo "     https://github.com/pagespeed/ngx_pagespeed#how-to-build"
@@ -118,6 +118,13 @@ case "$NGX_GCC_VER" in
   ;;
 esac
 
+# workaround for a bug in nginx-1.9.11, see:
+# http://hg.nginx.org/nginx/rev/ff1e625ae55b
+NGX_VERSION=`grep nginx_version src/core/nginx.h | sed -e 's/^.* \(.*\)$/\1/'`
+if [ "$NGX_VERSION" = "1009011" ]; then
+    CFLAGS="$CFLAGS -Wno-write-strings"
+fi
+
 if [ "$WNO_ERROR" = "YES" ]; then
     CFLAGS="$CFLAGS -Wno-error"
 fi
@@ -186,65 +193,7 @@ ngx_feature_test="
 # Test whether we have pagespeed and can compile and link against it.
 . "$ngx_addon_dir/cpp_feature"
 
-if [ $ngx_found = yes ]; then
-  ps_src="$ngx_addon_dir/src"
-  ngx_addon_name=ngx_pagespeed
-  NGX_ADDON_DEPS="$NGX_ADDON_DEPS \
-    $ps_src/log_message_handler.h \
-    $ps_src/ngx_base_fetch.h \
-    $ps_src/ngx_caching_headers.h \
-    $ps_src/ngx_event_connection.h \
-    $ps_src/ngx_fetch.h \
-    $ps_src/ngx_gzip_setter.h \
-    $ps_src/ngx_list_iterator.h \
-    $ps_src/ngx_message_handler.h \
-    $ps_src/ngx_pagespeed.h \
-    $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 \
-    $psol_binary"
-  NGX_ADDON_SRCS="$NGX_ADDON_SRCS \
-    $ps_src/log_message_handler.cc \
-    $ps_src/ngx_base_fetch.cc \
-    $ps_src/ngx_caching_headers.cc \
-    $ps_src/ngx_event_connection.cc \
-    $ps_src/ngx_fetch.cc \
-    $ps_src/ngx_gzip_setter.cc \
-    $ps_src/ngx_list_iterator.cc \
-    $ps_src/ngx_message_handler.cc \
-    $ps_src/ngx_pagespeed.cc \
-    $ps_src/ngx_rewrite_driver_factory.cc \
-    $ps_src/ngx_rewrite_options.cc \
-    $ps_src/ngx_server_context.cc \
-    $ps_src/ngx_url_async_fetcher.cc"
-  # Save our sources in a separate var since we may need it in config.make
-  PS_NGX_SRCS="$NGX_ADDON_SRCS"
-
-  if [ "$position_aux" = "true" ] ; then
-     HTTP_AUX_FILTER_MODULES="$HTTP_AUX_FILTER_MODULES $ngx_addon_name"
-  else
-    # Make pagespeed run immediately before gzip and Brotli.
-    if echo $HTTP_FILTER_MODULES | grep ngx_http_brotli_filter_module >/dev/null; then
-      module=ngx_http_brotli_filter_module
-    elif [ $HTTP_GZIP = YES ]; then
-      module=$HTTP_GZIP_FILTER_MODULE
-    else
-      module=$HTTP_RANGE_HEADER_FILTER_MODULE
-    fi
-
-    HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
-      sed "s/$module/$module $ngx_addon_name/")
-  fi
-
-  # Make the etag header filter run immediately before range header filter.
-  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
-    sed "s/$HTTP_RANGE_HEADER_FILTER_MODULE/$HTTP_RANGE_HEADER_FILTER_MODULE ngx_pagespeed_etag_filter/")
-
-  CORE_LIBS="$CORE_LIBS $pagespeed_libs"
-  CORE_INCS="$CORE_INCS $pagespeed_include"
-  echo "List of modules (in reverse order of applicability): "$HTTP_FILTER_MODULES
-else
+if [ $ngx_found = no ]; then
   cat << END
 $0: error: module ngx_pagespeed requires the pagespeed optimization library.
 Look in obj/autoconf.err for more details.
@@ -252,6 +201,115 @@ END
   exit 1
 fi
 
+ps_src="$ngx_addon_dir/src"
+ngx_addon_name=ngx_pagespeed
+NGX_ADDON_DEPS="$NGX_ADDON_DEPS \
+$ps_src/log_message_handler.h \
+$ps_src/ngx_base_fetch.h \
+$ps_src/ngx_caching_headers.h \
+$ps_src/ngx_event_connection.h \
+$ps_src/ngx_fetch.h \
+$ps_src/ngx_gzip_setter.h \
+$ps_src/ngx_list_iterator.h \
+$ps_src/ngx_message_handler.h \
+$ps_src/ngx_pagespeed.h \
+$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 \
+$psol_binary"
+NPS_SRCS=" \
+$ps_src/log_message_handler.cc \
+$ps_src/ngx_base_fetch.cc \
+$ps_src/ngx_caching_headers.cc \
+$ps_src/ngx_event_connection.cc \
+$ps_src/ngx_fetch.cc \
+$ps_src/ngx_gzip_setter.cc \
+$ps_src/ngx_list_iterator.cc \
+$ps_src/ngx_message_handler.cc \
+$ps_src/ngx_pagespeed.cc \
+$ps_src/ngx_rewrite_driver_factory.cc \
+$ps_src/ngx_rewrite_options.cc \
+$ps_src/ngx_server_context.cc \
+$ps_src/ngx_url_async_fetcher.cc"
+# Save our sources in a separate var since we may need it in config.make
+PS_NGX_SRCS="$NGX_ADDON_SRCS \
+$NPS_SRCS"
+
+# Make pagespeed run immediately before gzip and Brotli.
+if echo $HTTP_FILTER_MODULES | grep ngx_http_brotli_filter_module >/dev/null; then
+  next=ngx_http_brotli_filter_module
+elif [ $HTTP_GZIP = YES ]; then
+  next=ngx_http_gzip_filter_module
+else
+  next=ngx_http_range_header_filter_module
+fi
+
+if [ -n "$ngx_module_link" ]; then
+  # nginx-1.9.11+
+  ngx_module_type=HTTP_FILTER
+  ngx_module_name="ngx_pagespeed ngx_pagespeed_etag_filter"
+  ngx_module_incs="$ngx_feature_path"
+  ngx_module_deps=
+  ngx_module_srcs="$NPS_SRCS"
+  ngx_module_libs="$ngx_feature_libs"
+  ngx_module_order="ngx_http_range_header_filter_module\
+                    ngx_pagespeed_etag_filter\
+                    ngx_http_gzip_filter_module \
+                    ngx_http_brotli_filter_module \
+                    ngx_pagespeed \
+                    ngx_http_postpone_filter_module \
+                    ngx_http_ssi_filter_module \
+                    ngx_http_charset_filter_module \
+                    ngx_http_xslt_filter_module \
+                    ngx_http_image_filter_module \
+                    ngx_http_sub_filter_module \
+                    ngx_http_addition_filter_module \
+                    ngx_http_gunzip_filter_module \
+                    ngx_http_userid_filter_module \
+                    ngx_http_headers_filter_module"
+
+    . auto/module
+
+    if [ $ngx_module_link != DYNAMIC ]; then
+      # ngx_module_order doesn't work with static modules,
+      # so we must re-order filters here.
+      if [ "$position_aux" = "true" ] ; then
+        HTTP_AUX_FILTER_MODULES="$HTTP_AUX_FILTER_MODULES $ngx_addon_name"
+      else
+        HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES \
+                             | sed "s/ngx_pagespeed//" \
+                             | sed "s/$next/$next ngx_pagespeed/")
+      fi
+      # Make the etag header filter run immediately before range header filter.
+      HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES \
+                           | sed "s/ngx_pagespeed_etag_filter//" \
+                           | sed "s/ngx_http_range_header_filter_module/ngx_http_range_header_filter_module ngx_pagespeed_etag_filter/")
+    else
+      # config.make is not executed for dynamic modules
+      CFLAGS="$CFLAGS -Wno-c++11-extensions"
+      if [ "$position_aux" = "true" ] ; then
+        ngx_module_type=HTTP_AUX_FILTER
+        ngx_module_order=""
+      fi
+    fi
+else
+  CORE_LIBS="$CORE_LIBS $pagespeed_libs"
+  CORE_INCS="$CORE_INCS $pagespeed_include"
+  NGX_ADDON_SRCS="$PS_NGX_SRCS"
+  if [ "$position_aux" = "true" ] ; then
+    HTTP_AUX_FILTER_MODULES="$HTTP_AUX_FILTER_MODULES $ngx_addon_name"
+  else
+    HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES | sed "s/$next/$next $ngx_addon_name/")
+  fi
+
+  # Make the etag header filter run immediately before range header filter.
+  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
+    sed "s/ngx_http_range_header_filter_module/ngx_http_range_header_filter_module ngx_pagespeed_etag_filter/")
+fi
+
+echo "List of modules (in reverse order of applicability): "$HTTP_FILTER_MODULES
+
 # Test whether the compiler is compatible
 ngx_feature="psol-compiler-compat"
 ngx_feature_name=""

src/ngx_pagespeed.cc

@@ -302,6 +302,12 @@ ngx_int_t ps_base_fetch_handler(ngx_http_request_t* r) {
                 STR_CASE_EQ_LITERAL(header->key, "Last-Modified") ||
                 STR_CASE_EQ_LITERAL(header->key, "Expires"))))) {
           header->hash = 0;
+          if (STR_CASE_EQ_LITERAL(header->key, "Location")) {
+            // There's a possible issue with the location header, where setting
+            // the hash to 0 is not enough. See:
+            // https://github.com/nginx/nginx/blob/master/src/http/ngx_http_header_filter_module.c#L314
+            r->headers_out.location = NULL;
+          }
         }
       }
     } else {
@@ -452,11 +458,6 @@ void copy_response_headers_from_ngx(const ngx_http_request_t* r,
 
   headers->set_status_code(r->headers_out.status);
 
-  if (r->headers_out.location != NULL) {
-    headers->Add(HttpAttributes::kLocation,
-                 str_to_string_piece(r->headers_out.location->value));
-  }
-
   // Manually copy over the content type because it's not included in
   // request_->headers_out.headers.
   headers->Add(HttpAttributes::kContentType,
@@ -636,7 +637,7 @@ typedef struct {
 typedef struct {
   // If pagespeed is configured in some server block but not this one our
   // per-request code will be invoked but server context will be null.  In those
-  // cases we neet to short circuit, not changing anything.  Currently our
+  // cases we need to short circuit, not changing anything.  Currently our
   // header filter, body filter, and content handler all do this, but if anyone
   // adds another way for nginx to give us a request to process we need to check
   // there as well.
@@ -986,6 +987,7 @@ void* ps_create_main_conf(ngx_conf_t* cf) {
       "" /* hostname, not used */,
       -1 /* port, not used */);
   active_driver_factory = cfg_m->driver_factory;
+  active_driver_factory->LoggingInit(ngx_cycle->log, false);
   cfg_m->driver_factory->Init();
   ps_set_conf_cleanup_handler(cf, ps_cleanup_main_conf, cfg_m);
   return cfg_m;
@@ -2478,8 +2480,8 @@ ngx_int_t send_out_headers_and_body(
 
   rc = ngx_http_send_header(r);
 
-  if (rc != NGX_OK) {
-    return NGX_ERROR;
+  if (rc == NGX_ERROR || rc > NGX_OK || r->header_only) {
+    return rc;
   }
 
   // Send the body.
@@ -3051,7 +3053,7 @@ ngx_int_t ps_init_module(ngx_cycle_t* cycle) {
       return NGX_ERROR;
     }
 
-    cfg_m->driver_factory->LoggingInit(cycle->log);
+    cfg_m->driver_factory->LoggingInit(cycle->log, true);
     cfg_m->driver_factory->RootInit();
   } else {
     delete cfg_m->driver_factory;
@@ -3065,7 +3067,9 @@ void ps_exit_child_process(ngx_cycle_t* cycle) {
   ps_main_conf_t* cfg_m = static_cast<ps_main_conf_t*>(
       ngx_http_cycle_get_module_main_conf(cycle, ngx_pagespeed));
   NgxBaseFetch::Terminate();
+  if (cfg_m->driver_factory != NULL) {
     cfg_m->driver_factory->ShutDown();
+  }
 }
 
 // Called when nginx forks worker processes.  No threads should be started
@@ -3083,7 +3087,7 @@ ngx_int_t ps_init_child_process(ngx_cycle_t* cycle) {
 
   // ChildInit() will initialise all ServerContexts, which we need to
   // create ProxyFetchFactories below
-  cfg_m->driver_factory->LoggingInit(cycle->log);
+  cfg_m->driver_factory->LoggingInit(cycle->log, true);
   cfg_m->driver_factory->ChildInit();
 
   ngx_http_core_main_conf_t* cmcf = static_cast<ngx_http_core_main_conf_t*>(

src/ngx_rewrite_driver_factory.cc

@@ -208,10 +208,11 @@ void NgxRewriteDriverFactory::StartThreads() {
   threads_started_ = true;
 }
 
-void NgxRewriteDriverFactory::LoggingInit(ngx_log_t* log) {
+void NgxRewriteDriverFactory::LoggingInit(
+    ngx_log_t* log, bool may_install_crash_handler) {
   log_ = log;
   net_instaweb::log_message_handler::Install(log);
-  if (install_crash_handler()) {
+  if (may_install_crash_handler && install_crash_handler()) {
     NgxMessageHandler::InstallCrashHandler(log);
   }
   ngx_message_handler_->set_log(log);

src/ngx_rewrite_driver_factory.h

@@ -115,7 +115,7 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
     return process_script_variables_;
   }
 
-  void LoggingInit(ngx_log_t* log);
+  void LoggingInit(ngx_log_t* log, bool may_install_crash_handler);
 
   virtual void ShutDownMessageHandlers();