remove C++11isms from 1.11.33.3, for use with dynamic modules

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

config

@@ -17,6 +17,8 @@
 #   PSOL_BINARY: absolute path to pagespeed_automatic.a
 
 mod_pagespeed_dir="${MOD_PAGESPEED_DIR:-unset}"
+position_aux="${POSITION_AUX:-unset}"
+
 if [ "$mod_pagespeed_dir" = "unset" ] ; then
   mod_pagespeed_dir="$ngx_addon_dir/psol/include"
   build_from_source=false
@@ -27,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.14.tar.gz"
-    echo "     $ tar -xzvf 1.9.32.14.tar.gz # expands to psol/"
+    echo "     $ wget https://dl.google.com/dl/page-speed/psol/1.11.33.3.tar.gz"
+    echo "     $ tar -xzvf 1.11.33.3.tar.gz # expands to psol/"
     echo ""
     echo "   Or see the installation instructions:"
     echo "     https://github.com/pagespeed/ngx_pagespeed#how-to-build"
@@ -69,6 +71,20 @@ else
   buildtype=Release
 fi
 
+# If the compiler is gcc, we want to use g++ to link, if at all possible,
+# so that -static-libstdc++ works.
+# Annoyingly, the feature test doesn't even use $LINK for linking, so that
+# needs an explicit -lstdc++
+pagespeed_libs=
+ps_maybe_gpp_base=`basename $CC| sed s/gcc/g++/`
+ps_maybe_gpp="`dirname $CC`/$ps_maybe_gpp_base"
+if [ -n "$NGX_GCC_VER" -a \( -x "$ps_maybe_gpp" \) ]; then
+  LINK=$ps_maybe_gpp
+  NGX_TEST_LD_OPT="$NGX_TEST_LD_OPT -lstdc++"
+else
+  pagespeed_libs="-lstdc++"
+fi
+
 # The compiler needs to know that __sync_add_and_fetch_4 is ok,
 # and this requires an instruction that didn't exist on i586 or i386.
 if [ "$uname_arch" = "i686" ]; then
@@ -77,6 +93,13 @@ fi
 
 CFLAGS="$CFLAGS $FLAG_MARCH"
 
+# For now, standardize on gcc-4.x ABI --- if we don't set this, people building
+# with new gcc defaulting to gcc-5 C++11 ABI will have build trouble linking
+# to our libpsol.a
+# See https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
+CFLAGS="$CFLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
+CC_TEST_FLAGS="$CC_TEST_FLAGS -D_GLIBCXX_USE_CXX11_ABI=0"
+
 case "$NGX_GCC_VER" in
   4.8*)
     # On GCC 4.8 and above, -Wall enables -Wunused-local-typedefs.  This breaks
@@ -95,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
@@ -103,7 +133,7 @@ psol_binary="${PSOL_BINARY:-unset}"
 if [ "$psol_binary" = "unset" ] ; then
   if $build_from_source ; then
     psol_binary="\
-        $mod_pagespeed_dir/net/instaweb/automatic/pagespeed_automatic.a"
+        $mod_pagespeed_dir/pagespeed/automatic/pagespeed_automatic.a"
   else
     psol_library_dir="$ngx_addon_dir/psol/lib/$buildtype/$os_name/$arch_name"
     psol_binary="$psol_library_dir/pagespeed_automatic.a"
@@ -139,9 +169,10 @@ pagespeed_include="\
   $mod_pagespeed_dir/third_party/aprutil/gen/arch/$os_name/$arch_name/include"
 ngx_feature_path="$pagespeed_include"
 
-pagespeed_libs="-lstdc++ $psol_binary -lrt -pthread -lm"
+pagespeed_libs="$pagespeed_libs $psol_binary -lrt -pthread -lm"
 ngx_feature_libs="$pagespeed_libs"
 ngx_feature_test="
+
   GoogleString output_buffer;
   net_instaweb::StringWriter write_to_string(&output_buffer);
 
@@ -162,49 +193,160 @@ 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_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_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"
-
-  # Make pagespeed run immediately before gzip.
-  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
-    sed "s/$HTTP_GZIP_FILTER_MODULE/$HTTP_GZIP_FILTER_MODULE $ngx_addon_name/")
-  # Make the etag header filter run immediately after gzip.
-  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
-    sed "s/$HTTP_GZIP_FILTER_MODULE/ngx_pagespeed_etag_filter $HTTP_GZIP_FILTER_MODULE/")
-  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.
+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=""
+ngx_feature_run=no
+ngx_feature_incs=""
+ngx_feature_path=""
+ngx_feature_libs="-lstdc++"
+ngx_feature_test="
+
+#if defined(__clang__) && defined(__GLIBCXX__)
+   // See https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html#abi.versioning 
+   // for a list of various values of __GLIBCXX__. Note that they're not monotonic
+   // with respect to version numbers.
+   #if __GLIBCXX__ == 20120322  || __GLIBCXX__ == 20120614
+      #error \"clang is using libstdc++ 4.7.0 or 4.7.1, which can cause binary incompatibility.\"
+   #endif
+#endif
+
+#if !defined(__clang__) && defined(__GNUC__)
+   #if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
+      #error \"GCC < 4.8 no longer supported. Please use gcc >= 4.8 or clang >= 3.3\"
+   #endif
+#endif
+
+#if defined(__clang__)
+   #if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 3)
+      #error \"Please use gcc >= 4.8 or clang >= 3.3\"
+   #endif
+#endif
+"
+
+. "$ngx_addon_dir/cpp_feature"
+
+if [ $ngx_found = no ]; then
+  cat << END
+$0: error: module ngx_pagespeed requires gcc >= 4.8 or clang >= 3.3.
+See https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source for some recommendations.
 Look in objs/autoconf.err for more details.
 END
   exit 1

config.make

@@ -0,0 +1,7 @@
+if [ -n "$NGX_CLANG_VER" ]; then
+  # Chromium headers assume clang is always in C++11 mode. Oblige it.
+  for ps_src_file in $PS_NGX_SRCS; do
+    ps_obj_file="$NGX_OBJS/addon/src/`basename $ps_src_file .cc`.o"
+    echo "$ps_obj_file : CFLAGS += --std=c++11" >> $NGX_MAKEFILE
+  done
+fi

src/log_message_handler.cc

@@ -29,7 +29,7 @@
 #include "base/debug/stack_trace.h"
 #include "base/logging.h"
 #include "net/instaweb/public/version.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/string_util.h"
 
 // Make sure we don't attempt to use LOG macros here, since doing so
 // would cause us to go into an infinite log loop.
@@ -38,7 +38,7 @@
 
 namespace {
 
-ngx_log_t* log = NULL;
+ngx_log_t* ngx_log = NULL;
 
 ngx_uint_t GetNgxLogLevel(int severity) {
   switch (severity) {
@@ -78,7 +78,7 @@ bool LogMessageHandler(int severity, const char* file, int line,
     message.resize(last_msg_character_index);
   }
 
-  ngx_log_error(this_log_level, log, 0, "[ngx_pagespeed %s] %s",
+  ngx_log_error(this_log_level, ngx_log, 0, "[ngx_pagespeed %s] %s",
                 net_instaweb::kModPagespeedVersion,
                 message.c_str());
 
@@ -99,12 +99,12 @@ namespace log_message_handler {
 
 
 void Install(ngx_log_t* log_in) {
-  log = log_in;
+  ngx_log = log_in;
   logging::SetLogMessageHandler(&LogMessageHandler);
 
   // All VLOG(2) and higher will be displayed as DEBUG logs if the nginx log
   // level is DEBUG.
-  if (log->log_level >= NGX_LOG_DEBUG) {
+  if (ngx_log->log_level >= NGX_LOG_DEBUG) {
     logging::SetMinLogLevel(-2);
   }
 }

src/ngx_base_fetch.cc

@@ -12,40 +12,194 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
+ *
+ * Author: jefftk@google.com (Jeff Kaufman)
  */
 
-// Author: jefftk@google.com (Jeff Kaufman)
+#include "ngx_pagespeed.h"  // Must come first, see comments in CollectHeaders.
+
+#include <unistd.h> //for usleep
 
 #include "ngx_base_fetch.h"
+#include "ngx_event_connection.h"
 #include "ngx_list_iterator.h"
 
 #include "ngx_pagespeed.h"
 
-#include "net/instaweb/http/public/response_headers.h"
+#include "net/instaweb/rewriter/public/rewrite_driver.h"
+#include "net/instaweb/rewriter/public/rewrite_options.h"
 #include "net/instaweb/rewriter/public/rewrite_stats.h"
-#include "net/instaweb/util/public/google_message_handler.h"
-#include "net/instaweb/util/public/message_handler.h"
+#include "pagespeed/kernel/base/google_message_handler.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/posix_timer.h"
+#include "pagespeed/kernel/http/response_headers.h"
 
 namespace net_instaweb {
 
-NgxBaseFetch::NgxBaseFetch(ngx_http_request_t* r, int pipe_fd,
+const char kHeadersComplete = 'H';
+const char kFlush = 'F';
+const char kDone = 'D';
+
+NgxEventConnection* NgxBaseFetch::event_connection = NULL;
+int NgxBaseFetch::active_base_fetches = 0;
+
+NgxBaseFetch::NgxBaseFetch(StringPiece url,
+                           ngx_http_request_t* r,
                            NgxServerContext* server_context,
                            const RequestContextPtr& request_ctx,
-                           PreserveCachingHeaders preserve_caching_headers)
+                           PreserveCachingHeaders preserve_caching_headers,
+                           NgxBaseFetchType base_fetch_type,
+                           const RewriteOptions* options)
     : AsyncFetch(request_ctx),
+      url_(url.data(), url.size()),
       request_(r),
       server_context_(server_context),
+      options_(options),
       done_called_(false),
       last_buf_sent_(false),
-      pipe_fd_(pipe_fd),
       references_(2),
-      ipro_lookup_(false),
-      preserve_caching_headers_(preserve_caching_headers) {
+      base_fetch_type_(base_fetch_type),
+      preserve_caching_headers_(preserve_caching_headers),
+      detached_(false),
+      suppress_(false) {
   if (pthread_mutex_init(&mutex_, NULL)) CHECK(0);
+  __sync_add_and_fetch(&NgxBaseFetch::active_base_fetches, 1);
 }
 
 NgxBaseFetch::~NgxBaseFetch() {
   pthread_mutex_destroy(&mutex_);
+  __sync_add_and_fetch(&NgxBaseFetch::active_base_fetches, -1);
+}
+
+bool NgxBaseFetch::Initialize(ngx_cycle_t* cycle) {
+  CHECK(event_connection == NULL) << "event connection already set";
+  event_connection = new NgxEventConnection(ReadCallback);
+  return event_connection->Init(cycle);
+}
+
+void NgxBaseFetch::Terminate() {
+  if (event_connection != NULL) {
+    GoogleMessageHandler handler;
+    PosixTimer timer;
+    int64 timeout_us = Timer::kSecondUs * 30;
+    int64 end_us = timer.NowUs() + timeout_us;
+    static unsigned int sleep_microseconds = 100;
+
+    handler.Message(
+        kInfo,"NgxBaseFetch::Terminate rounding up %d active base fetches.",
+        NgxBaseFetch::active_base_fetches);
+
+    // Try to continue processing and get the active base fetch count to 0
+    // untill the timeout expires.
+    // TODO(oschaaf): This needs more work.
+    while (NgxBaseFetch::active_base_fetches > 0 && end_us > timer.NowUs()) {
+      event_connection->Drain();
+      usleep(sleep_microseconds);
+    }
+
+    if (NgxBaseFetch::active_base_fetches != 0) {
+      handler.Message(
+          kWarning,"NgxBaseFetch::Terminate timed out with %d active base fetches.",
+          NgxBaseFetch::active_base_fetches);
+    }
+
+    // Close down the named pipe.
+    event_connection->Shutdown();
+    delete event_connection;
+    event_connection = NULL;
+  }
+}
+
+const char* BaseFetchTypeToCStr(NgxBaseFetchType type) {
+  switch(type) {
+    case kPageSpeedResource:
+      return "ps resource";
+    case kHtmlTransform:
+      return "html transform";
+    case kAdminPage:
+      return "admin page";
+    case kIproLookup:
+      return "ipro lookup";
+    case kPageSpeedProxy:
+      return "pagespeed proxy";
+  }
+  CHECK(false);
+  return "can't get here";
+}
+
+// TODO(oschaaf): replace the ngx_log_error with VLOGS or pass in a
+// MessageHandler and use that.
+void NgxBaseFetch::ReadCallback(const ps_event_data& data) {
+  NgxBaseFetch* base_fetch = reinterpret_cast<NgxBaseFetch*>(data.sender);
+  ngx_http_request_t* r = base_fetch->request();
+  bool detached = base_fetch->detached();
+#if (NGX_DEBUG)  // `type` is unused if NGX_DEBUG isn't set, needed for -Werror.
+  const char* type = BaseFetchTypeToCStr(base_fetch->base_fetch_type_);
+#endif
+  int refcount = base_fetch->DecrementRefCount();
+
+#if (NGX_DEBUG)
+  ngx_log_error(NGX_LOG_DEBUG, ngx_cycle->log, 0,
+     "pagespeed [%p] event: %c. bf:%p (%s) - refcnt:%d - det: %c", r,
+     data.type, base_fetch, type, refcount, detached ? 'Y': 'N');
+#endif
+
+  // If we ended up destructing the base fetch, or the request context is
+  // detached, skip this event.
+  if (refcount == 0 || detached) {
+    return;
+  }
+
+  ps_request_ctx_t* ctx = ps_get_request_context(r);
+
+  // If our request context was zeroed, skip this event.
+  // See https://github.com/pagespeed/ngx_pagespeed/issues/1081
+  if (ctx == NULL) {
+    // Should not happen normally, when it does this message will cause our
+    // system tests to fail.
+    ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0,
+        "pagespeed [%p] skipping event: request context gone", r);
+    return;
+  }
+
+  // Normally we expect the sender to equal the active NgxBaseFetch instance.
+  DCHECK(data.sender == ctx->base_fetch);
+
+  // If someone changed our request context or NgxBaseFetch, skip processing.
+  if (data.sender != ctx->base_fetch) {
+      ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0,
+          "pagespeed [%p] skipping event: event originating from disassociated"
+          " NgxBaseFetch instance.", r);
+      return;
+  }
+
+  int rc;
+  bool run_posted = true;
+  // If we are unlucky enough to have our connection finalized mid-ipro-lookup,
+  // we must enter a different flow. Also see ps_in_place_check_header_filter().
+  if ((ctx->base_fetch->base_fetch_type_ != kIproLookup)
+      && r->connection->error) {
+    ngx_log_error(NGX_LOG_DEBUG, ngx_cycle->log, 0,
+      "pagespeed [%p] request already finalized %d", r, r->count);
+    rc = NGX_ERROR;
+    run_posted = false;
+  } else {
+    rc = ps_base_fetch::ps_base_fetch_handler(r);
+  }
+
+#if (NGX_DEBUG)
+  ngx_log_error(NGX_LOG_DEBUG, ngx_cycle->log, 0,
+                "pagespeed [%p] ps_base_fetch_handler() returned %d for %c",
+                r, rc, data.type);
+#endif
+
+  ngx_connection_t* c = r->connection;
+  ngx_http_finalize_request(r, rc);
+
+  if (run_posted) {
+    // See http://forum.nginx.org/read.php?2,253006,253061
+    ngx_http_run_posted_requests(c);
+  }
 }
 
 void NgxBaseFetch::Lock() {
@@ -104,6 +258,13 @@ ngx_int_t NgxBaseFetch::CollectAccumulatedWrites(ngx_chain_t** link_ptr) {
 }
 
 ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
+  // nginx defines _FILE_OFFSET_BITS to 64, which changes the size of off_t.
+  // If a standard header is accidentally included before the nginx header,
+  // on a 32-bit system off_t will be 4 bytes and we don't assign all the
+  // bits of content_length_n. Sanity check that did not happen.
+  // This could use static_assert, but this file is not built with --std=c++11.
+  bool sanity_check_off_t[sizeof(off_t) == 8 ? 1 : -1] __attribute__ ((unused));
+
   const ResponseHeaders* pagespeed_headers = response_headers();
 
   if (content_length_known()) {
@@ -115,21 +276,19 @@ ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
                                       preserve_caching_headers_);
 }
 
-void NgxBaseFetch::RequestCollection() {
-  int rc;
-  char c = 'A';  // What byte we write is arbitrary.
-  while (true) {
-    rc = write(pipe_fd_, &c, 1);
-    if (rc == 1) {
-      break;
-    } else if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) {
-      // TODO(jefftk): is this rare enough that spinning isn't a problem?  Could
-      // we get into a case where the pipe fills up and we spin forever?
+void NgxBaseFetch::RequestCollection(char type) {
+  if (suppress_) {
+    return;
+  }
 
-    } else {
-      perror("NgxBaseFetch::RequestCollection");
-      break;
-    }
+  // We must optimistically increment the refcount, and decrement it
+  // when we conclude we failed. If we only increment on a successfull write,
+  // there's a small chance that between writing and adding to the refcount
+  // both pagespeed and nginx will release their refcount -- destructing
+  // this NgxBaseFetch instance.
+  IncrementRefCount();
+  if (!event_connection->WriteEvent(type, this)) {
+    DecrementRefCount();
   }
 }
 
@@ -137,49 +296,60 @@ void NgxBaseFetch::HandleHeadersComplete() {
   int status_code = response_headers()->status_code();
   bool status_ok = (status_code != 0) && (status_code < 400);
 
-  if (!ipro_lookup_ || status_ok) {
+  if ((base_fetch_type_ != kIproLookup) || status_ok) {
     // If this is a 404 response we need to count it in the stats.
     if (response_headers()->status_code() == HttpStatus::kNotFound) {
       server_context_->rewrite_stats()->resource_404_count()->Add(1);
     }
   }
 
-  // For the IPRO lookup, suppress notification of the nginx side here.
-  // If we send both this event and the one from done, nasty stuff will happen
-  // if we loose the race with with the nginx side destructing this base fetch
-  // instance (and thereby clearing the byte and its pending extraneous event.
-  if (!ipro_lookup_) {
-    RequestCollection();  // Headers available.
+  RequestCollection(kHeadersComplete);  // Headers available.
+
+  // For the IPRO lookup, supress notification of the nginx side here.
+  // If we send both the headerscomplete event and the one from done, nasty
+  // stuff will happen if we loose the race with with the nginx side destructing
+  // this base fetch instance.
+  if (base_fetch_type_ == kIproLookup && !status_ok) {
+    suppress_ = true;
   }
 }
 
 bool NgxBaseFetch::HandleFlush(MessageHandler* handler) {
-  RequestCollection();  // A new part of the response body is available.
+  RequestCollection(kFlush);  // A new part of the response body is available
   return true;
 }
 
-void NgxBaseFetch::Release() {
-  DecrefAndDeleteIfUnreferenced();
+int NgxBaseFetch::DecrementRefCount() {
+  return DecrefAndDeleteIfUnreferenced();
 }
 
-void NgxBaseFetch::DecrefAndDeleteIfUnreferenced() {
+int NgxBaseFetch::IncrementRefCount() {
+  return __sync_add_and_fetch(&references_, 1);
+}
+
+int NgxBaseFetch::DecrefAndDeleteIfUnreferenced() {
   // Creates a full memory barrier.
-  if (__sync_add_and_fetch(&references_, -1) == 0) {
+  int r = __sync_add_and_fetch(&references_, -1);
+  if (r == 0) {
     delete this;
   }
+  return r;
 }
 
 void NgxBaseFetch::HandleDone(bool success) {
   // TODO(jefftk): it's possible that instead of locking here we can just modify
   // CopyBufferToNginx to only read done_called_ once.
+  CHECK(!done_called_) << "Done already called!";
   Lock();
   done_called_ = true;
   Unlock();
-
-  close(pipe_fd_);  // Indicates to nginx that we're done with the rewrite.
-  pipe_fd_ = -1;
-
+  RequestCollection(kDone);
   DecrefAndDeleteIfUnreferenced();
 }
 
+bool NgxBaseFetch::IsCachedResultValid(const ResponseHeaders& headers) {
+  return OptionsAwareHTTPCacheCallback::IsCacheValid(
+      url_, *options_, request_context(), headers);
+}
+
 }  // namespace net_instaweb

src/ngx_base_fetch.h

@@ -17,22 +17,35 @@
 // Author: jefftk@google.com (Jeff Kaufman)
 //
 // Collects output from pagespeed and buffers it until nginx asks for it.
-// Notifies nginx via pipe to call CollectAccumulatedWrites() on flush.
+// Notifies nginx via NgxEventConnection to call ReadCallback() when
+// the headers are computed, when a flush should be performed, and when done.
 //
 //  - nginx creates a base fetch and passes it to a new proxy fetch.
 //  - The proxy fetch manages rewriting and thread complexity, and through
 //    several chained steps passes rewritten html to HandleWrite().
 //  - Written data is buffered.
-//  - When Flush() is called the base fetch writes a byte to a pipe nginx is
-//    watching so nginx knows to call CollectAccumulatedWrites() to pick up the
-//    rewritten html.
-//  - When Done() is called the base fetch closes the pipe, which tells nginx to
-//    make a final call to CollectAccumulatedWrites().
+//  - When HandleHeadersComplete(), HandleFlush(), or HandleDone() is called by
+//    PSOL, events are written to NgxEventConnection which will end up being
+//    handled by ReadCallback() on nginx's thread.
+//    When applicable, request processing will be continued via a call to
+//    ps_base_fetch_handler().
+//  - ps_base_fetch_handler() will pull the header and body bytes from PSOL
+//    via CollectAccumulatedWrites() and write those to the module's output.
 //
-// This class is referred two in two places: the proxy fetch and nginx's
-// request.  It must stay alive until both are finished.  The proxy fetch will
-// call Done() to indicate this; nginx will call Release().  Once both Done()
-// and Release() have been called this class will delete itself.
+// This class is referred to in three places: the proxy fetch, nginx's request,
+// and pending events written to the associated NgxEventConnection. It must stay
+// alive until the proxy fetch and nginx request are finished, and no more
+// events are pending.
+//  - The proxy fetch will call Done() to indicate this.
+//  - nginx will call Detach() when the associated request is handled
+//    completely (e.g. the request context is about to be destroyed).
+//  - ReadCallback() will call DecrementRefCount() on instances associated to
+//    events it handles.
+//
+// When the last reference is dropped, this class will delete itself.
+//
+// TODO(jmarantz): consider sharing the cache-invalidation infrastructure
+// with ApacheFetch, using a common base class.
 
 #ifndef NGX_BASE_FETCH_H_
 #define NGX_BASE_FETCH_H_
@@ -45,22 +58,46 @@ extern "C" {
 
 #include "ngx_pagespeed.h"
 
+#include "ngx_event_connection.h"
 #include "ngx_server_context.h"
 
 #include "net/instaweb/http/public/async_fetch.h"
-#include "net/instaweb/http/public/headers.h"
-#include "net/instaweb/util/public/string.h"
+#include "net/instaweb/rewriter/public/rewrite_options.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/http/headers.h"
 
 namespace net_instaweb {
 
+enum NgxBaseFetchType {
+  kIproLookup,
+  kHtmlTransform,
+  kPageSpeedResource,
+  kAdminPage,
+  kPageSpeedProxy
+};
+
 class NgxBaseFetch : public AsyncFetch {
  public:
-  NgxBaseFetch(ngx_http_request_t* r, int pipe_fd,
+  NgxBaseFetch(StringPiece url, ngx_http_request_t* r,
                NgxServerContext* server_context,
                const RequestContextPtr& request_ctx,
-               PreserveCachingHeaders preserve_caching_headers);
+               PreserveCachingHeaders preserve_caching_headers,
+               NgxBaseFetchType base_fetch_type,
+               const RewriteOptions* options);
   virtual ~NgxBaseFetch();
 
+  // Statically initializes event_connection, require for PSOL and nginx to
+  // communicate.
+  static bool Initialize(ngx_cycle_t* cycle);
+
+  // Attempts to finish up request processing queued up in the named pipe and
+  // PSOL for a fixed amount of time. If time is up, a fast and rough shutdown
+  // is attempted.
+  // Statically terminates and NULLS event_connection.
+  static void Terminate();
+
+  static void ReadCallback(const ps_event_data& data);
+
   // Puts a chain in link_ptr if we have any output data buffered.  Returns
   // NGX_OK on success, NGX_ERROR on errors.  If there's no data to send, sends
   // data only if Done() has been called.  Indicates the end of output by
@@ -77,9 +114,24 @@ class NgxBaseFetch : public AsyncFetch {
   // time for resource fetches.  Not called at all for proxy fetches.
   ngx_int_t CollectHeaders(ngx_http_headers_out_t* headers_out);
 
-  // Called by nginx when it's done with us.
-  void Release();
-  void set_ipro_lookup(bool x) { ipro_lookup_ = x; }
+  // Called by nginx to decrement the refcount.
+  int DecrementRefCount();
+
+  // Called by pagespeed to increment the refcount.
+  int IncrementRefCount();
+
+  // Detach() is called when the nginx side releases this base fetch. It
+  // sets detached_ to true and decrements the refcount. We need to know
+  // this to be able to handle events which nginx request context has been
+  // released while the event was in-flight.
+  void Detach() { detached_ = true; DecrementRefCount(); }
+
+  bool detached() { return detached_; }
+
+  ngx_http_request_t* request() { return request_; }
+  NgxBaseFetchType base_fetch_type() { return base_fetch_type_; }
+
+  virtual bool IsCachedResultValid(const ResponseHeaders& headers);
 
  private:
   virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler);
@@ -89,7 +141,7 @@ class NgxBaseFetch : public AsyncFetch {
 
   // Indicate to nginx that we would like it to call
   // CollectAccumulatedWrites().
-  void RequestCollection();
+  void RequestCollection(char type);
 
   // Lock must be acquired first.
   // Returns:
@@ -105,20 +157,31 @@ class NgxBaseFetch : public AsyncFetch {
 
   // Called by Done() and Release().  Decrements our reference count, and if
   // it's zero we delete ourself.
-  void DecrefAndDeleteIfUnreferenced();
+  int DecrefAndDeleteIfUnreferenced();
 
+  static NgxEventConnection* event_connection;
+
+  // Live count of NgxBaseFetch instances that are currently in use.
+  static int active_base_fetches;
+
+  GoogleString url_;
   ngx_http_request_t* request_;
   GoogleString buffer_;
   NgxServerContext* server_context_;
+  const RewriteOptions* options_;
   bool done_called_;
   bool last_buf_sent_;
-  int pipe_fd_;
   // How many active references there are to this fetch. Starts at two,
-  // decremented once when Done() is called and once when Release() is called.
+  // decremented once when Done() is called and once when Detach() is called.
+  // Incremented for each event written by pagespeed for this NgxBaseFetch, and
+  // decremented on the nginx side for each event read for it.
   int references_;
   pthread_mutex_t mutex_;
-  bool ipro_lookup_;
+  NgxBaseFetchType base_fetch_type_;
   PreserveCachingHeaders preserve_caching_headers_;
+  // Set to true just before the nginx side releases its reference
+  bool detached_;
+  bool suppress_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxBaseFetch);
 };

src/ngx_event_connection.cc

@@ -0,0 +1,172 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Author: oschaaf@we-amp.com (Otto van der Schaaf)
+
+extern "C" {
+
+#include <ngx_channel.h>
+
+}
+
+#include "ngx_event_connection.h"
+
+#include "pagespeed/kernel/base/google_message_handler.h"
+#include "pagespeed/kernel/base/message_handler.h"
+
+namespace net_instaweb {
+
+  NgxEventConnection::NgxEventConnection(callbackPtr callback)
+    : event_handler_(callback) {
+}
+
+bool NgxEventConnection::Init(ngx_cycle_t* cycle) {
+  int file_descriptors[2];
+
+  if (pipe(file_descriptors) != 0) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, "pagespeed: pipe() failed");
+    return false;
+  }
+  if (ngx_nonblocking(file_descriptors[0]) == -1) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, ngx_socket_errno,
+                  ngx_nonblocking_n "pagespeed:  pipe[0] failed");
+  } else if (ngx_nonblocking(file_descriptors[1]) == -1) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, ngx_socket_errno,
+                  ngx_nonblocking_n "pagespeed:  pipe[1] failed");
+  } else if (!CreateNgxConnection(cycle, file_descriptors[0])) {
+    ngx_log_error(NGX_LOG_EMERG, cycle->log, 0,
+                  "pagespeed: failed to create connection.");
+  } else {
+    pipe_read_fd_ = file_descriptors[0];
+    pipe_write_fd_ = file_descriptors[1];
+    return true;
+  }
+  close(file_descriptors[0]);
+  close(file_descriptors[1]);
+  return false;
+}
+
+bool NgxEventConnection::CreateNgxConnection(ngx_cycle_t* cycle,
+                                             ngx_fd_t pipe_fd) {
+  // pipe_fd (the read side of the pipe will end up as c->fd on the
+  // underlying ngx_connection_t that gets created here)
+  ngx_int_t rc = ngx_add_channel_event(cycle, pipe_fd, NGX_READ_EVENT,
+      &NgxEventConnection::ReadEventHandler);
+  return rc  == NGX_OK;
+}
+
+void NgxEventConnection::ReadEventHandler(ngx_event_t* ev) {
+  ngx_connection_t* c = static_cast<ngx_connection_t*>(ev->data);
+  ngx_int_t result = ngx_handle_read_event(ev, 0);
+  if (result != NGX_OK) {
+    CHECK(false) << "pagespeed: ngx_handle_read_event error: " << result;
+  }
+
+  if (ev->timedout) {
+    ev->timedout = 0;
+    return;
+  }
+
+  if (!NgxEventConnection::ReadAndNotify(c->fd)) {
+    // This was copied from ngx_channel_handler(): for epoll, we need to call
+    // ngx_del_conn(). Sadly, no documentation as to why.
+    if (ngx_event_flags & NGX_USE_EPOLL_EVENT) {
+      ngx_del_conn(c, 0);
+    }
+    ngx_close_connection(c);
+    ngx_del_event(ev, NGX_READ_EVENT, 0);
+  }
+}
+
+// Deserialize ps_event_data's from the pipe as they become available.
+// Subsequently do some bookkeeping, cleanup, and error checking to keep
+// the mess out of ps_base_fetch_handler.
+bool NgxEventConnection::ReadAndNotify(ngx_fd_t fd) {
+  while (true) {
+    // We read only one ps_event_data at a time for now:
+    // We can end up recursing all the way and end up calling ourselves here.
+    // If that happens in the middle of looping over multiple ps_event_data's we
+    // have obtained with read(), the results from the next read() will make us
+    // process events out of order. Which can give headaches.
+    // Alternatively, we could maintain a queue to make sure we process in
+    // sequence
+    ps_event_data data;
+    ngx_int_t size = read(fd, static_cast<void*>(&data), sizeof(data));
+
+    if (size == -1) {
+      if (errno == EINTR) {
+        continue;
+      // TODO(oschaaf): should we worry about spinning here?
+      } else if (ngx_errno == EAGAIN || ngx_errno == EWOULDBLOCK) {
+        return true;
+      }
+    }
+
+    if (size <= 0) {
+      return false;
+    }
+
+    data.connection->event_handler_(data);
+    return true;
+  }
+}
+
+bool NgxEventConnection::WriteEvent(void* sender) {
+  return WriteEvent('X' /* Anything char is fine */, sender);
+}
+
+bool NgxEventConnection::WriteEvent(char type, void* sender) {
+  ssize_t size = 0;
+  ps_event_data data;
+
+  ngx_memzero(&data, sizeof(data));
+  data.type = type;
+  data.sender = sender;
+  data.connection = this;
+
+  while (true) {
+    size = write(pipe_write_fd_,
+                 static_cast<void*>(&data), sizeof(data));
+    if (size == sizeof(data)) {
+      return true;
+    } else if (size == -1) {
+      // TODO(oschaaf): should we worry about spinning here?
+      if (ngx_errno == EINTR || ngx_errno == EAGAIN
+          || ngx_errno == EWOULDBLOCK) {
+        continue;
+      } else {
+        return false;
+      }
+    } else {
+      CHECK(false) << "pagespeed: unexpected return value from write(): "
+                   << size;
+    }
+  }
+  CHECK(false) << "Should not get here";
+  return false;
+}
+
+// Reads and processes what is available in the pipe.
+void NgxEventConnection::Drain() {
+  NgxEventConnection::ReadAndNotify(pipe_read_fd_);
+}
+
+void NgxEventConnection::Shutdown() {
+  close(pipe_write_fd_);
+  close(pipe_read_fd_);
+}
+
+}  // namespace net_instaweb

src/ngx_event_connection.h

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Author: oschaaf@we-amp.com (Otto van der Schaaf)
+//
+// NgxEventConnection implements a means to send events from other threads to
+// nginx's event loop, and is implemented by a named pipe under the hood.
+// A single instance is used by NgxBaseFetch, and one instance is created per
+// NgxUrlAsyncFetcher when native fetching is on.
+
+#ifndef NGX_EVENT_CONNECTION_H_
+#define NGX_EVENT_CONNECTION_H_
+
+extern "C" {
+#include <ngx_http.h>
+}
+
+#include <pthread.h>
+
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/http/headers.h"
+
+namespace net_instaweb {
+
+class NgxEventConnection;
+
+// Represents a single event that can be written to or read from the pipe.
+// Technically, sender is the only data we need to send. type and connection are
+// included to provide a means to trace the events along with some more
+// info.
+typedef struct {
+  char type;
+  void* sender;
+  NgxEventConnection* connection;
+} ps_event_data;
+
+// Handler signature for receiving events
+typedef void (*callbackPtr)(const ps_event_data&);
+
+// Abstracts a connection to nginx through which events can be written.
+class NgxEventConnection {
+ public:
+  explicit NgxEventConnection(callbackPtr handler);
+
+  // Creates the file descriptors and ngx_connection_t required for event
+  // messaging between pagespeed and nginx.
+  bool Init(ngx_cycle_t* cycle);
+  // Shuts down the underlying file descriptors and connection created in Init()
+  void Shutdown();
+  // Constructs a ps_event_data and writes it to the underlying named pipe.
+  bool WriteEvent(char type, void* sender);
+  // Convenience overload for clients that have a single event type.
+  bool WriteEvent(void* sender);
+  // Reads and processes what is available in the named pipe's buffer.
+  void Drain();
+ private:
+  static bool CreateNgxConnection(ngx_cycle_t* cycle, ngx_fd_t pipe_fd);
+  static void ReadEventHandler(ngx_event_t* e);
+  static bool ReadAndNotify(ngx_fd_t fd);
+
+  callbackPtr event_handler_;
+  // We own these file descriptors
+  ngx_fd_t pipe_write_fd_;
+  ngx_fd_t pipe_read_fd_;
+
+  DISALLOW_COPY_AND_ASSIGN(NgxEventConnection);
+};
+
+}  // namespace net_instaweb
+
+#endif  // NGX_EVENT_CONNECTION_H_

src/ngx_fetch.cc

@@ -37,7 +37,7 @@ extern "C" {
 }
 
 #include "ngx_fetch.h"
-#include "net/instaweb/util/public/basictypes.h"
+
 #include "base/logging.h"
 
 #include <algorithm>
@@ -45,71 +45,28 @@ extern "C" {
 #include <typeinfo>
 #include <vector>
 
-#include "net/instaweb/util/public/scoped_ptr.h"
 #include "net/instaweb/http/public/async_fetch.h"
 #include "net/instaweb/http/public/inflating_fetch.h"
-#include "net/instaweb/http/public/request_headers.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/http/public/response_headers_parser.h"
 #include "net/instaweb/public/version.h"
 #include "net/instaweb/public/global_constants.h"
-#include "net/instaweb/util/public/condvar.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/pool_element.h"
-#include "net/instaweb/util/public/pthread_mutex.h"
-#include "net/instaweb/util/public/statistics.h"
-#include "net/instaweb/util/public/string_writer.h"
-#include "net/instaweb/util/public/string_util.h"
-#include "net/instaweb/util/public/thread_system.h"
-#include "net/instaweb/util/public/timer.h"
-#include "net/instaweb/util/public/writer.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/condvar.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/pool_element.h"
+#include "pagespeed/kernel/base/scoped_ptr.h"
+#include "pagespeed/kernel/base/statistics.h"
+#include "pagespeed/kernel/base/string_writer.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/base/thread_system.h"
+#include "pagespeed/kernel/base/timer.h"
+#include "pagespeed/kernel/base/writer.h"
+#include "pagespeed/kernel/http/request_headers.h"
+#include "pagespeed/kernel/http/response_headers.h"
+#include "pagespeed/kernel/http/response_headers_parser.h"
 
 namespace net_instaweb {
 
-class NgxConnection : public PoolElement<NgxConnection> {
- public:
-  NgxConnection(MessageHandler* handler, int max_keepalive_requests);
-  ~NgxConnection();
-  void SetSock(u_char *sockaddr, socklen_t socklen) {
-    socklen_ = socklen;
-    ngx_memcpy(&sockaddr_, sockaddr, socklen);
-  }
-  // Close ensures that NgxConnection deletes itself at the appropriate time,
-  // which can be after receiving a non-keepalive response, or when the remote
-  // server closes the connection when the NgxConnection is pooled and idle.
-  void Close();
-
-  // Once keepalive is disabled, it can't be toggled back on.
-  void set_keepalive(bool k) { keepalive_ = keepalive_ && k; }
-  bool keepalive() { return keepalive_; }
-
-  typedef Pool<NgxConnection> NgxConnectionPool;
-
-  static NgxConnection* Connect(ngx_peer_connection_t* pc,
-                                MessageHandler* handler,
-                                int max_keepalive_requests);
-  static void IdleWriteHandler(ngx_event_t* ev);
-  static void IdleReadHandler(ngx_event_t* ev);
-
-  static NgxConnectionPool connection_pool;
-  static PthreadMutex connection_pool_mutex;
-
-  // c_ is owned by NgxConnection and freed in ::Close()
-  ngx_connection_t* c_;
-  static const int64 keepalive_timeout_ms;
-  static const GoogleString ka_header;
-
- private:
-  int max_keepalive_requests_;
-  bool keepalive_;
-  socklen_t socklen_;
-  u_char sockaddr_[NGX_SOCKADDRLEN];
-  MessageHandler* handler_;
-
-  DISALLOW_COPY_AND_ASSIGN(NgxConnection);
-};
-
 NgxConnection::NgxConnectionPool NgxConnection::connection_pool;
 PthreadMutex NgxConnection::connection_pool_mutex;
 // Default keepalive 60s.
@@ -130,7 +87,18 @@ NgxConnection::NgxConnection(MessageHandler* handler,
 }
 
 NgxConnection::~NgxConnection() {
-  CHECK(c_ == NULL) << "NgxFetch: Underlying connection should be NULL";
+  CHECK(c_ == NULL) << "NgxConnection: Underlying connection should be NULL";
+}
+
+void NgxConnection::Terminate() {
+  for (NgxConnectionPool::iterator p = connection_pool.begin();
+       p != connection_pool.end(); ++p) {
+    NgxConnection* nc = *p;
+    ngx_close_connection(nc->c_);
+    nc->c_ = NULL;
+    delete nc;
+  }
+  connection_pool.Clear();
 }
 
 NgxConnection* NgxConnection::Connect(ngx_peer_connection_t* pc,
@@ -163,7 +131,7 @@ NgxConnection* NgxConnection::Connect(ngx_peer_connection_t* pc,
         connection_pool.Remove(nc);
 
         ngx_log_error(NGX_LOG_DEBUG, pc->log, 0,
-                      "NgxFetch: re-using connection %p (pool size: %l)\n",
+                      "NgxFetch: re-using connection %p (pool size: %l)",
                       nc, connection_pool.size());
         return nc;
       }
@@ -194,7 +162,7 @@ void NgxConnection::Close() {
         // out or has been closed remotely.
         connection_pool.Remove(this);
         ngx_log_error(NGX_LOG_DEBUG, c_->log, 0,
-                      "NgxFetch: removed connection %p (pool size: %l)\n",
+                      "NgxFetch: removed connection %p (pool size: %l)",
                       this, connection_pool.size());
         removed_from_pool = true;
         break;
@@ -241,7 +209,7 @@ void NgxConnection::Close() {
     connection_pool.Add(this);
     ngx_log_error(NGX_LOG_DEBUG, c_->log, 0,
                   "NgxFetch: Added connection %p (pool size: %l - "
-                  " max_keepalive_requests_ %d)\n",
+                  " max_keepalive_requests_ %d)",
                   this, connection_pool.size(), max_keepalive_requests_);
   }
 }
@@ -329,7 +297,7 @@ bool NgxFetch::Start(NgxUrlAsyncFetcher* fetcher) {
   fetcher_ = fetcher;
   bool ok = Init();
   if (ok) {
-    ngx_log_error(NGX_LOG_DEBUG, log_, 0, "NgxFetch %p: initialized\n",
+    ngx_log_error(NGX_LOG_DEBUG, log_, 0, "NgxFetch %p: initialized",
                   this);
   }  // else Init() will have emitted a reason
   return ok;
@@ -347,7 +315,9 @@ bool NgxFetch::Init() {
   }
 
   if (!ParseUrl()) {
-    message_handler_->Message(kError, "NgxFetch: ParseUrl() failed");
+    message_handler_->Message(kError, 
+                              "NgxFetch: ParseUrl() failed for [%s]:%s",
+                              str_url_.c_str(), url_.err);
     return false;
   }
 
@@ -414,7 +384,7 @@ bool NgxFetch::Init() {
       return false;
     } else {
       ngx_log_error(NGX_LOG_DEBUG, log_, 0,
-                    "NgxFetch %p: start resolve for: %s\n",
+                    "NgxFetch %p: start resolve for: %s",
                     this, s_ipaddress.c_str());
     }
 
@@ -447,10 +417,10 @@ const char* NgxFetch::str_url() {
   return str_url_.c_str();
 }
 
-// This function should be called only once. The only argument is success or
+// This function should be called only once. The only argument is sucess or
 // not.
 void NgxFetch::CallbackDone(bool success) {
-  ngx_log_error(NGX_LOG_DEBUG, log_, 0, "NgxFetch %p: CallbackDone: %s\n",
+  ngx_log_error(NGX_LOG_DEBUG, log_, 0, "NgxFetch %p: CallbackDone: %s",
                 this, success ? "OK":"FAIL");
 
   if (async_fetch_ == NULL) {
@@ -487,7 +457,7 @@ void NgxFetch::CallbackDone(bool success) {
         }
       }
       ngx_log_error(NGX_LOG_DEBUG, log_, 0,
-                    "NgxFetch %p: connection %p attempt keep-alive: %s\n",
+                    "NgxFetch %p: connection %p attempt keep-alive: %s",
                     this, connection_, keepalive ? "Yes":"No");
     }
 
@@ -496,9 +466,6 @@ void NgxFetch::CallbackDone(bool success) {
     connection_ = NULL;
   }
 
-  // TODO(oschaaf): see https://github.com/pagespeed/ngx_pagespeed/pull/755
-  async_fetch_->Done(success);
-
   if (fetcher_ != NULL) {
     if (fetcher_->track_original_content_length()
         && async_fetch_->response_headers()->Has(
@@ -508,7 +475,7 @@ void NgxFetch::CallbackDone(bool success) {
     }
     fetcher_->FetchComplete(this);
   }
-
+  async_fetch_->Done(success);
   async_fetch_ = NULL;
 }
 
@@ -739,7 +706,7 @@ int NgxFetch::Connect() {
   connection_ = NgxConnection::Connect(&pc, message_handler(),
                                        fetcher_->max_keepalive_requests_);
   ngx_log_error(NGX_LOG_DEBUG, fetcher_->log_, 0,
-                "NgxFetch %p Connect() connection %p for [%s]\n",
+                "NgxFetch %p Connect() connection %p for [%s]",
                 this, connection_, str_url());
 
   if (connection_ == NULL) {
@@ -755,13 +722,13 @@ int NgxFetch::Connect() {
 }
 
 // When the fetch sends the request completely, it will hook the read event,
-// and prepare to parse the response.
+// and prepare to parse the response. Timer set in Init() is still in effect.
 void NgxFetch::ConnectionWriteHandler(ngx_event_t* wev) {
   ngx_connection_t* c = static_cast<ngx_connection_t*>(wev->data);
   NgxFetch* fetch = static_cast<NgxFetch*>(c->data);
   ngx_buf_t* out = fetch->out_;
-
-  while (out->pos < out->last) {
+  bool ok = true;
+  while (wev->ready && out->pos < out->last) {
     int n = c->send(c, out->pos, out->last - out->pos);
     ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
                   "NgxFetch %p: ConnectionWriteHandler "
@@ -770,31 +737,36 @@ void NgxFetch::ConnectionWriteHandler(ngx_event_t* wev) {
     if (n >= 0) {
       out->pos += n;
     } else if (n == NGX_AGAIN) {
-      if (ngx_handle_write_event(c->write, 0) != NGX_OK) {
-        fetch->CallbackDone(false);
-      }
-      // Timer set in Init() is still in effect.
-      return;
+      break;
     } else {
-      c->error = 1;
-      fetch->CallbackDone(false);
-      return;
+      ok = false;
+      break;
     }
   }
 
-  if (ngx_handle_read_event(c->read, 0) != NGX_OK) {
+  if (ok) {
+    if (out->pos == out->last) {
+      ok = ngx_handle_read_event(c->read, 0) == NGX_OK;
+    } else {
+      ok = ngx_handle_write_event(c->write, 0) == NGX_OK;
+    }
+  }
+
+  if (!ok) {
+    fetch->message_handler()->Message(
+        kWarning, "NgxFetch %p: failed to hook next event", fetch);
     c->error = 1;
     fetch->CallbackDone(false);
   }
-
-  return;
 }
 
+// Timer set in Init() is still in effect.
 void NgxFetch::ConnectionReadHandler(ngx_event_t* rev) {
   ngx_connection_t* c = static_cast<ngx_connection_t*>(rev->data);
   NgxFetch* fetch = static_cast<NgxFetch*>(c->data);
+  bool ok = true;
 
-  for (;;) {
+  while(rev->ready) {
     int n = c->recv(
         c, fetch->in_->start, fetch->in_->end - fetch->in_->start);
 
@@ -804,56 +776,39 @@ void NgxFetch::ConnectionReadHandler(ngx_event_t* rev) {
 
     if (n == NGX_AGAIN) {
       break;
-    }
-
-    if (n == 0) {
+    } else if (n == 0) {
       // If the content length was not known, we assume that we have read
       // all if we at least parsed the headers.
       // If we do know the content length, having a mismatch on the bytes read
       // will be interpreted as an error.
-      if (fetch->content_length_known_) {
-        fetch->CallbackDone(fetch->content_length_ == fetch->bytes_received_);
-      } else {
-        fetch->CallbackDone(fetch->parser_.headers_complete());
-      }
-
-      return;
+      ok = (fetch->content_length_known_ && fetch->content_length_ == fetch->bytes_received_)
+          || fetch->parser_.headers_complete();
+      fetch->done_ = true;
+      break;
     } else if (n > 0) {
       fetch->in_->pos = fetch->in_->start;
       fetch->in_->last = fetch->in_->start + n;
-      if (!fetch->response_handler(c)) {
-        fetch->CallbackDone(false);
-        return;
+      ok = fetch->response_handler(c);
+      if (fetch->done_ || !ok) {
+        break;
       }
-
-      if (fetch->done_) {
-        fetch->CallbackDone(true);
-        return;
-      }
-    }
-
-    if (!rev->ready) {
-      break;
     }
   }
 
-  if (fetch->done_) {
+  if (!ok) {
+    fetch->CallbackDone(false);
+  } else if (fetch->done_) {
     fetch->CallbackDone(true);
-    return;
-  }
-
-  if (ngx_handle_read_event(rev, 0) != NGX_OK) {
+  } else if (ngx_handle_read_event(rev, 0) != NGX_OK) {
     fetch->CallbackDone(false);
   }
-
-  // Timer set in Init() is still in effect.
 }
 
 // Parse the status line: "HTTP/1.1 200 OK\r\n"
 bool NgxFetch::HandleStatusLine(ngx_connection_t* c) {
   NgxFetch* fetch = static_cast<NgxFetch*>(c->data);
   ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-                "NgxFetch %p: Handle status line\n", fetch);
+                "NgxFetch %p: Handle status line", fetch);
 
   // This function only works after Nginx-1.1.4. Before nginx-1.1.4,
   // ngx_http_parse_status_line didn't save http_version.
@@ -866,14 +821,20 @@ bool NgxFetch::HandleStatusLine(ngx_connection_t* c) {
   } else if (n == NGX_AGAIN) {  // not completed
     return true;
   }
+
   ResponseHeaders* response_headers =
       fetch->async_fetch_->response_headers();
   response_headers->SetStatusAndReason(
       static_cast<HttpStatus::Code>(fetch->get_status_code()));
   response_headers->set_major_version(fetch->get_major_version());
   response_headers->set_minor_version(fetch->get_minor_version());
+
+  fetch->in_->pos += n;
   fetch->set_response_handler(NgxFetch::HandleHeader);
-  return fetch->response_handler(c);
+  if ((fetch->in_->last - fetch->in_->pos) > 0) {
+    return fetch->response_handler(c);
+  }
+  return true;
 }
 
 // Parse the HTTP headers
@@ -885,12 +846,16 @@ bool NgxFetch::HandleHeader(ngx_connection_t* c) {
                                        fetch->message_handler_);
 
   ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-                "NgxFetch %p: Handle headers\n", fetch);
+                "NgxFetch %p: Handle headers", fetch);
 
   if (n > size) {
     return false;
   } else if (fetch->parser_.headers_complete()) {
-    if (fetch->async_fetch_->response_headers()->FindContentLength(
+    // TODO(oschaaf): We should also check if the request method was HEAD
+    // - but I don't think PSOL uses that at this point.
+    if (fetch->get_status_code() == 304 || fetch->get_status_code() == 204) {
+      fetch->done_ = true;
+    } else if (fetch->async_fetch_->response_headers()->FindContentLength(
             &fetch->content_length_)) {
       if (fetch->content_length_ < 0) {
         fetch->message_handler_->Message(
@@ -911,9 +876,11 @@ bool NgxFetch::HandleHeader(ngx_connection_t* c) {
     }
 
     fetch->in_->pos += n;
-    fetch->set_response_handler(NgxFetch::HandleBody);
-    if ((fetch->in_->last - fetch->in_->pos) > 0) {
-      return fetch->response_handler(c);
+    if (!fetch->done_) {
+      fetch->set_response_handler(NgxFetch::HandleBody);
+      if ((fetch->in_->last - fetch->in_->pos) > 0) {
+        return fetch->response_handler(c);
+      }
     }
   } else {
     fetch->in_->pos += n;
@@ -930,7 +897,7 @@ bool NgxFetch::HandleBody(ngx_connection_t* c) {
   fetch->bytes_received_add(size);
 
   ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-                "NgxFetch %p: Handle body (%d bytes)\n", fetch, size);
+                "NgxFetch %p: Handle body (%d bytes)", fetch, size);
 
   if ( fetch->async_fetch_->Write(StringPiece(data, size),
                                   fetch->message_handler()) ) {
@@ -940,7 +907,7 @@ bool NgxFetch::HandleBody(ngx_connection_t* c) {
     fetch->in_->pos += size;
   } else {
     ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-                  "NgxFetch %p: async fetch write failure\n", fetch);
+                  "NgxFetch %p: async fetch write failure", fetch);
     return false;
   }
   return true;
@@ -949,7 +916,7 @@ bool NgxFetch::HandleBody(ngx_connection_t* c) {
 void NgxFetch::TimeoutHandler(ngx_event_t* tev) {
   NgxFetch* fetch = static_cast<NgxFetch*>(tev->data);
   ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-                "NgxFetch %p: TimeoutHandler called\n", fetch);
+                "NgxFetch %p: TimeoutHandler called", fetch);
   fetch->CallbackDone(false);
 }
 
@@ -973,8 +940,8 @@ void NgxFetch::FixUserAgent() {
     user_agent += "NgxNativeFetcher";
   }
   GoogleString version = StrCat(
-      " ", kModPagespeedSubrequestUserAgent,
-      "/" MOD_PAGESPEED_VERSION_STRING "-" LASTCHANGE_STRING);
+      " (", kModPagespeedSubrequestUserAgent,
+      "/" MOD_PAGESPEED_VERSION_STRING "-" LASTCHANGE_STRING ")");
   if (!StringPiece(user_agent).ends_with(version)) {
     user_agent += version;
   }

src/ngx_fetch.h

@@ -39,12 +39,14 @@ extern "C" {
 
 #include "ngx_url_async_fetcher.h"
 #include <vector>
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/string.h"
 #include "net/instaweb/http/public/url_async_fetcher.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/http/public/response_headers_parser.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/http/response_headers.h"
+#include "pagespeed/kernel/http/response_headers_parser.h"
+#include "pagespeed/kernel/thread/pthread_mutex.h"
+
 
 namespace net_instaweb {
 
@@ -53,6 +55,51 @@ typedef bool (*response_handler_pt)(ngx_connection_t* c);
 class NgxUrlAsyncFetcher;
 class NgxConnection;
 
+class NgxConnection : public PoolElement<NgxConnection> {
+ public:
+  NgxConnection(MessageHandler* handler, int max_keepalive_requests);
+  ~NgxConnection();
+  void SetSock(u_char *sockaddr, socklen_t socklen) {
+    socklen_ = socklen;
+    ngx_memcpy(&sockaddr_, sockaddr, socklen);
+  }
+  // Close ensures that NgxConnection deletes itself at the appropriate time,
+  // which can be after receiving a non-keepalive response, or when the remote
+  // server closes the connection when the NgxConnection is pooled and idle.
+  void Close();
+
+  // Once keepalive is disabled, it can't be toggled back on.
+  void set_keepalive(bool k) { keepalive_ = keepalive_ && k; }
+  bool keepalive() { return keepalive_; }
+
+  typedef Pool<NgxConnection> NgxConnectionPool;
+
+  static NgxConnection* Connect(ngx_peer_connection_t* pc,
+                                MessageHandler* handler,
+                                int max_keepalive_requests);
+  static void IdleWriteHandler(ngx_event_t* ev);
+  static void IdleReadHandler(ngx_event_t* ev);
+  // Terminate will cleanup any idle connections upon shutdown.
+  static void Terminate();
+
+  static NgxConnectionPool connection_pool;
+  static PthreadMutex connection_pool_mutex;
+
+  // c_ is owned by NgxConnection and freed in ::Close()
+  ngx_connection_t* c_;
+  static const int64 keepalive_timeout_ms;
+  static const GoogleString ka_header;
+
+ private:
+  int max_keepalive_requests_;
+  bool keepalive_;
+  socklen_t socklen_;
+  u_char sockaddr_[NGX_SOCKADDRLEN];
+  MessageHandler* handler_;
+
+  DISALLOW_COPY_AND_ASSIGN(NgxConnection);
+};
+
 class NgxFetch : public PoolElement<NgxFetch> {
  public:
   NgxFetch(const GoogleString& url,

src/ngx_gzip_setter.cc

@@ -70,7 +70,7 @@ extern "C" {
   }
 }
 
-NgxGZipSetter::NgxGZipSetter() : enabled_(false), initialized_(false) { }
+NgxGZipSetter::NgxGZipSetter() : enabled_(0) { }
 NgxGZipSetter::~NgxGZipSetter() { }
 
 // Helper functions to determine signature.
@@ -95,7 +95,7 @@ bool IsNgxBitmaskCommand(ngx_command_t* command) {
           HasLocalConfig(command));
 }
 
-// Initialize the NgxGZipSetter.
+// Initialize the NgxGzipSetter.
 // Find the gzip, gzip_vary, gzip_http_version and gzip_types commands in the
 // gzip module. Enable if the signature of the zip command matches with what we
 // trust. Also sets up redirects for the configurations. These redirect handle
@@ -105,16 +105,6 @@ void NgxGZipSetter::Init(ngx_conf_t* cf) {
 #if (NGX_HTTP_GZIP)
   bool gzip_signature_mismatch = false;
   bool other_signature_mismatch = false;
-  // If we initialized already we don't have to scan again.
-  if (initialized_) {
-    // Config might have changed, so re-enable if we have gzip.
-    if (gzip_command_.command_ != NULL) {
-      enabled_ = true;
-    } else {
-      enabled_ = false;
-    }
-    return;
-  } 
   for (int m = 0; ngx_modules[m] != NULL; m++) {
     if (ngx_modules[m]->commands != NULL) {
       for (int c = 0; ngx_modules[m]->commands[c].name.len; c++) {
@@ -132,7 +122,7 @@ void NgxGZipSetter::Init(ngx_conf_t* cf) {
             current_command->set = ngx_gzip_redirect_conf_set_flag_slot;
             gzip_command_.command_ = current_command;
             gzip_command_.module_ = ngx_modules[m];
-            enabled_ = true;
+            enabled_ = 1;
           } else {
             ngx_conf_log_error(
                 NGX_LOG_WARN, cf, 0,
@@ -199,7 +189,6 @@ void NgxGZipSetter::Init(ngx_conf_t* cf) {
       }
     }
   }
-  initialized_ = true;
   if (gzip_signature_mismatch) {
     return;  // Already logged error.
   } else if (!enabled_) {
@@ -392,7 +381,7 @@ void NgxGZipSetter::AddGZipHTTPTypes(ngx_conf_t* cf) {
 }
 
 void NgxGZipSetter::RollBackAndDisable(ngx_conf_t* cf) {
-  ngx_conf_log_error(NGX_LOG_DEBUG, cf, 0,
+  ngx_conf_log_error(NGX_LOG_INFO, cf, 0,
                      "pagespeed: rollback gzip, explicit configuration");
   for (std::vector<ngx_flag_t*>::iterator i = ngx_flags_set_.begin();
        i != ngx_flags_set_.end(); ++i) {

src/ngx_gzip_setter.h

@@ -60,7 +60,7 @@ extern "C" {
 
 #include "ngx_pagespeed.h"
 
-#include "net/instaweb/util/public/basictypes.h"
+#include "pagespeed/kernel/base/basictypes.h"
 
 namespace net_instaweb {
 
@@ -91,7 +91,6 @@ class NgxGZipSetter {
   ngx_command_ctx gzip_vary_command_;
   ngx_command_ctx gzip_http_version_command_;
   bool enabled_;
-  bool initialized_;
 
  public:
   NgxGZipSetter();

src/ngx_message_handler.cc

@@ -18,13 +18,13 @@
 
 #include <signal.h>
 
-#include "net/instaweb/util/public/abstract_mutex.h"
-#include "net/instaweb/util/public/debug.h"
-#include "net/instaweb/util/public/shared_circular_buffer.h"
-#include "net/instaweb/util/public/string_util.h"
 #include "net/instaweb/public/version.h"
+#include "pagespeed/kernel/base/abstract_mutex.h"
+#include "pagespeed/kernel/base/debug.h"
 #include "pagespeed/kernel/base/posix_timer.h"
+#include "pagespeed/kernel/base/string_util.h"
 #include "pagespeed/kernel/base/time_util.h"
+#include "pagespeed/kernel/sharedmem/shared_circular_buffer.h"
 
 namespace {
 
@@ -88,31 +88,28 @@ ngx_uint_t NgxMessageHandler::GetNgxLogLevel(MessageType type) {
   return NGX_LOG_ALERT;
 }
 
-void NgxMessageHandler::MessageVImpl(MessageType type, const char* msg,
-                                     va_list args) {
-  ngx_uint_t log_level = GetNgxLogLevel(type);
-  GoogleString formatted_message = Format(msg, args);
+void NgxMessageHandler::MessageSImpl(MessageType type,
+                                     const GoogleString& message) {
   if (log_ != NULL) {
+    ngx_uint_t log_level = GetNgxLogLevel(type);
     ngx_log_error(log_level, log_, 0/*ngx_err_t*/, "[%s %s] %s",
-                  kModuleName, kModPagespeedVersion, formatted_message.c_str());
+                  kModuleName, kModPagespeedVersion, message.c_str());
   } else {
-    GoogleMessageHandler::MessageVImpl(type, msg, args);
+    GoogleMessageHandler::MessageSImpl(type, message);
   }
   // Prepare a log message for the SharedCircularBuffer only.
-  AddMessageToBuffer(type, formatted_message);
+  AddMessageToBuffer(type, message);
 }
 
-void NgxMessageHandler::FileMessageVImpl(MessageType type, const char* file,
-                                         int line, const char* msg,
-                                         va_list args) {
-  ngx_uint_t log_level = GetNgxLogLevel(type);
-  GoogleString formatted_message = Format(msg, args);
+void NgxMessageHandler::FileMessageSImpl(
+    MessageType type, const char* file, int line, const GoogleString& message) {
   if (log_ != NULL) {
+    ngx_uint_t log_level = GetNgxLogLevel(type);
     ngx_log_error(log_level, log_, 0/*ngx_err_t*/, "[%s %s] %s:%d:%s",
                   kModuleName, kModPagespeedVersion, file, line,
-                  formatted_message.c_str());
+                  message.c_str());
   } else {
-    GoogleMessageHandler::FileMessageVImpl(type, file, line, msg, args);
+    GoogleMessageHandler::FileMessageSImpl(type, file, line, message);
   }
 }
 

src/ngx_message_handler.h

@@ -28,11 +28,11 @@ extern "C" {
 
 #include <cstdarg>
 
-#include "net/instaweb/system/public/system_message_handler.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/system/system_message_handler.h"
 
 namespace net_instaweb {
 
@@ -53,10 +53,10 @@ class NgxMessageHandler : public SystemMessageHandler {
   ngx_log_t* log() { return log_; }
 
  protected:
-  virtual void MessageVImpl(MessageType type, const char* msg, va_list args);
+  virtual void MessageSImpl(MessageType type, const GoogleString& message);
 
-  virtual void FileMessageVImpl(MessageType type, const char* filename,
-                                int line, const char* msg, va_list args);
+  virtual void FileMessageSImpl(MessageType type, const char* file,
+                                int line, const GoogleString& message);
 
  private:
   ngx_uint_t GetNgxLogLevel(MessageType type);

src/ngx_pagespeed.h

@@ -34,8 +34,8 @@ extern "C" {
 }
 
 #include "base/logging.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/http/response_headers.h"
 
 namespace net_instaweb {
 
@@ -84,15 +84,11 @@ enum PreserveCachingHeaders {
 typedef struct {
   NgxBaseFetch* base_fetch;
 
-  ngx_connection_t* pagespeed_connection;
   ngx_http_request_t* r;
 
   bool html_rewrite;
   bool in_place;
 
-  bool write_pending;
-  bool fetch_done;
-
   PreserveCachingHeaders preserve_caching_headers;
 
   // for html rewrite
@@ -107,8 +103,15 @@ typedef struct {
   // We need to remember the URL here as well since we may modify what NGX
   // gets by stripping our special query params and honoring X-Forwarded-Proto.
   GoogleString url_string;
+
+  // We need to remember if the upstream had headers_out->location set, because
+  // 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);
 
 void copy_request_headers_from_ngx(const ngx_http_request_t* r,
                                    RequestHeaders* headers);
@@ -123,6 +126,12 @@ ngx_int_t copy_response_headers_to_ngx(
 
 StringPiece ps_determine_host(ngx_http_request_t* r);
 
+namespace ps_base_fetch {
+
+ngx_int_t ps_base_fetch_handler(ngx_http_request_t* r);
+
+}  // namespace ps_base_fetch
+
 }  // namespace net_instaweb
 
 #endif  // NGX_PAGESPEED_H_

src/ngx_rewrite_driver_factory.cc

@@ -26,30 +26,30 @@
 #include "ngx_server_context.h"
 #include "ngx_url_async_fetcher.h"
 
-#include "net/instaweb/http/public/content_type.h"
 #include "net/instaweb/http/public/rate_controller.h"
 #include "net/instaweb/http/public/rate_controlling_url_async_fetcher.h"
 #include "net/instaweb/http/public/wget_url_fetcher.h"
 #include "net/instaweb/rewriter/public/rewrite_driver.h"
 #include "net/instaweb/rewriter/public/rewrite_driver_factory.h"
 #include "net/instaweb/rewriter/public/server_context.h"
-#include "net/instaweb/system/public/in_place_resource_recorder.h"
-#include "net/instaweb/system/public/serf_url_async_fetcher.h"
-#include "net/instaweb/system/public/system_caches.h"
-#include "net/instaweb/system/public/system_rewrite_options.h"
-#include "net/instaweb/util/public/google_message_handler.h"
-#include "net/instaweb/util/public/null_shared_mem.h"
-#include "net/instaweb/util/public/posix_timer.h"
 #include "net/instaweb/util/public/property_cache.h"
-#include "net/instaweb/util/public/scheduler_thread.h"
-#include "net/instaweb/util/public/shared_circular_buffer.h"
-#include "net/instaweb/util/public/shared_mem_statistics.h"
-#include "net/instaweb/util/public/slow_worker.h"
-#include "net/instaweb/util/public/stdio_file_system.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/string_util.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/google_message_handler.h"
+#include "pagespeed/kernel/base/null_shared_mem.h"
+#include "pagespeed/kernel/base/posix_timer.h"
+#include "pagespeed/kernel/base/stdio_file_system.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/base/thread_system.h"
+#include "pagespeed/kernel/http/content_type.h"
+#include "pagespeed/kernel/sharedmem/shared_circular_buffer.h"
+#include "pagespeed/kernel/sharedmem/shared_mem_statistics.h"
 #include "pagespeed/kernel/thread/pthread_shared_mem.h"
+#include "pagespeed/kernel/thread/scheduler_thread.h"
+#include "pagespeed/kernel/thread/slow_worker.h"
+#include "pagespeed/system/in_place_resource_recorder.h"
+#include "pagespeed/system/serf_url_async_fetcher.h"
+#include "pagespeed/system/system_caches.h"
+#include "pagespeed/system/system_rewrite_options.h"
 
 namespace net_instaweb {
 
@@ -69,7 +69,6 @@ NgxRewriteDriverFactory::NgxRewriteDriverFactory(
     SystemThreadSystem* system_thread_system, StringPiece hostname, int port)
     : SystemRewriteDriverFactory(process_context, system_thread_system,
         NULL /* default shared memory runtime */, hostname, port),
-      main_conf_(NULL),
       threads_started_(false),
       ngx_message_handler_(
           new NgxMessageHandler(timer(), thread_system()->NewMutex())),
@@ -84,7 +83,8 @@ NgxRewriteDriverFactory::NgxRewriteDriverFactory(
       hostname_(hostname.as_string()),
       port_(port),
       process_script_variables_(false),
-      process_script_variables_set_(false) {
+      process_script_variables_set_(false),
+      shut_down_(false) {
   InitializeDefaultOptions();
   default_options()->set_beacon_url("/ngx_pagespeed_beacon");
   SystemRewriteOptions* system_options = dynamic_cast<SystemRewriteOptions*>(
@@ -147,18 +147,17 @@ NamedLockManager* NgxRewriteDriverFactory::DefaultLockManager() {
 
 RewriteOptions* NgxRewriteDriverFactory::NewRewriteOptions() {
   NgxRewriteOptions* options = new NgxRewriteOptions(thread_system());
+  // TODO(jefftk): figure out why using SetDefaultRewriteLevel like
+  // mod_pagespeed does in mod_instaweb.cc:create_dir_config() isn't enough here
+  // -- if you use that instead then ngx_pagespeed doesn't actually end up
+  // defaulting CoreFilters.
+  // See: https://github.com/pagespeed/ngx_pagespeed/issues/1190
   options->SetRewriteLevel(RewriteOptions::kCoreFilters);
   return options;
 }
 
-bool NgxRewriteDriverFactory::InitNgxUrlAsyncFetchers() {
-  log_ = ngx_cycle->log;
-  for (size_t i = 0; i < ngx_url_async_fetchers_.size(); ++i) {
-    if (!ngx_url_async_fetchers_[i]->Init()) {
-      return false;
-    }
-  }
-  return true;
+RewriteOptions* NgxRewriteDriverFactory::NewRewriteOptionsForQuery() {
+  return new NgxRewriteOptions(thread_system());
 }
 
 bool NgxRewriteDriverFactory::CheckResolver() {
@@ -186,6 +185,13 @@ ServerContext* NgxRewriteDriverFactory::NewServerContext() {
   return NULL;
 }
 
+void NgxRewriteDriverFactory::ShutDown() {
+  if (!shut_down_) {
+    shut_down_ = true;
+    SystemRewriteDriverFactory::ShutDown();
+  }
+}
+
 void NgxRewriteDriverFactory::ShutDownMessageHandlers() {
   ngx_message_handler_->set_buffer(NULL);
   ngx_html_parse_message_handler_->set_buffer(NULL);
@@ -210,9 +216,19 @@ void NgxRewriteDriverFactory::StartThreads() {
   threads_started_ = true;
 }
 
-void NgxRewriteDriverFactory::LoggingInit(ngx_log_t* log) {
+void NgxRewriteDriverFactory::SetMainConf(NgxRewriteOptions* main_options) {
+  // Propagate process-scope options from the copy we had during nginx option
+  // parsing to our own.
+  if (main_options != NULL) {
+    default_options()->MergeOnlyProcessScopeOptions(*main_options);
+  }
+}
+
+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

@@ -32,13 +32,9 @@ extern "C" {
 
 #include <set>
 
-#include "net/instaweb/system/public/system_rewrite_driver_factory.h"
-#include "net/instaweb/util/public/md5_hasher.h"
-#include "net/instaweb/util/public/scoped_ptr.h"
-
-// TODO(oschaaf): We should reparent ApacheRewriteDriverFactory and
-// NgxRewriteDriverFactory to a new class OriginRewriteDriverFactory and factor
-// out as much as possible.
+#include "pagespeed/kernel/base/md5_hasher.h"
+#include "pagespeed/kernel/base/scoped_ptr.h"
+#include "pagespeed/system/system_rewrite_driver_factory.h"
 
 namespace net_instaweb {
 
@@ -67,10 +63,10 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   virtual Timer* DefaultTimer();
   virtual NamedLockManager* DefaultLockManager();
   // Create a new RewriteOptions.  In this implementation it will be an
-  // NgxRewriteOptions.
+  // NgxRewriteOptions, and it will have CoreFilters explicitly set.
   virtual RewriteOptions* NewRewriteOptions();
+  virtual RewriteOptions* NewRewriteOptionsForQuery();
   virtual ServerContext* NewDecodingServerContext();
-  bool InitNgxUrlAsyncFetchers();
   // Check resolver configured or not.
   bool CheckResolver();
 
@@ -80,6 +76,7 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   static void InitStats(Statistics* statistics);
   NgxServerContext* MakeNgxServerContext(StringPiece hostname, int port);
   virtual ServerContext* NewServerContext();
+  virtual void ShutDown();
 
   // Starts pagespeed threads if they've not been started already.  Must be
   // called after the caller has finished any forking it intends to do.
@@ -94,7 +91,7 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
     InitStats(statistics);
   }
 
-  void set_main_conf(NgxRewriteOptions* main_conf) {  main_conf_ = main_conf; }
+  void SetMainConf(NgxRewriteOptions* main_conf);
 
   void set_resolver(ngx_resolver_t* resolver) {
     resolver_ = resolver;
@@ -119,7 +116,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();
 
@@ -137,10 +134,6 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
  private:
   Timer* timer_;
 
-  // main_conf will have only options set in the main block.  It may be NULL,
-  // and we do not take ownership.
-  NgxRewriteOptions* main_conf_;
-
   bool threads_started_;
   NgxMessageHandler* ngx_message_handler_;
   NgxMessageHandler* ngx_html_parse_message_handler_;
@@ -163,6 +156,7 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   int port_;
   bool process_script_variables_;
   bool process_script_variables_set_;
+  bool shut_down_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxRewriteDriverFactory);
 };

src/ngx_rewrite_options.cc

@@ -30,9 +30,9 @@ extern "C" {
 #include "net/instaweb/public/version.h"
 #include "net/instaweb/rewriter/public/file_load_policy.h"
 #include "net/instaweb/rewriter/public/rewrite_options.h"
-#include "net/instaweb/system/public/system_caches.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/timer.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/timer.h"
+#include "pagespeed/system/system_caches.h"
 
 namespace net_instaweb {
 
@@ -276,10 +276,16 @@ const char* NgxRewriteOptions::ParseAndSetOptions(
 
   ScriptLine* script_line;
   script_line = NULL;
-  // Only allow script variable support for LoadFromFile for now.
   // Note that LoadFromFile should not be scriptable on wildcard hosts,
   // as browsers might be able to manipulate its natural use-case: $http_host.
-  if (!StringCaseStartsWith(directive, "LoadFromFile")) {
+  if (!StringCaseStartsWith(directive, "LoadFromFile") &&
+      !StringCaseEqual(directive, "EnableFilters") &&
+      !StringCaseEqual(directive, "DisableFilters") &&
+      !StringCaseEqual(directive, "DownstreamCachePurgeLocationPrefix") &&
+      !StringCaseEqual(directive, "DownstreamCachePurgeMethod") &&
+      !StringCaseEqual(directive,
+                       "DownstreamCacheRewrittenPercentageThreshold") &&
+      !StringCaseEqual(directive, "ShardDomain")){
     compile_scripts = false;
   }
 
@@ -510,10 +516,6 @@ NgxRewriteOptions* NgxRewriteOptions::Clone() const {
   return options;
 }
 
-void NgxRewriteOptions::Merge(const RewriteOptions& src) {
-  SystemRewriteOptions::Merge(src);
-}
-
 const NgxRewriteOptions* NgxRewriteOptions::DynamicCast(
     const RewriteOptions* instance) {
   return dynamic_cast<const NgxRewriteOptions*>(instance);

src/ngx_rewrite_options.h

@@ -29,11 +29,11 @@ extern "C" {
 
 #include <vector>
 
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/ref_counted_ptr.h"
-#include "net/instaweb/util/public/stl_util.h"          // for STLDeleteElements
 #include "net/instaweb/rewriter/public/rewrite_options.h"
-#include "net/instaweb/system/public/system_rewrite_options.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/ref_counted_ptr.h"
+#include "pagespeed/kernel/base/stl_util.h"          // for STLDeleteElements
+#include "pagespeed/system/system_rewrite_options.h"
 
 #define NGX_PAGESPEED_MAX_ARGS 10
 
@@ -140,7 +140,6 @@ class NgxRewriteOptions : public SystemRewriteOptions {
 
   // Make an identical copy of these options and return it.
   virtual NgxRewriteOptions* Clone() const;
-  virtual void Merge(const RewriteOptions& src);
 
   // Returns a suitably down cast version of 'instance' if it is an instance
   // of this class, NULL if not.

src/ngx_server_context.cc

@@ -27,9 +27,9 @@ extern "C" {
 #include "ngx_rewrite_driver_factory.h"
 #include "ngx_rewrite_options.h"
 #include "net/instaweb/rewriter/public/rewrite_driver.h"
-#include "net/instaweb/system/public/add_headers_fetcher.h"
-#include "net/instaweb/system/public/loopback_route_fetcher.h"
-#include "net/instaweb/system/public/system_request_context.h"
+#include "pagespeed/system/add_headers_fetcher.h"
+#include "pagespeed/system/loopback_route_fetcher.h"
+#include "pagespeed/system/system_request_context.h"
 
 namespace net_instaweb {
 

src/ngx_server_context.h

@@ -22,7 +22,7 @@
 #define NGX_SERVER_CONTEXT_H_
 
 #include "ngx_message_handler.h"
-#include "net/instaweb/system/public/system_server_context.h"
+#include "pagespeed/system/system_server_context.h"
 
 extern "C" {
 #include <ngx_http.h>
@@ -40,8 +40,9 @@ class NgxServerContext : public SystemServerContext {
       NgxRewriteDriverFactory* factory, StringPiece hostname, int port);
   virtual ~NgxServerContext();
 
-  // We expect to use ProxyFetch with HTML.
-  virtual bool ProxiesHtml() const { return true; }
+  // We don't allow ProxyFetch to fetch HTML via MapProxyDomain. We will call
+  // set_trusted_input() on any ProxyFetches we use to transform internal HTML.
+  virtual bool ProxiesHtml() const { return false; }
 
   // Call only when you need an NgxRewriteOptions.  If you don't need
   // nginx-specific behavior, call global_options() instead which doesn't

src/ngx_url_async_fetcher.cc

@@ -31,22 +31,22 @@ extern "C" {
 #include <map>
 #include <set>
 
-#include "net/instaweb/util/public/basictypes.h"
 #include "net/instaweb/http/public/async_fetch.h"
 #include "net/instaweb/http/public/inflating_fetch.h"
-#include "net/instaweb/http/public/request_headers.h"
-#include "net/instaweb/http/public/response_headers.h"
-#include "net/instaweb/http/public/response_headers_parser.h"
 #include "net/instaweb/public/version.h"
-#include "net/instaweb/util/public/condvar.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/pool_element.h"
-#include "net/instaweb/util/public/statistics.h"
-#include "net/instaweb/util/public/string_util.h"
-#include "net/instaweb/util/public/thread_system.h"
-#include "net/instaweb/util/public/timer.h"
-#include "net/instaweb/util/public/writer.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/condvar.h"
+#include "pagespeed/kernel/base/message_handler.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/pool_element.h"
+#include "pagespeed/kernel/base/statistics.h"
+#include "pagespeed/kernel/base/string_util.h"
+#include "pagespeed/kernel/base/thread_system.h"
+#include "pagespeed/kernel/base/timer.h"
+#include "pagespeed/kernel/base/writer.h"
+#include "pagespeed/kernel/http/request_headers.h"
+#include "pagespeed/kernel/http/response_headers.h"
+#include "pagespeed/kernel/http/response_headers_parser.h"
 
 namespace net_instaweb {
 
@@ -65,7 +65,8 @@ namespace net_instaweb {
       thread_system_(thread_system),
       message_handler_(handler),
       mutex_(NULL),
-      max_keepalive_requests_(max_keepalive_requests) {
+      max_keepalive_requests_(max_keepalive_requests),
+      event_connection_(NULL) {
     resolver_timeout_ = resolver_timeout;
     fetch_timeout_ = fetch_timeout;
     ngx_memzero(&proxy_, sizeof(proxy_));
@@ -76,12 +77,17 @@ namespace net_instaweb {
     mutex_ = thread_system_->NewMutex();
     log_ = log;
     pool_ = NULL;
-    command_connection_ = NULL;
-    pipe_fd_ = -1;
     resolver_ = resolver;
+    // If init fails, set shutdown_ so no fetches will be attempted.
+    if (!Init(const_cast<ngx_cycle_t*>(ngx_cycle))) {
+      shutdown_ = true;
+      message_handler_->Message(
+          kError, "NgxUrlAsyncFetcher failed to init, fetching disabled.");
+    }
   }
 
   NgxUrlAsyncFetcher::~NgxUrlAsyncFetcher() {
+    DCHECK(shutdown_)  << "Shut down before destructing NgxUrlAsyncFetcher.";
     message_handler_->Message(
         kInfo,
         "Destruct NgxUrlAsyncFetcher with [%d] active fetchers",
@@ -89,19 +95,12 @@ namespace net_instaweb {
 
     CancelActiveFetches();
     active_fetches_.DeleteAll();
+    NgxConnection::Terminate();
 
     if (pool_ != NULL) {
       ngx_destroy_pool(pool_);
       pool_ = NULL;
     }
-    if (command_connection_ != NULL) {
-      ngx_close_connection(command_connection_);
-      command_connection_ = NULL;
-    }
-    if (pipe_fd_ != -1) {
-      close(pipe_fd_);
-      pipe_fd_ = -1;
-    }
     if (mutex_ != NULL) {
       delete mutex_;
       mutex_ = NULL;
@@ -152,9 +151,13 @@ namespace net_instaweb {
 
   // Create the pool for fetcher, create the pipe, add the read event for main
   // thread. It should be called in the worker process.
-  bool NgxUrlAsyncFetcher::Init() {
-    log_ = ngx_cycle->log;
-
+  bool NgxUrlAsyncFetcher::Init(ngx_cycle_t* cycle) {
+    log_ = cycle->log;
+    CHECK(event_connection_ == NULL) << "event connection already set";
+    event_connection_ = new NgxEventConnection(ReadCallback);
+    if (!event_connection_->Init(cycle)) {
+      return false;
+    }
     if (pool_ == NULL) {
       pool_ = ngx_create_pool(4096, log_);
       if (pool_ == NULL) {
@@ -164,41 +167,6 @@ namespace net_instaweb {
       }
     }
 
-    int pipe_fds[2];
-    int rc = pipe(pipe_fds);
-    if (rc != 0) {
-      ngx_log_error(NGX_LOG_ERR, log_, 0, "pipe() failed");
-      return false;
-    }
-    if (ngx_nonblocking(pipe_fds[0]) == -1) {
-      ngx_log_error(NGX_LOG_ERR, log_, 0, "nonblocking pipe[0] failed");
-      return false;
-    }
-    if (ngx_nonblocking(pipe_fds[1]) == -1) {
-      ngx_log_error(NGX_LOG_ERR, log_, 0, "nonblocking pipe[1] failed");
-      return false;
-    }
-
-    pipe_fd_ = pipe_fds[1];
-    command_connection_ = ngx_get_connection(pipe_fds[0], log_);
-    if (command_connection_ == NULL) {
-      close(pipe_fds[1]);
-      close(pipe_fds[0]);
-      pipe_fd_ = -1;
-      return false;
-    }
-
-    command_connection_->recv = ngx_recv;
-    command_connection_->send = ngx_send;
-    command_connection_->recv_chain = ngx_recv_chain;
-    command_connection_->send_chain = ngx_send_chain;
-    command_connection_->log = log_;
-    command_connection_->read->log = log_;
-    command_connection_->write->log = log_;
-    command_connection_->data = this;
-    command_connection_->read->handler = CommandHandler;
-    ngx_add_event(command_connection_->read, NGX_READ_EVENT, 0);
-
     if (proxy_.url.len == 0) {
       return true;
     }
@@ -214,8 +182,29 @@ namespace net_instaweb {
   }
 
   void NgxUrlAsyncFetcher::ShutDown() {
-      shutdown_ = true;
-      SendCmd('S');
+    shutdown_ = true;
+    if (!pending_fetches_.empty()) {
+      for (Pool<NgxFetch>::iterator p = pending_fetches_.begin(),
+           e = pending_fetches_.end(); p != e; p++) {
+        NgxFetch* fetch = *p;
+        fetch->CallbackDone(false);
+      }
+      pending_fetches_.DeleteAll();
+    }
+
+    if (!active_fetches_.empty()) {
+      for (Pool<NgxFetch>::iterator p = active_fetches_.begin(),
+           e = active_fetches_.end(); p != e; p++) {
+        NgxFetch* fetch = *p;
+        fetch->CallbackDone(false);
+      }
+      active_fetches_.Clear();
+    }
+    if (event_connection_ != NULL) {
+      event_connection_->Shutdown();
+      delete event_connection_;
+      event_connection_ = NULL;
+    }
   }
 
   // It's called in the rewrite thread. All the fetches are started at
@@ -223,92 +212,46 @@ namespace net_instaweb {
   void NgxUrlAsyncFetcher::Fetch(const GoogleString& url,
                                  MessageHandler* message_handler,
                                  AsyncFetch* async_fetch) {
+    // Don't accept new fetches when shut down. This flow is also entered when
+    // we did not initialize properly in ::Init().
+    if (shutdown_) {
+      async_fetch->Done(false);
+      return;
+    }
     async_fetch = EnableInflation(async_fetch);
     NgxFetch* fetch = new NgxFetch(url, async_fetch,
           message_handler, log_);
     ScopedMutex lock(mutex_);
     pending_fetches_.Add(fetch);
-    SendCmd('F');
-  }
 
-  // send command to nginx main thread
-  // 'F' : start a fetch
-  // 'S' : shutdown the fetcher
-  bool NgxUrlAsyncFetcher::SendCmd(const char command) {
-    int rc;
-    while (true) {
-      rc = write(pipe_fd_, &command, 1);
-      if (rc == 1) {
-        return true;
-      } else if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) {
-        // TODO(junmin): It's rare. But it need be fixed.
-      } else {
-        return false;
-      }
-    }
-    return true;
+    // TODO(oschaaf): thread safety on written vs shutdown.
+    // It is possible that shutdown() is called after writing an event? In that
+    // case, this could (rarely) fail when it shouldn't.
+    bool written = event_connection_->WriteEvent(this);
+    CHECK(written || shutdown_) << "NgxUrlAsyncFetcher: event write failure";
   }
 
   // This is the read event which is called in the main thread.
   // It will do the real work. Add the work event and start the fetch.
-  void NgxUrlAsyncFetcher::CommandHandler(ngx_event_t* cmdev) {
-    char command;
-    int rc;
-    ngx_connection_t* c = static_cast<ngx_connection_t*>(cmdev->data);
-    NgxUrlAsyncFetcher* fetcher = static_cast<NgxUrlAsyncFetcher*>(c->data);
-    do {
-      rc = read(c->fd, &command, 1);
-    } while (rc == -1 && errno == EINTR);
+  void NgxUrlAsyncFetcher::ReadCallback(const ps_event_data& data) {
+    std::vector<NgxFetch*> to_start;
+    NgxUrlAsyncFetcher* fetcher = reinterpret_cast<NgxUrlAsyncFetcher*>(
+      data.sender);
 
-    CHECK(rc == -1 || rc == 0 || rc == 1);
+    fetcher->mutex_->Lock();
+    fetcher->completed_fetches_.DeleteAll();
 
-    if (rc == -1 || rc == 0) {
-      // EAGAIN
-      return;
+    for (Pool<NgxFetch>::iterator p = fetcher->pending_fetches_.begin(),
+             e = fetcher->pending_fetches_.end(); p != e; p++) {
+      NgxFetch* fetch = *p;
+      to_start.push_back(fetch);
     }
 
-    std::vector<NgxFetch*> to_start;
+    fetcher->pending_fetches_.Clear();
+    fetcher->mutex_->Unlock();
 
-    switch (command) {
-      // All the new fetches are appended in the pending_fetches.
-      // Start all these fetches.
-      case 'F':
-        fetcher->mutex_->Lock();
-        fetcher->completed_fetches_.DeleteAll();
-        for (Pool<NgxFetch>::iterator p = fetcher->pending_fetches_.begin(),
-                 e = fetcher->pending_fetches_.end(); p != e; p++) {
-          NgxFetch* fetch = *p;
-          to_start.push_back(fetch);
-        }
-
-        fetcher->pending_fetches_.Clear();
-        fetcher->mutex_->Unlock();
-
-        for (size_t i = 0; i < to_start.size(); i++) {
-          fetcher->StartFetch(to_start[i]);
-        }
-        CHECK(ngx_handle_read_event(cmdev, 0) == NGX_OK);
-        break;
-
-      // Shutdown all the fetches.
-      case 'S':
-        if (!fetcher->pending_fetches_.empty()) {
-          fetcher->pending_fetches_.DeleteAll();
-        }
-
-        if (!fetcher->active_fetches_.empty()) {
-          for (Pool<NgxFetch>::iterator p = fetcher->active_fetches_.begin(),
-               e = fetcher->active_fetches_.end(); p != e; p++) {
-            NgxFetch* fetch = *p;
-            fetch->CallbackDone(false);
-          }
-          fetcher->active_fetches_.Clear();
-        }
-        CHECK(ngx_del_event(cmdev, NGX_READ_EVENT, 0) == NGX_OK);
-        break;
-
-      default:
-        break;
+    for (size_t i = 0; i < to_start.size(); i++) {
+      fetcher->StartFetch(to_start[i]);
     }
 
     return;

src/ngx_url_async_fetcher.h

@@ -33,11 +33,14 @@ extern "C" {
 }
 
 #include <vector>
+
+#include "ngx_event_connection.h"
+
 #include "net/instaweb/http/public/url_async_fetcher.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/pool.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/basictypes.h"
+#include "pagespeed/kernel/base/pool.h"
+#include "pagespeed/kernel/base/string.h"
+#include "pagespeed/kernel/base/thread_system.h"
 
 
 namespace net_instaweb {
@@ -59,22 +62,21 @@ class NgxUrlAsyncFetcher : public UrlAsyncFetcher {
   ~NgxUrlAsyncFetcher();
 
   // It should be called in the module init_process callback function. Do some
-  // initializations which can't be done in the master process
-  bool Init();
+  // intializations which can't be done in the master process
+  bool Init(ngx_cycle_t* cycle);
 
   // shutdown all the fetches.
   virtual void ShutDown();
 
+  // the read handler in the main thread
+  static void ReadCallback(const ps_event_data& data);
+
   virtual bool SupportsHttps() const { return false; }
 
   virtual void Fetch(const GoogleString& url,
                      MessageHandler* message_handler,
                      AsyncFetch* callback);
 
-  // send the command from the current thread to main thread
-  bool SendCmd(const char command);
-  // the read handler in the main thread
-  static void CommandHandler(ngx_event_t* cmdev);
   bool StartFetch(NgxFetch* fetch);
 
   // Remove the completed fetch from the active fetch set, and put it into a
@@ -137,13 +139,13 @@ class NgxUrlAsyncFetcher : public UrlAsyncFetcher {
 
   ngx_pool_t* pool_;
   ngx_log_t* log_;
-  ngx_connection_t* command_connection_;  // the command pipe
-  int pipe_fd_;  // the write pipe end
   ngx_resolver_t* resolver_;
   int max_keepalive_requests_;
   ngx_msec_t resolver_timeout_;
   ngx_msec_t fetch_timeout_;
 
+  NgxEventConnection* event_connection_;
+
   DISALLOW_COPY_AND_ASSIGN(NgxUrlAsyncFetcher);
 };
 

test/pagespeed_test.conf.template

@@ -24,6 +24,9 @@ http {
                    '"$http_user_agent"';
   access_log "@@ACCESS_LOG@@" cache;
 
+  # Don't put entries in the error log for 403s and 404s.
+  log_not_found off;
+
   proxy_cache_path "@@PROXY_CACHE@@"  levels=1:2   keys_zone=htmlcache:60m inactive=90m  max_size=50m;
   proxy_temp_path "@@TMP_PROXY_CACHE@@";
 
@@ -266,14 +269,14 @@ http {
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
-    server_name mpd.example.com;
+    server_name secondary.example.com;
     pagespeed FileCachePath "@@SECONDARY_CACHE@@";
 
-    pagespeed MapProxyDomain mpd.example.com/gstatic_images
+    pagespeed MapProxyDomain secondary.example.com/gstatic_images
                              http://www.gstatic.com/psa/static;
   }
 
-  # These two vhosts are for testing the experiment framework (Furious).
+  # These three vhosts are for testing the experiment framework (Furious).
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
@@ -283,12 +286,31 @@ http {
 
     pagespeed RunExperiment on;
     pagespeed AnalyticsID "123-45-6734";
+    pagespeed UseAnalyticsJs false;
     pagespeed ExperimentVariable 2;
     pagespeed ExperimentSpec
       "id=7;enable=recompress_images;disable=convert_jpeg_to_progressive;percent=50";
     pagespeed ExperimentSpec "id=2;enable=recompress_images;percent=50";
     pagespeed ExperimentSpec "id=3;default;percent=0";
   }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name contentexperiment.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    pagespeed InPlaceResourceOptimization off;
+
+    pagespeed RunExperiment on;
+    pagespeed AnalyticsID "123-45-6734";
+    pagespeed UseAnalyticsJs false;
+    pagespeed ExperimentVariable 2;
+    pagespeed ExperimentSpec
+      "id=7;enable=recompress_images;disable=convert_jpeg_to_progressive;percent=50;options=ContentExperimentID=ID:H1BpS2TCRFmJzZgjwyeBHQ,ContentExperimentVariantID=111";
+    pagespeed ExperimentSpec
+      "id=2;enable=recompress_images;percent=50;options=ContentExperimentID=ID:H1BpS2TCRFmJzZgjwyeBHQ,ContentExperimentVariantID=222";
+    pagespeed ExperimentSpec
+      "id=3;default;percent=0;options=ContentExperimentID=123,ContentExperimentVariantID=333";
+  }
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
@@ -302,6 +324,16 @@ http {
     pagespeed ExperimentSpec "id=2;enable=recompress_images;percent=50";
     pagespeed ExperimentSpec "id=3;default;percent=0";
   }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name experiment.devicematch.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+
+    pagespeed RunExperiment on;
+    pagespeed ExperimentSpec
+      "id=1;percent=100;matches_device_type=mobile;enable=recompress_images";
+  }
 
   server {
     listen @@SECONDARY_PORT@@;
@@ -372,6 +404,34 @@ http {
     pagespeed DownstreamCachePurgeLocationPrefix "http://localhost:@@SECONDARY_PORT@@/purge";
   }
 
+  # Set up a reverse proxy (rproxy.) and origin (origin.) as vhosts for
+  # showing that we can configure PageSpeed via response headers.
+  server {
+    listen @@SECONDARY_PORT@@;
+    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://127.0.0.1:@@SECONDARY_PORT@@/;
+      proxy_set_header "Host" "origin.rmcomments.example.com";
+    }
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name origin.rmcomments.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    pagespeed off;
+    location / {
+      add_header PageSpeedFilters remove_comments;
+    }
+  }
+
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
@@ -387,9 +447,6 @@ http {
   # Build a configuration hierarchy where at the root we have turned on
   # OptimizeForBandwidth, and in various subdirectories we override settings
   # to make them more aggressive.
-  #
-  # In Apache we can do this all with Directory blocks, but to get the same
-  # inheretence in Nginx we need to have location blocks inside a server block.
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
@@ -507,6 +564,10 @@ http {
     pagespeed UrlValuedAttribute img alt-src Image;
     pagespeed UrlValuedAttribute video alt-a Image;
     pagespeed UrlValuedAttribute video alt-b Image;
+    pagespeed UrlValuedAttribute link data-stylesheet Stylesheet;
+    pagespeed UrlValuedAttribute span data-stylesheet-a Stylesheet;
+    pagespeed UrlValuedAttribute span data-stylesheet-b Stylesheet;
+    pagespeed UrlValuedAttribute span data-stylesheet-c Stylesheet;
 
     # Also test that we can redefine spec-defined attributes.
     pagespeed UrlValuedAttribute blockquote cite Image;
@@ -548,15 +609,6 @@ http {
     pagespeed EnableFilters rewrite_domains;
   }
 
-  server {
-    listen @@SECONDARY_PORT@@;
-    listen [::]:@@SECONDARY_PORT@@;
-    server_name retaincomment.example.com;
-    pagespeed FileCachePath "@@FILE_CACHE@@";
-
-    pagespeed RetainComment " google_ad_section*";
-  }
-
   server {
     # Test host for shared memory cache.
     listen @@SECONDARY_PORT@@;
@@ -573,8 +625,23 @@ http {
     listen [::]:@@SECONDARY_PORT@@;
     server_name xfp.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-
     pagespeed RespectXForwardedProto on;
+
+    location /redirecting_origin {
+      pagespeed off;
+      # Hack: we clear the response headers using headers_more.
+      # If we don't, nginx will add an extra empty Location: headers here.
+      # It is kind of hard to get nginx to generate a relative location header
+      # that starts with "/".
+      more_clear_headers 'Location';
+      add_header Location /mod_pagespeed_example;
+      return 301;
+    }
+    location /redirect {
+      proxy_method GET;
+      proxy_pass http://127.0.0.1:@@SECONDARY_PORT@@/redirecting_origin;
+      proxy_set_header "Host" "xfp.example.com";
+    }
   }
 
   server {
@@ -640,7 +707,44 @@ http {
     pagespeed EnableFilters rewrite_images,rewrite_css;
     pagespeed EnableFilters convert_to_webp_lossless;
     pagespeed EnableFilters in_place_optimize_for_browser;
+    pagespeed JpegRecompressionQuality 75;
+    pagespeed WebpRecompressionQuality 70;
     pagespeed InPlaceResourceOptimization on;
+    pagespeed AllowVaryOn "Accept";
+    pagespeed FileCachePath "@@IPRO_CACHE@@";
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name ipro-for-browser-vary-on-auto.example.com;
+    root "@@SERVER_ROOT@@/mod_pagespeed_example";
+    pagespeed EnableFilters rewrite_images,rewrite_css;
+    pagespeed EnableFilters convert_to_webp_animated;
+    pagespeed EnableFilters convert_to_webp_lossless;
+    pagespeed EnableFilters in_place_optimize_for_browser;
+    pagespeed InPlaceResourceOptimization on;
+    # pagespeed AllowVaryOn "Accept";  # Default is "Auto".
+    pagespeed ImageRecompressionQuality 90;
+    pagespeed JpegRecompressionQuality 75;
+    pagespeed JpegRecompressionQualityForSmallScreens 55;
+    pagespeed JpegQualityForSaveData 35;
+    pagespeed WebpRecompressionQuality 70;
+    pagespeed WebpRecompressionQualityForSmallScreens 50;
+    pagespeed WebpQualityForSaveData 30;
+    pagespeed WebpAnimatedRecompressionQuality 60;
+    pagespeed FileCachePath "@@IPRO_CACHE@@";
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name ipro-for-browser-vary-on-none.example.com;
+    root "@@SERVER_ROOT@@/mod_pagespeed_example";
+    pagespeed EnableFilters rewrite_images,in_place_optimize_for_browser;
+    pagespeed InPlaceResourceOptimization on;
+    pagespeed AllowVaryOn "None";
+    pagespeed ImageRecompressionQuality 75;
     pagespeed FileCachePath "@@IPRO_CACHE@@";
   }
 
@@ -708,7 +812,7 @@ http {
     server_name www.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
-    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@;
+    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@/;
     pagespeed MapRewriteDomain cdn.example.com origin.example.com;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters rewrite_css,rewrite_images;
@@ -722,7 +826,7 @@ http {
     server_name origin.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
-    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@;
+    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@/;
     pagespeed MapRewriteDomain cdn.example.com origin.example.com;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters rewrite_css,rewrite_images;
@@ -736,7 +840,7 @@ http {
     server_name cdn.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
-    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@;
+    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@/;
     pagespeed MapRewriteDomain cdn.example.com origin.example.com;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters rewrite_css,rewrite_images;
@@ -790,6 +894,13 @@ http {
     pagespeed ConsolePath /custom_pagespeed_console;
     pagespeed MessagesPath /custom_pagespeed_message;
     pagespeed AdminPath /custom_pagespeed_admin;
+
+    pagespeed StatisticsDomains Allow *;
+    pagespeed GlobalStatisticsDomains Allow *;
+    pagespeed MessagesDomains Allow *;
+    pagespeed ConsoleDomains Allow *;
+    pagespeed AdminDomains Allow *;
+    pagespeed GlobalAdminDomains Allow *;
   }
 
   server {
@@ -797,18 +908,13 @@ http {
     listen [::]:@@SECONDARY_PORT@@;
     server_name inherit-paths.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-  }
 
-  server {
-    listen @@SECONDARY_PORT@@;
-    listen [::]:@@SECONDARY_PORT@@;
-    server_name notransform.example.com;
-    pagespeed FileCachePath "@@FILE_CACHE@@";
-
-    pagespeed RewriteLevel PassThrough;
-    pagespeed EnableFilters rewrite_images;
-
-    add_header Cache-Control no-transform;
+    pagespeed StatisticsDomains Allow *;
+    pagespeed GlobalStatisticsDomains Allow *;
+    pagespeed MessagesDomains Allow *;
+    pagespeed ConsoleDomains Allow *;
+    pagespeed AdminDomains Allow *;
+    pagespeed GlobalAdminDomains Allow *;
   }
 
   server {
@@ -869,7 +975,10 @@ http {
     pagespeed FileCachePath "@@SECONDARY_CACHE@@";
     pagespeed RewriteLevel PassThrough;
     pagespeed on;
-    add_header PageSpeedFilters add_instrumentation;
+    location / {
+      proxy_pass http://localhost:@@SECONDARY_PORT@@;
+      proxy_set_header Host "enable-filter-header-origin.example.com";
+    }
   }
 
   server {
@@ -879,7 +988,32 @@ http {
     pagespeed FileCachePath "@@SECONDARY_CACHE@@";
     pagespeed EnableFilters add_instrumentation;
     pagespeed on;
-    add_header PageSpeed off;
+    location / {
+      proxy_pass http://localhost:@@SECONDARY_PORT@@;
+      proxy_set_header Host "pagespeed-off-header-origin.example.com";
+    }
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name headers.example.com;
+    pagespeed InPlaceRewriteDeadlineMs -1;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed LoadFromFile "http://headers.example.com/"
+              "@@SERVER_ROOT@@/";
+    location /mod_pagespeed_test/ {
+      more_set_headers "Server: override";
+      more_set_headers -s '404' 'Cache-Control: override';
+    }
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name custom404.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    error_page 404 /404.html;
   }
 
   server {
@@ -962,7 +1096,7 @@ http {
     listen [::]:@@SECONDARY_PORT@@;
     server_name date.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    add_header "Date" "Date: Fri, 16 Oct 2009 23:05:07 GMT";
+    add_header "Date" "Fri, 16 Oct 2009 23:05:07 GMT";
   }
 
   server {
@@ -1044,6 +1178,56 @@ http {
     pagespeed DisableFilters remove_comments,add_instrumentation;
   }
 
+  # Start remote config servers.
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name remote-config.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RemoteConfigurationUrl "http://127.0.0.1:@@RCPORT1@@/remote.cfg";
+    pagespeed RemoteConfigurationTimeoutMs 1500;
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name remote-config-partially-invalid.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RemoteConfigurationUrl "http://127.0.0.1:@@RCPORT2@@/remote.cfg";
+    pagespeed RemoteConfigurationTimeoutMs 1500;
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name remote-config-invalid.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RemoteConfigurationUrl "http://127.0.0.1:@@RCPORT3@@/remote.cfg";
+    pagespeed RemoteConfigurationTimeoutMs 1500;
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name remote-config-failed-fetch.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RemoteConfigurationUrl "http://127.0.0.1:@@RCPORT5@@/remote.cfg";
+    pagespeed RemoteConfigurationTimeoutMs 1500;
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name remote-config-slow-fetch.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RemoteConfigurationUrl "http://127.0.0.1:@@RCPORT6@@/remote.cfg";
+    pagespeed RemoteConfigurationTimeoutMs 1500;
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name remote-config-experiment.example.com;
+    pagespeed FileCachePath "@@SECONDARY_CACHE@@";
+    pagespeed RemoteConfigurationUrl "http://127.0.0.1:@@RCPORT7@@/remote.cfg";
+    pagespeed RemoteConfigurationTimeoutMs 1500;
+  }
+
   # Test that pagespeed is disabled when sendfile headers are present.
   server {
     listen @@SECONDARY_PORT@@;
@@ -1086,6 +1270,44 @@ http {
     }
   }
 
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name compressedcache.example.com;
+    pagespeed on;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    pagespeed EnableFilters rewrite_css;
+    pagespeed HttpCacheCompressionLevel 9;
+    location ~ \.php$ {
+      fastcgi_param SCRIPT_FILENAME $request_filename;
+      fastcgi_param QUERY_STRING $query_string;
+      fastcgi_param REQUEST_METHOD $request_method;
+      fastcgi_param CONTENT_TYPE $content_type;
+      fastcgi_param CONTENT_LENGTH $content_length;
+      fastcgi_pass 127.0.0.1:9000;
+      fastcgi_buffering off;
+    }
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name uncompressedcache.example.com;
+    pagespeed on;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    pagespeed EnableFilters rewrite_css;
+    pagespeed HttpCacheCompressionLevel 0;
+    location ~ \.php$ {
+      fastcgi_param SCRIPT_FILENAME $request_filename;
+      fastcgi_param QUERY_STRING $query_string;
+      fastcgi_param REQUEST_METHOD $request_method;
+      fastcgi_param CONTENT_TYPE $content_type;
+      fastcgi_param CONTENT_LENGTH $content_length;
+      fastcgi_pass 127.0.0.1:9000;
+      fastcgi_buffering off;
+    }
+  }
+
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
@@ -1148,7 +1370,7 @@ http {
     listen [::]:@@SECONDARY_PORT@@;
     server_name proxy-post-origin.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/";
   }
 
   server {
@@ -1161,6 +1383,24 @@ http {
     pagespeed FileCachePath "@@FILE_CACHE@@";
   }
 
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name script-filters.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    root "@@SERVER_ROOT@@/";
+    set $filters "";
+    set $domain_shards "cdn1.example.com,cdn2.example.com";
+    if ($http_X_Script) {
+        set $filters "add_instrumentation";
+        set $domain_shards "";
+    }
+    pagespeed RewriteLevel PassThrough;
+    pagespeed EnableFilters rewrite_domains;
+    pagespeed EnableFilters $filters;
+    pagespeed ShardDomain script-filters.example.com "$domain_shards";
+  }
+
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
@@ -1170,7 +1410,7 @@ http {
     pagespeed EnableCachePurge on;
 
     pagespeed PurgeMethod PURGE;
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/purge";
     pagespeed FileCachePath "@@FILE_CACHE@@_purge";
     pagespeed DisableFilters add_instrumentation;
     pagespeed RewriteLevel PassThrough;
@@ -1180,19 +1420,154 @@ http {
   server {
     listen @@SECONDARY_PORT@@;
     listen [::]:@@SECONDARY_PORT@@;
-    server_name headers.example.com;
-    root "@@SERVER_ROOT@@";
-    pagespeed FileCachePath "@@FILE_CACHE@@_purge";
-    pagespeed RewriteLevel CoreFilters;
+    server_name psoff-dir-on.example.com;
 
-    pagespeed InPlaceRewriteDeadlineMs -1;
-    pagespeed LoadFromFile "http://headers.example.com/"
-              "@@SERVER_ROOT@@/";
-    location /mod_pagespeed_test/ {
-      more_set_headers "Server: overriden";
+    # Test purging individual URLs without flushing the entire metadata cache.
+    pagespeed EnableCachePurge on;
+
+    pagespeed PurgeMethod PURGE;
+    root "@@SERVER_ROOT@@/purge";
+    pagespeed FileCachePath "@@FILE_CACHE@@_dir_on";
+    pagespeed DisableFilters add_instrumentation;
+    pagespeed RewriteLevel PassThrough;
+    pagespeed EnableFilters rewrite_css;
+    pagespeed off;
+    location / {
+      pagespeed on;
     }
   }
 
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name  cc-header-origin.example.com;
+    pagespeed InPlaceResourceOptimization off;
+    pagespeed off;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    root "@@SERVER_ROOT@@/";
+    location /mod_pagespeed_test/nostore {
+      add_header "Cache-Control" "max-age=12345";
+      add_header "Cache-Control" "public, no-store";
+      add_header "Cache-Control" "max-age=14";
+    }
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name  enable-filter-header-origin.example.com;
+    pagespeed InPlaceResourceOptimization off;
+    pagespeed off;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    root "@@SERVER_ROOT@@/";
+    location / {
+      add_header "PageSpeedFilters" "add_instrumentation";
+    }
+  }
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name  pagespeed-off-header-origin.example.com;
+    pagespeed InPlaceResourceOptimization off;
+    pagespeed off;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    root "@@SERVER_ROOT@@/";
+    location / {
+      add_header "PageSpeed" "off";
+    }
+  }
+
+  pagespeed MessagesDomains Allow messages-allowed.example.com;
+  pagespeed MessagesDomains Allow cleared-inherited.example.com;
+  pagespeed MessagesDomains Allow cleared-inherited-reallowed.example.com;
+  pagespeed MessagesDomains Allow more-messages-allowed.example.com;
+  pagespeed MessagesDomains Allow anything-*-wildcard.example.com;
+  pagespeed MessagesDomains Allow localhost;
+
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name messages-allowed.example.com
+                messages-not-allowed.example.com
+                more-messages-allowed.example.com
+                anything-a-wildcard.example.com
+                anything-b-wildcard.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name messages-still-not-allowed.example.com
+                but-this-message-allowed.example.com
+                and-this-one.example.com;
+    pagespeed MessagesDomains Allow but-this-message-allowed.example.com;
+    pagespeed MessagesDomains Allow and-this-one.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name cleared-inherited.example.com
+                cleared-inherited-reallowed.example.com
+                messages-allowed-at-vhost.example.com
+                messages-not-allowed-at-vhost.example.com
+                anything-c-wildcard.example.com;
+    pagespeed MessagesDomains Disallow *;
+    pagespeed MessagesDomains Allow cleared-inherited-reallowed.example.com;
+    pagespeed MessagesDomains Allow messages-allowed-at-vhost.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name cleared-inherited-unlisted.example.com;
+    pagespeed MessagesDomains Allow *;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
+  server {
+    server_name nothing-allowed.example.com;
+    pagespeed MessagesDomains Disallow *;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
+  server {
+    server_name nothing-explicitly-allowed.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+  }
+  server {
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name everything-explicitly-allowed.example.com
+                everything-explicitly-allowed-but-aliased.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+
+    pagespeed StatisticsDomains Allow everything-explicitly-allowed.example.com;
+    pagespeed GlobalStatisticsDomains
+      Allow everything-explicitly-allowed.example.com;
+    pagespeed MessagesDomains Allow everything-explicitly-allowed.example.com;
+    pagespeed ConsoleDomains Allow everything-explicitly-allowed.example.com;
+    pagespeed AdminDomains Allow everything-explicitly-allowed.example.com;
+    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 {
+    pagespeed on;
+    listen @@SECONDARY_PORT@@;
+    listen [::]:@@SECONDARY_PORT@@;
+    server_name debug-filters.example.com;
+    pagespeed FileCachePath "@@FILE_CACHE@@";
+    pagespeed RewriteLevel PassThrough;
+    pagespeed EnableFilters debug;
+  }
   server {
     listen @@PRIMARY_PORT@@;
     listen [::]:@@PRIMARY_PORT@@;
@@ -1237,6 +1612,10 @@ http {
       add_header 'Cache-Control' 'no-transform';
     }
 
+    location /mod_pagespeed_test/no_transform {
+      add_header 'Cache-Control' 'no-transform';
+    }
+
     # uncomment the following two lines if you're testing memcached
     #pagespeed MemcachedServers "localhost:11211";
     #pagespeed MemcachedThreads 1;
@@ -1251,7 +1630,16 @@ http {
     #pagespeed ExperimentSpec "id=2;percent=50";
 
     pagespeed Library 43 1o978_K0_LNE5_ystNklf
-    http://www.modpagespeed.com/rewrite_javascript.js;
+      http://www.modpagespeed.com/rewrite_javascript.js;
+    pagespeed RetainComment " google_ad_section*";
+
+    # Test proxying of non-.pagespeed. resources.
+    pagespeed MapProxyDomain http://localhost:@@PRIMARY_PORT@@/modpagespeed_http
+                           http://@@PAGESPEED_TEST_HOST@@/do_not_modify;
+    pagespeed MapProxyDomain http://localhost:@@PRIMARY_PORT@@/content_type_present
+                           http://@@PAGESPEED_TEST_HOST@@:8091;
+    pagespeed MapProxyDomain http://localhost:@@PRIMARY_PORT@@/content_type_absent
+                           http://@@PAGESPEED_TEST_HOST@@:8092;
 
     add_header X-Extra-Header 1;
 
@@ -1277,9 +1665,8 @@ http {
     pagespeed AvoidRenamingIntrospectiveJavascript off;
 
     location /mod_pagespeed_test/nostore {
-      add_header "Cache-Control" "max-age=12345";
-      add_header "Cache-Control" "public, no-store";
-      add_header "Cache-Control" "max-age=14";
+      proxy_pass http://localhost:@@SECONDARY_PORT@@;
+      proxy_set_header Host "cc-header-origin.example.com";
     }
 
     location /mod_pagespeed_test/forbid_all_disabled/disabled {
@@ -1372,6 +1759,30 @@ http {
       expires 5m;
     }
 
+    location /mod_pagespeed_test/ipro/wait/ {
+      # TODO(jmarantz): ModPagespeedInPlaceWaitForOptimized should be superfluous,
+      # or made equivalent to ModPagespeedInPlaceRewriteDeadlineMs -1, which waits
+      # forever.  Otherwise ModPagespeedInPlaceRewriteDeadlineMs should just have
+      # the specified deadline.
+      # # See https://github.com/pagespeed/mod_pagespeed/issues/1171 for more
+      # detailed discussion.
+      pagespeed InPlaceWaitForOptimized on;
+    }
+
+    location /mod_pagespeed_test/ipro/wait/long/ {
+      # Make the deadline long here for valgrind tests.  We could
+      # conditionalize this.
+      pagespeed InPlaceRewriteDeadlineMs 10000;
+    }
+
+    location /mod_pagespeed_test/ipro/wait/short/ {
+      pagespeed EnableFilters in_place_optimize_for_browser;
+
+      # Make the deadline short here as we expect to always miss it
+      # in tests.
+      pagespeed InPlaceRewriteDeadlineMs 1;
+    }
+
     location /mod_pagespeed_test/ipro/instant/wait/ {
       pagespeed InPlaceWaitForOptimized on;
 
@@ -1395,7 +1806,7 @@ http {
     # optimized.
     location /mod_pagespeed_test/auth/ {
        auth_basic "Restricted";
-       auth_basic_user_file "@@SERVER_ROOT@@mod_pagespeed_test/auth/passwd.conf";
+       auth_basic_user_file "@@SERVER_ROOT@@/mod_pagespeed_test/auth/passwd.conf";
     }
 
     location /mod_pagespeed_test/ipro/cookie/ {
@@ -1441,6 +1852,25 @@ http {
           https://www.gstatic.com/psa/static;
     }
 
+    location /mod_pagespeed_test/strip_subresource_hints/default/ {
+      pagespeed DisableFilters add_instrumentation;
+      pagespeed RewriteLevel CoreFilters;
+      pagespeed DisAllow *dontrewriteme*;
+    }
+    location /mod_pagespeed_test/strip_subresource_hints/default_passthrough/ {
+      pagespeed DisableFilters add_instrumentation;
+      pagespeed RewriteLevel PassThrough;
+    }
+    location /mod_pagespeed_test/strip_subresource_hints/preserve_on/ {
+      pagespeed DisableFilters add_instrumentation;
+      pagespeed PreserveSubresourceHints on;
+      pagespeed RewriteLevel CoreFilters;
+    }
+    location /mod_pagespeed_test/strip_subresource_hints/preserve_off/ {
+      pagespeed DisableFilters add_instrumentation;
+      pagespeed PreserveSubresourceHints off;
+      pagespeed RewriteLevel CoreFilters;
+    }
     # $host implicitly tests script variable support. I'd love to test it more
     # directly, but so far this is the best I've come up with and duplicating
     # the test doesn't seem to make sense.
@@ -1448,6 +1878,11 @@ http {
         "http://$host:@@PRIMARY_PORT@@/mod_pagespeed_test/ipro/instant/"
         "@@SERVER_ROOT@@/mod_pagespeed_test/ipro/instant/";
 
+    location /mod_pagespeed_test/public/ {
+      add_header "Cache-Control" "public, max-age=600";
+      pagespeed PreserveUrlRelativity off;
+    }
+
     pagespeed EnableFilters remove_comments;
 
     # Test LoadFromFile mapping by mapping one dir to another.

test/run_tests.sh

@@ -23,9 +23,11 @@
 # Exits with status 2 if command line args are wrong.
 #
 # Usage:
-#   ./run_tests.sh primary_port secondary_port mod_pagespeed_dir
+#   ./run_tests.sh primary_port secondary_port mod_pagespeed_dir ngx_binary
+#   pagespeed_test_host
 # Example:
-#   ./run_tests.sh 8050 8051 /path/to/mod_pagespeed
+#   ./run_tests.sh 8050 8051 /path/to/mod_pagespeed /path/to/nginx/binary
+#   selfsigned.modpagespeed.com
 #
 
 # Normally we test only with the native fetcher off.  Set
@@ -43,7 +45,7 @@ RUN_TESTS=${RUN_TESTS:-true}
 # true.
 USE_VALGRIND=${USE_VALGRIND:-false}
 
-if [ "$#" -ne 4 ] ; then
+if [ "$#" -ne 5 ] ; then
   echo "Usage: $0 primary_port secondary_port mod_pagespeed_dir"
   echo "  nginx_executable"
   exit 2
@@ -53,6 +55,14 @@ PRIMARY_PORT="$1"
 SECONDARY_PORT="$2"
 MOD_PAGESPEED_DIR="$3"
 NGINX_EXECUTABLE="$4"
+PAGESPEED_TEST_HOST="$5"
+RCPORT1=9991
+RCPORT2=9992
+RCPORT3=9993
+RCPORT4=9994
+RCPORT5=9995
+RCPORT6=9996
+RCPORT7=9997
 
 this_dir="$( cd $(dirname "$0") && pwd)"
 
@@ -62,7 +72,15 @@ function run_test_checking_failure() {
     SECONDARY_PORT="$SECONDARY_PORT" \
     MOD_PAGESPEED_DIR="$MOD_PAGESPEED_DIR" \
     NGINX_EXECUTABLE="$NGINX_EXECUTABLE" \
+    PAGESPEED_TEST_HOST="$PAGESPEED_TEST_HOST" \
     RUN_TESTS="$RUN_TESTS" \
+    RCPORT1="$RCPORT1" \
+    RCPORT2="$RCPORT2" \
+    RCPORT3="$RCPORT3" \
+    RCPORT4="$RCPORT4" \
+    RCPORT5="$RCPORT5" \
+    RCPORT6="$RCPORT6" \
+    RCPORT7="$RCPORT7" \
     bash "$this_dir/nginx_system_test.sh"
   STATUS=$?
   echo "With $@ setup."