update mps

I initially planned to get this working on Ubuntu 12, but decided it wasn't
worth it and switched to Ubuntu 14 (Trusty).  Now it passes tests on Travis, so
I've added other people's emails back to the set of people to send emails to.

Also update mps while we're at it.

.gitignore

@@ -1,4 +1,5 @@
 test/tmp
 psol/
 psol-*.tar.gz
-
+*.*.*.*.tar.gz
+nginx

.gitmodules

@@ -0,0 +1,18 @@
+[submodule "testing-dependencies/mod_pagespeed"]
+	path = testing-dependencies/mod_pagespeed
+	url = https://github.com/pagespeed/mod_pagespeed.git
+[submodule "testing-dependencies/ngx_cache_purge"]
+	path = testing-dependencies/ngx_cache_purge
+	url = https://github.com/FRiCKLE/ngx_cache_purge.git
+[submodule "testing-dependencies/nginx"]
+	path = testing-dependencies/nginx
+	url = https://github.com/nginx/nginx.git
+[submodule "testing-dependencies/set-misc-nginx-module"]
+	path = testing-dependencies/set-misc-nginx-module
+	url = https://github.com/openresty/set-misc-nginx-module
+[submodule "testing-dependencies/ngx_devel_kit"]
+	path = testing-dependencies/ngx_devel_kit
+	url = https://github.com/simpl/ngx_devel_kit
+[submodule "testing-dependencies/headers-more-nginx-module"]
+	path = testing-dependencies/headers-more-nginx-module
+	url = https://github.com/openresty/headers-more-nginx-module

.travis.yml

@@ -0,0 +1,37 @@
+language: c++
+env:
+  global:
+    - MAKEFLAGS=-j3
+# By default travis loads submodules serially, but we can load them in parallel
+# if we install an updated git and use --jobs.  Some timing numbers:
+#   serial: 257s
+#   jobs=4: 182s (29s to install new git, 153s to run the downloads)
+#   jobs=8: 179s (29s to install new git, 150s to run the downloads)
+# We can't use --depth=1, though, because github doesn't have
+# allowReachableSHA1InWant set.
+#
+# TODO(jefftk): once we're running on a server with git 2.8 or later we can have
+#               --jobs without installing a new git.
+git:
+  submodules: false
+before_install:
+  - sudo add-apt-repository --yes ppa:git-core/ppa
+  - sudo apt-get update
+  - sudo apt-get install git
+  - git submodule update --init --recursive --jobs=8
+install:
+  scripts/build_ngx_pagespeed.sh --devel --assume-yes
+script:
+  test/run_tests.sh $PWD/testing-dependencies/mod_pagespeed/ \
+                    $PWD/nginx/sbin/nginx
+dist: trusty
+sudo: required
+compiler:
+  - gcc
+notifications:
+  email:
+    - cheesy@google.com
+    - jefftk@google.com
+    - morlovich@google.com
+    - jmarantz@google.com
+    - huibao@google.com

PSOL_BINARY_URL

@@ -0,0 +1,4 @@
+In a release this file would contain the URL to download the pre-compiled PSOL
+binary, but on development branches (like this one) you have to build PSOL from
+source yourself.  See:
+  https://github.com/pagespeed/ngx_pagespeed/wiki/Building-PSOL-From-Source

README.md

@@ -1,5 +1,7 @@
 ![ngx_pagespeed](https://lh6.googleusercontent.com/-qufedJIJq7Y/UXEvVYxyYvI/AAAAAAAADo8/JHDFQhs91_c/s401/04_ngx_pagespeed.png)
 
+[![Build Status](https://travis-ci.org/pagespeed/ngx_pagespeed.svg?branch=master)](https://travis-ci.org/pagespeed/ngx_pagespeed)
+
 ngx_pagespeed speeds up your site and reduces page load time by automatically
 applying web performance best practices to pages and associated assets (CSS,
 JavaScript, images) without requiring you to modify your existing content or
@@ -19,94 +21,15 @@ optimizations, see our <a href="http://ngxpagespeed.com">demonstration site</a>.
 
 ## How to build
 
-Because nginx does not support dynamic loading of modules, you need to compile
+Follow the steps on <a
-nginx from source to add ngx_pagespeed. Alternatively, if you're using Tengine you can [install ngx_pagespeed without
+href="https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source">build
-recompiling Tengine](https://github.com/pagespeed/ngx_pagespeed/wiki/Using-ngx_pagespeed-with-Tengine).
+ngx_pagespeed from source</a>.
-
-1. Install dependencies:
-
-   ```bash
-   # These are for RedHat, CentOS, and Fedora.
-   $ sudo yum install gcc-c++ pcre-dev pcre-devel zlib-devel make
-
-   # These are for Debian. Ubuntu will be similar.
-   $ sudo apt-get install build-essential zlib1g-dev libpcre3 libpcre3-dev
-   ```
-
-2. Download ngx_pagespeed:
-
-   ```bash
-   $ cd ~
-   $ wget https://github.com/pagespeed/ngx_pagespeed/archive/release-1.6.29.5-beta.zip
-   $ unzip release-1.6.29.5-beta.zip # or unzip release-1.6.29.5-beta
-   $ cd ngx_pagespeed-release-1.6.29.5-beta/
-   $ wget https://dl.google.com/dl/page-speed/psol/1.6.29.5.tar.gz
-   $ tar -xzvf 1.6.29.5.tar.gz # expands to psol/
-   ```
-
-3. Download and build nginx:
-
-   ```bash
-   $ # check http://nginx.org/en/download.html for the latest version
-   $ wget http://nginx.org/download/nginx-1.4.1.tar.gz
-   $ tar -xvzf nginx-1.4.1.tar.gz
-   $ cd nginx-1.4.1/
-   $ ./configure --add-module=$HOME/ngx_pagespeed-release-1.6.29.5-beta
-   $ make
-   $ sudo make install
-   ```
-
-If this doesn't work see the [build
-troubleshooting](https://github.com/pagespeed/ngx_pagespeed/wiki/Build-Troubleshooting) page.
-
-This will use a binary PageSpeed Optimization Library, but it's also possible to
-[build PSOL from
-source](https://github.com/pagespeed/ngx_pagespeed/wiki/Building-PSOL-From-Source).
-
-Note: ngx_pagespeed currently doesn't support Windows or MacOS because the
-underlying PSOL library doesn't.
 
 ## How to use
 
-In your `nginx.conf`, add to the main or server block:
+Follow the steps on <a
-
+href="https://developers.google.com/speed/pagespeed/module/configuration">PageSpeed
-```nginx
+configuration</a>.
-pagespeed on;
-
-# needs to exist and be writable by nginx
-pagespeed FileCachePath /var/ngx_pagespeed_cache;
-```
-
-In every server block where pagespeed is enabled add:
-
-```apache
-#  Ensure requests for pagespeed optimized resources go to the pagespeed
-#  handler and no extraneous headers get set.
-location ~ "\.pagespeed\.([a-z]\.)?[a-z]{2}\.[^.]{10}\.[^.]+" { add_header "" ""; }
-location ~ "^/ngx_pagespeed_static/" { }
-location ~ "^/ngx_pagespeed_beacon$" { }
-location /ngx_pagespeed_statistics { allow 127.0.0.1; deny all; }
-location /ngx_pagespeed_message { allow 127.0.0.1; deny all; }
-location /pagespeed_console { allow 127.0.0.1; deny all; }
-```
-
-To confirm that the module is loaded, fetch a page and check that you see the
-`X-Page-Speed` header:
-
-```bash
-$ curl -I 'http://localhost:8050/some_page/' | grep X-Page-Speed
-X-Page-Speed: 1.6.29.5-...
-```
-
-Looking at the source of a few pages you should see various changes, such as
-urls being replaced with new ones like `yellow.css.pagespeed.ce.lzJ8VcVi1l.css`.
-
-For complete documentation, see [Using
-PageSpeed](https://developers.google.com/speed/pagespeed/module/using).
-
-There are extensive system tests which cover most of ngx_pagespeed's
-functionality.  Consider [testing your
-installation](https://github.com/pagespeed/ngx_pagespeed/wiki/Testing).
 
 For feedback, questions, and to follow
 the progress of the project:

config

@@ -17,42 +17,40 @@
 #   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
 
   if [ ! -e "$mod_pagespeed_dir" ] ; then
     echo "ngx_pagespeed: pagespeed optimization library not found:"
-    echo ""
+
-    echo "   You need to separately download the pagespeed library:"
+    psol_binary_url="$($ngx_addon_dir/scripts/format_binary_url.sh \
-    echo ""
+                           $ngx_addon_dir/PSOL_BINARY_URL)"
-    echo "     $ cd /path/to/ngx_pagespeed"
+    if [[ "$psol_binary_url" != https://* ]]; then
-    echo "     $ wget https://dl.google.com/dl/page-speed/psol/1.6.29.5.tar.gz"
+      echo "
-    echo "     $ tar -xzvf 1.6.29.5.tar.gz # expands to psol/"
+  This is a development branch of ngx_pagespeed, which means there is no
-    echo ""
+  precompiled PSOL library available to link against.  Either build from a
-    echo "   Or see the installation instructions:"
+  release tag, like latest-beta, or build PSOL from source:
-    echo "     https://github.com/pagespeed/ngx_pagespeed#how-to-build"
+    https://github.com/pagespeed/ngx_pagespeed/wiki/Building-PSOL-From-Source"
       exit 1
     fi
 
+    echo "
+  You need to separately download the pagespeed library:
+     $ cd $ngx_addon_dir
+     $ wget $psol_binary_url
+     $ tar -xzvf $(basename $psol_binary_url) # expands to psol/
+
+  Or see the installation instructions:
+     https://developers.google.com/speed/pagespeed/module/build_ngx_pagespeed_from_source"
+    exit 1
+  fi
 else
   build_from_source=true
 fi
 
-echo "mod_pagespeed_dir=$mod_pagespeed_dir"
-echo "build_from_source=$build_from_source"
-
-ngx_feature="psol"
-ngx_feature_name=""
-ngx_feature_run=no
-ngx_feature_incs="
-#include \"net/instaweb/htmlparse/public/html_parse.h\"
-#include \"net/instaweb/htmlparse/public/html_writer_filter.h\"
-#include \"net/instaweb/util/public/string.h\"
-#include \"net/instaweb/util/public/string_writer.h\"
-#include \"net/instaweb/util/public/null_message_handler.h\"
-"
-
 os_name='unknown_os'
 arch_name='unknown_arch'
 uname_os=`uname`
@@ -79,46 +77,131 @@ fi
 
 if [ "$NGX_DEBUG" = "YES" ]; then
   buildtype=Debug
+  # If we're using a psol tarball that doesn't contain Debug/ (which is the case
+  # from 1.12 onward) then this will be overriden to buildtype=Release below.
 else
   buildtype=Release
 fi
 
-# Building with HTTPS fetching enabled pulls in a version of OpenSSL that causes
+# If the compiler is gcc, we want to use g++ to link, if at all possible,
-# linker errors, so disable it here.
+# so that -static-libstdc++ works.
-CFLAGS="$CFLAGS -DSERF_HTTPS_FETCHING=0"
+# 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
+fi
+pagespeed_libs="-lstdc++"
+
+# 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
+  FLAG_MARCH='-march=i686'
+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_OLD_TEST_FLAGS="$CC_TEST_FLAGS"
+CC_TEST_FLAGS="$CC_TEST_FLAGS -D_GLIBCXX_USE_CXX11_ABI=0 --std=c++11"
+
+case "$NGX_GCC_VER" in
+  4.8*)
+    # On GCC 4.8 and above, -Wall enables -Wunused-local-typedefs.  This breaks
+    # on VerifySizesAreEqual in bit_cast in chromium/src/base/basictypes.h which
+    # has a typedef that is intentionally unused.
+    CFLAGS="$CFLAGS -Wno-unused-local-typedefs"
+
+    # On GCC 4.8 and above, we get the following compiler warning:
+    # chromium/src/base/memory/scoped_ptr.h:133:7: warning: declaration of ‘class scoped_ptr<C>’ [enabled by default]
+    # Based on discussion at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54055,
+    # this is invalid code, but hasn't been fixed yet in chromium.
+    # Unfortunately, there also does not appear to be a flag for just disabling
+    # that warning, so we add Wno-error to override nginx's default -Werror
+    # option.
+    CFLAGS="$CFLAGS -Wno-error"
+  ;;
+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
+
+psol_binary="${PSOL_BINARY:-unset}"
+if [ "$psol_binary" = "unset" ] ; then
+  if $build_from_source ; then
+    psol_binary="\
+        $mod_pagespeed_dir/pagespeed/automatic/pagespeed_automatic.a"
+  else
+    if ! [ -d "$ngx_addon_dir/psol/lib/$buildtype" ]; then
+      echo "
+You have set --with-debug for building nginx, but precompiled Debug binaries for
+PSOL, which ngx_pagespeed depends on, aren't available.  If you're trying to
+debug PSOL you need to build it from source.  If you just want to run nginx with
+debug-level logging you can use the Release binaries."
+      echo -n "
+Use the available Release binaries?"
+      read -p " [Y/n] " yn
+      if [[ "$yn" == N* || "$yn" == n* ]]; then
+        echo "Cancelled."
+        exit 1
+      fi
+
+      buildtype=Release
+    fi
+    psol_library_dir="$ngx_addon_dir/psol/lib/$buildtype/$os_name/$arch_name"
+    psol_binary="$psol_library_dir/pagespeed_automatic.a"
+  fi
+fi
+
+echo "mod_pagespeed_dir=$mod_pagespeed_dir"
+echo "build_from_source=$build_from_source"
+
+ngx_feature="psol"
+ngx_feature_name=""
+ngx_feature_run=no
+ngx_feature_incs="
+#include \"pagespeed/kernel/base/string.h\"
+#include \"pagespeed/kernel/base/string_writer.h\"
+#include \"pagespeed/kernel/base/null_message_handler.h\"
+#include \"pagespeed/kernel/html/html_parse.h\"
+#include \"pagespeed/kernel/html/html_writer_filter.h\"
+"
 
 pagespeed_include="\
   $mod_pagespeed_dir \
   $mod_pagespeed_dir/third_party/chromium/src \
-  $mod_pagespeed_dir/third_party/google-sparsehash/src \
+  $mod_pagespeed_dir/third_party/google-sparsehash/src/src \
   $mod_pagespeed_dir/third_party/google-sparsehash/gen/arch/$os_name/$arch_name/include \
-  $mod_pagespeed_dir/third_party/protobuf/src \
+  $mod_pagespeed_dir/third_party/grpc/src/include \
+  $mod_pagespeed_dir/third_party/protobuf/src/src \
   $mod_pagespeed_dir/third_party/re2/src \
   $mod_pagespeed_dir/out/$buildtype/obj/gen \
+  $mod_pagespeed_dir/out/$buildtype/obj/gen/protoc_out/instaweb \
   $mod_pagespeed_dir/third_party/apr/src/include \
   $mod_pagespeed_dir/third_party/aprutil/src/include \
   $mod_pagespeed_dir/third_party/apr/gen/arch/$os_name/$arch_name/include \
-  $mod_pagespeed_dir/third_party/aprutil/gen/arch/$os_name/$arch_name/include"
+  $mod_pagespeed_dir/third_party/aprutil/gen/arch/$os_name/$arch_name/include \
+  $mod_pagespeed_dir/url"
 ngx_feature_path="$pagespeed_include"
 
-if $build_from_source ; then
+pagespeed_libs="$psol_binary $pagespeed_libs -lrt -pthread -lm -luuid"
-  psol_library_binaries="\
-    $mod_pagespeed_dir/net/instaweb/automatic/pagespeed_automatic.a \
-    $mod_pagespeed_dir/out/$buildtype/obj.target/third_party/serf/libserf.a \
-    $mod_pagespeed_dir/out/$buildtype/obj.target/third_party/aprutil/libaprutil.a \
-    $mod_pagespeed_dir/out/$buildtype/obj.target/third_party/apr/libapr.a"
-else
-  psol_library_dir="$ngx_addon_dir/psol/lib/$buildtype/$os_name/$arch_name"
-  psol_library_binaries="\
-    $psol_library_dir/pagespeed_automatic.a \
-    $psol_library_dir/libserf.a \
-    $psol_library_dir/libaprutil.a \
-    $psol_library_dir/libapr.a"
-fi
-
-pagespeed_libs="-lstdc++ $psol_library_binaries -lrt -pthread -lm"
 ngx_feature_libs="$pagespeed_libs"
 ngx_feature_test="
+
   GoogleString output_buffer;
   net_instaweb::StringWriter write_to_string(&output_buffer);
 
@@ -139,57 +222,163 @@ 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
+if [ $ngx_found = no ]; then
+  cat << END
+$0: error: module ngx_pagespeed requires the pagespeed optimization library.
+Look in $PWD/$NGX_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_request_context.h \
 $ps_src/ngx_rewrite_driver_factory.h \
 $ps_src/ngx_rewrite_options.h \
 $ps_src/ngx_server_context.h \
-    $ps_src/ngx_thread_system.h \
 $ps_src/ngx_url_async_fetcher.h \
-    $ps_src/pthread_shared_mem.h"
+$psol_binary"
-  NGX_ADDON_SRCS="$NGX_ADDON_SRCS \
+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_request_context.cc \
 $ps_src/ngx_rewrite_driver_factory.cc \
 $ps_src/ngx_rewrite_options.cc \
 $ps_src/ngx_server_context.cc \
-    $ps_src/ngx_thread_system.cc \
+$ps_src/ngx_url_async_fetcher.cc"
-    $ps_src/ngx_url_async_fetcher.cc \
+# Save our sources in a separate var since we may need it in config.make
-    $ps_src/pthread_shared_mem.cc \
+PS_NGX_SRCS="$NGX_ADDON_SRCS \
-    $mod_pagespeed_dir/out/$buildtype/obj/gen/data2c_out/instaweb/net/instaweb/system/console_out.cc \
+$NPS_SRCS"
-    $mod_pagespeed_dir/out/$buildtype/obj/gen/data2c_out/instaweb/net/instaweb/system/console_css_out.cc \
+
-    $mod_pagespeed_dir/net/instaweb/system/add_headers_fetcher.cc \
+# Make pagespeed run immediately before gzip and Brotli.
-    $mod_pagespeed_dir/net/instaweb/system/loopback_route_fetcher.cc \
+if echo $HTTP_FILTER_MODULES | grep ngx_http_brotli_filter_module >/dev/null; then
-    $mod_pagespeed_dir/net/instaweb/system/serf_url_async_fetcher.cc"
+  next=ngx_http_brotli_filter_module
-  # Make pagespeed run immediately before gzip.
+elif [ $HTTP_GZIP = YES ]; then
-  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
+  next=ngx_http_gzip_filter_module
-    sed "s/$HTTP_GZIP_FILTER_MODULE/$HTTP_GZIP_FILTER_MODULE $ngx_addon_name/")
+else
-  # Make the etag header filter run immediately after gzip.
+  next=ngx_http_range_header_filter_module
-  HTTP_FILTER_MODULES=$(echo $HTTP_FILTER_MODULES |\
+fi
-    sed "s/$HTTP_GZIP_FILTER_MODULE/ngx_pagespeed_etag_filter $HTTP_GZIP_FILTER_MODULE/")
+
+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
+      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 the pagespeed optimization library
+$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 $PWD/$NGX_AUTOCONF_ERR for more details.
 END
   exit 1
 fi
 
 have=NGX_PAGESPEED . auto/have
+
+CC_TEST_FLAGS="$CC_OLD_TEST_FLAGS"

config.make

@@ -0,0 +1,7 @@
+# Since nginx build system doesn't normally do C++, there is no CXXFLAGS for us
+# to touch, and compilers are understandably unhappy with --std=c++11 on C
+# files. Hence, we hack the makefile to add it for just our sources.
+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

scripts/build_ngx_pagespeed.sh

@@ -0,0 +1,802 @@
+#!/bin/bash
+#
+# Copyright 2016 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.
+
+function usage() {
+  echo "
+Usage: build_ngx_pagespeed.sh [options]
+
+  Installs ngx_pagespeed and its dependencies.  Can optionally build and install
+  nginx as well.  Can be run either as:
+
+     bash <(curl -f -L -sS https://ngxpagespeed.com/install) [options]
+
+  Or:
+
+     git clone git@github.com:pagespeed/ngx_pagespeed.git
+     cd ngx_pagespeed/
+     git checkout <branch>
+     scripts/build_ngx_pagespeed.sh [options]
+
+Options:
+  -v, --ngx-pagespeed-version <ngx_pagespeed version>
+      What version of ngx_pagespeed to build.  Valid options include:
+      * latest-beta
+      * latest-stable
+      * a version number, such as 1.11.33.4
+
+      If you don't specify a version, defaults to latest-stable unless --devel
+      is specified, in which case it defaults to master.
+
+      This option doesn't make sense if we're running within an existing
+      ngx_pagespeed checkout.
+
+  -n, --nginx-version <nginx version>
+      What version of nginx to build.  If not set, this script only prepares the
+      ngx_pagespeed module, and expects you to handle including it when you
+      build nginx.
+
+      If you pass in 'latest' then this script scrapes the nginx download page
+      and attempts to determine the latest version automatically.
+
+  -m, --dynamic-module
+      Build ngx_pagespeed as a dynamic module.
+
+  -b, --builddir <directory>
+      Where to build.  Defaults to \$HOME.
+
+  -p, --no-deps-check
+      By default, this script checks for the packages it depends on and tries to
+      install them.  If you have installed dependencies from source or are on a
+      non-deb non-rpm system, this won't work.  In that case, install the
+      dependencies yourself and pass --no-deps-check.
+
+  -s, --psol-from-source
+      Build PSOL from source instead of downloading a pre-built binary module.
+
+  -l, --devel
+      Sets up a development environment in ngx_pagespeed/nginx, building with
+      testing-only dependencies.  Includes --psol-from-source, conflicts with
+      --nginx-version.  Uses a 'git clone' checkout for ngx_pagespeed and nginx
+      instead of downloading a tarball.
+
+  -t, --build-type
+      When building PSOL from source, what to tell it for BUILD_TYPE.  Defaults
+      to 'Release' unless --devel is set in which case it defaults to 'Debug'.
+
+  -y, --assume-yes
+      Assume the answer to all prompts is 'yes, please continue'.  Intended for
+      automated usage, such as buildbots.
+
+  -d, --dryrun
+      Don't make any changes to the system, just print what changes you
+      would have made.
+
+  -h, --help
+      Print this message and exit."
+}
+
+RED=31
+GREEN=32
+YELLOW=33
+function begin_color() {
+  color="$1"
+  echo -e -n "\e[${color}m"
+}
+function end_color() {
+  echo -e -n "\e[0m"
+}
+
+function echo_color() {
+  color="$1"
+  shift
+  begin_color "$color"
+  echo "$@"
+  end_color
+}
+
+function error() {
+  local error_message="$@"
+  echo_color "$RED" -n "Error: " >&2
+  echo "$@" >&2
+}
+
+# Prints an error message and exits with an error code.
+function fail() {
+  error "$@"
+
+  # Normally I'd use $0 in "usage" here, but since most people will be running
+  # this via curl, that wouldn't actually give something useful.
+  echo >&2
+  echo "For usage information, run this script with --help" >&2
+  exit 1
+}
+
+
+function status() {
+  echo_color "$GREEN" "$@"
+}
+
+# Intended to be called as:
+#   bash <(curl dl.google.com/.../build_ngx_pagespeed.sh) <args>
+
+# If we set -e or -u then users of this script will see it silently exit on
+# failure.  Instead we need to check the exit status of each command manually.
+# The run function handles exit-status checking for system-changing commands.
+# Additionally, this allows us to easily have a dryrun mode where we don't
+# actually make any changes.
+INITIAL_ENV=$(printenv | sort)
+function run() {
+  if "$DRYRUN"; then
+    echo_color "$YELLOW" -n "would run"
+    echo " $@"
+    env_differences=$(comm -13 <(echo "$INITIAL_ENV") <(printenv | sort))
+    if [ -n "$env_differences" ]; then
+      echo "  with the following additional environment variables:"
+      echo "$env_differences" | sed 's/^/    /'
+    fi
+  else
+    if ! "$@"; then
+      error "Failure running '$@', exiting."
+      exit 1
+    fi
+  fi
+}
+
+function redhat_is_installed() {
+  local package_name="$1"
+  rpm -qa $package_name | grep -q .
+}
+
+function debian_is_installed() {
+  local package_name="$1"
+  dpkg -l $package_name | grep ^ii | grep -q .
+}
+
+function version_sort() {
+  # We'd rather use sort -V, but that's not available on Centos 5.  This works
+  # for versions in the form A.B.C.D or shorter, which is enough for our use.
+  sort -t '.' -k 1,1 -k 2,2 -k 3,3 -k 4,4 -g
+}
+
+# Compare two numeric versions in the form "A.B.C".  Works with version numbers
+# having up to four components, since that's enough to handle both nginx (3) and
+# ngx_pagespeed (4).
+function version_older_than() {
+  local test_version="$1"
+  local compare_to="$2"
+
+  local older_version=$(echo $@ | tr ' ' '\n' | version_sort | head -n 1)
+  test "$older_version" != "$compare_to"
+}
+
+function determine_latest_nginx_version() {
+  # Scrape nginx's download page to try to find the most recent nginx version.
+
+  nginx_download_url="https://nginx.org/en/download.html"
+  function report_error() {
+    fail "
+Couldn't automatically determine the latest nginx version: failed to $@
+$nginx_download_url"
+  }
+
+  nginx_download_page=$(curl -sS --fail "$nginx_download_url") || \
+    report_error "download"
+
+  download_refs=$(echo "$nginx_download_page" | \
+    grep -o '/download/nginx-[0-9.]*[.]tar[.]gz') || \
+    report_error "parse"
+
+  versions_available=$(echo "$download_refs" | \
+    sed -e 's~^/download/nginx-~~' -e 's~\.tar\.gz$~~') || \
+    report_error "extract versions from"
+
+  latest_version=$(echo "$versions_available" | version_sort | tail -n 1) || \
+    report_error "determine latest version from"
+
+  if version_older_than "$latest_version" "1.11.4"; then
+    fail "
+Expected the latest version of nginx to be at least 1.11.4 but found
+$latest_version on $nginx_download_url"
+  fi
+
+  echo "$latest_version"
+}
+
+# Usage:
+#  install_dependencies install_pkg_cmd is_pkg_installed_cmd dep1 dep2 ...
+#
+# install_pkg_cmd is a command to install a dependency
+# is_pkg_installed_cmd is a command that returns true if the dependency is
+#   already installed
+# each dependency is a package name
+function install_dependencies() {
+  local install_pkg_cmd="$1"
+  local is_pkg_installed_cmd="$2"
+  shift 2
+
+  local missing_dependencies=""
+
+  for package_name in "$@"; do
+    if ! $is_pkg_installed_cmd $package_name; then
+      missing_dependencies+="$package_name "
+    fi
+  done
+  if [ -n "$missing_dependencies" ]; then
+    status "Detected that we're missing the following depencencies:"
+    echo "  $missing_dependencies"
+    status "Installing them:"
+    run sudo $install_pkg_cmd $missing_dependencies
+  fi
+}
+
+function gcc_too_old() {
+  # We need gcc >= 4.8
+  local gcc_major_version=$(gcc -dumpversion | awk -F. '{print $1}')
+  if [ "$gcc_major_version" -lt 4 ]; then
+    return 0  # too old
+  elif [ "$gcc_major_version" -gt 4 ]; then
+    return 1  # plenty new
+  fi
+  # It's gcc 4.x, check if x >= 8:
+  local gcc_minor_version=$(gcc -dumpversion | awk -F. '{print $2}')
+  test "$gcc_minor_version" -lt 8
+}
+
+function continue_or_exit() {
+  if "$ASSUME_YES"; then
+    return
+  fi
+
+  local prompt="$1"
+  echo_color "$YELLOW" -n "$prompt"
+  read -p " [Y/n] " yn
+  if [[ "$yn" == N* || "$yn" == n* ]]; then
+    echo "Cancelled."
+    exit 0
+  fi
+}
+
+# If a string is very simple we don't need to quote it.  But we should quote
+# everything else to be safe.
+function needs_quoting() {
+  echo "$@" | grep -q '[^a-zA-Z0-9./_=-]'
+}
+
+function escape_for_quotes() {
+  echo "$@" | sed -e 's~\\~\\\\~g' -e "s~'~\\\\'~g"
+}
+
+function quote_arguments() {
+  local argument_str=""
+  for argument in "$@"; do
+    if [ -n "$argument_str" ]; then
+      argument_str+=" "
+    fi
+    if needs_quoting "$argument"; then
+      argument="'$(escape_for_quotes "$argument")'"
+    fi
+    argument_str+="$argument"
+  done
+  echo "$argument_str"
+}
+
+function build_ngx_pagespeed() {
+  getopt --test
+  if [ "$?" != 4 ]; then
+    # Even Centos 5 and Ubuntu 10 LTS have new-style getopt, so I don't expect
+    # this to be hit in practice on systems that are actually able to run
+    # PageSpeed.
+    fail "Your version of getopt is too old.  Exiting with no changes made."
+  fi
+
+  opts=$(getopt -o v:n:mb:pslt:ydh \
+    --longoptions ngx-pagespeed-version:,nginx-version:,dynamic-module \
+    --longoptions buildir:,no-deps-check,psol-from-source,devel,build-type: \
+    --longoptions assume-yes,dryrun,help \
+    -n "$(basename "$0")" -- "$@")
+  if [ $? != 0 ]; then
+    usage
+    exit 1
+  fi
+  eval set -- "$opts"
+
+  NPS_VERSION="DEFAULT"
+  NGINX_VERSION=""
+  BUILDDIR="$HOME"
+  DO_DEPS_CHECK=true
+  PSOL_FROM_SOURCE=false
+  DEVEL=false
+  BUILD_TYPE=""
+  ASSUME_YES=false
+  DRYRUN=false
+  DYNAMIC_MODULE=false
+  while true; do
+    case "$1" in
+      -v | --ngx-pagespeed-version) shift
+        NPS_VERSION="$1"
+        shift
+        ;;
+      -n | --nginx-version) shift
+        NGINX_VERSION="$1"
+        shift
+        ;;
+      -m | --dynamic-module) shift
+        DYNAMIC_MODULE=true
+        ;;
+      -b | --builddir) shift
+        BUILDDIR="$1"
+        shift
+        ;;
+      -p | --no-deps-check) shift
+        DO_DEPS_CHECK=false
+        ;;
+      -s | --psol-from-source) shift
+        PSOL_FROM_SOURCE=true
+        ;;
+      -l | --devel) shift
+        DEVEL=true
+        ;;
+      -t | --build-type) shift
+        BUILD_TYPE="$1"
+        shift
+        ;;
+      -y | --assume-yes) shift
+        ASSUME_YES="true"
+        ;;
+      -d | --dryrun) shift
+        DRYRUN="true"
+        ;;
+      -h | --help) shift
+        usage
+        exit 0
+        ;;
+      --) shift
+        break
+        ;;
+      *)
+        echo "Invalid argument: $1"
+        usage
+        exit 1
+        ;;
+    esac
+  done
+
+  USE_GIT_CHECKOUT="$DEVEL"
+  ALREADY_CHECKED_OUT=false
+  if [ -e PSOL_BINARY_URL ]; then
+    status "Detected that we're running in an existing ngx_pagespeed checkout."
+    USE_GIT_CHECKOUT=true
+    ALREADY_CHECKED_OUT=true
+  fi
+
+  if "$ALREADY_CHECKED_OUT"; then
+    if [ "$NPS_VERSION" != "DEFAULT" ]; then
+      fail \
+"The --ngx-pagespeed-version argument doesn't make sense when running within an existing checkout."
+    fi
+  elif [ "$NPS_VERSION" = "DEFAULT" ]; then
+    if "$DEVEL"; then
+      NPS_VERSION="master"
+    else
+      NPS_VERSION="latest-stable"
+    fi
+  fi
+
+  if [ ! -d "$BUILDDIR" ]; then
+    fail "Told to build in $BUILDDIR, but that directory doesn't exist."
+  fi
+
+  BUILD_NGINX=false
+  if [ -n "$NGINX_VERSION" ]; then
+    BUILD_NGINX=true
+  fi
+
+  if "$DEVEL"; then
+    PSOL_FROM_SOURCE=true
+    BUILD_NGINX=true
+    if [ -n "$NGINX_VERSION" ]; then
+      fail \
+"The --devel argument conflicts with --nginx.  In devel mode we use the version of nginx that's included as a submodule."
+    fi
+    if "$DYNAMIC_MODULE"; then
+      fail "Can't currently build a dynamic module in --devel mode."
+    fi
+  fi
+
+  if "$PSOL_FROM_SOURCE" && [ -z "$BUILD_TYPE" ]; then
+    if "$DEVEL"; then
+      BUILD_TYPE="Debug"
+    else
+      BUILD_TYPE="Release"
+    fi
+  elif [ -n "$BUILD_TYPE" ]; then
+    fail "Setting --build-type requires --psol-from-source or --devel."
+  fi
+
+  if [ "$NGINX_VERSION" = "latest" ]; then
+    # When this function fails it prints the debugging information needed first
+    # to stderr.
+    NGINX_VERSION=$(determine_latest_nginx_version) || exit 1
+  fi
+
+  if "$DYNAMIC_MODULE"; then
+    # Check that ngx_pagespeed and nginx are recent enough to support dynamic
+    # modules.  Unfortunately NPS_VERSION might be a tag, in which case we don't
+    # know.  If it's not a numeric version number, then assume it's recent
+    # enough and if it's not they'll get an ugly compilation error later.
+    # Luckily 1.10.33.5 was a while ago now.
+    #
+    # I'd like to use =~ here, but they changed syntax between v3 and v4 (quotes
+    # moved from mandatory to optional to prohibited).
+    if [[ "${NPS_VERSION#*[^0-9.]}" = "$NPS_VERSION" ]] &&
+         version_older_than "$NPS_VERSION" "1.10.33.5"; then
+      fail "
+You're trying to build ngx_pagespeed $NPS_VERSION as a dynamic module, but
+ngx_pagespeed didn't add support for dynamic modules until 1.10.33.5."
+    fi
+
+    if [ ! -z "NGINX_VERSION" ]; then
+      if version_older_than "$NGINX_VERSION" "1.9.13"; then
+        fail "
+You're trying to build nginx $NGINX_VERSION as a dynamic module but nginx didn't
+add support for dynamic modules in a way compatible with ngx_pagespeed until
+1.9.13."
+      fi
+    fi
+  fi
+
+  if "$DRYRUN"; then
+    TEMPDIR="/tmp/output-of-mktemp"
+  else
+    TEMPDIR=$(mktemp -d)
+    function cleanup_tempdir {
+      rm -rf "$TEMPDIR"
+    }
+    trap cleanup_tempdir EXIT
+  fi
+
+  extra_flags=()
+  # Now make sure our dependencies are installed.
+  if "$DO_DEPS_CHECK"; then
+    if [ -f /etc/debian_version ]; then
+      status "Detected debian-based distro."
+
+      install_dependencies "apt-get install" debian_is_installed \
+        "build-essential zlib1g-dev libpcre3 libpcre3-dev unzip"
+
+      if gcc_too_old; then
+        if [ ! -e /usr/lib/gcc-mozilla/bin/gcc ]; then
+          status "Detected that gcc is older than 4.8.  Installing gcc-mozilla"
+          status "which installs gcc-4.8 into /usr/lib/gcc-mozilla/ and doesn't"
+          status "affect your global gcc installation."
+          run sudo apt-get install gcc-mozilla
+        fi
+
+        extra_flags=("--with-cc=/usr/lib/gcc-mozilla/bin/gcc" \
+                     "--with-ld-opt=-static-libstdc++")
+      fi
+
+    elif [ -f /etc/redhat-release ]; then
+      status "Detected redhat-based distro."
+
+      install_dependencies "yum install" redhat_is_installed \
+        "gcc-c++ pcre-devel zlib-devel make unzip wget"
+      if gcc_too_old; then
+        if [ ! -e /opt/rh/devtoolset-2/root/usr/bin/gcc ]; then
+          redhat_major_version=$(
+            cat /etc/redhat-release | grep -o -E '[0-9]+' | head -n 1)
+          if [ "$redhat_major_version" == 5 ]; then
+            slc_version=5
+          elif [ "$redhat_major_version" == 6 ]; then
+            slc_version=6
+          else
+            fail "
+Unexpected major version $redhat_major_version in /etc/redhat-release:
+$(cat /etc/redhat-release) Expected 5 or 6."
+          fi
+
+          status "Detected that gcc is older than 4.8.  Scientific Linux"
+          status "provides a gcc package that installs gcc-4.8 into /opt/ and"
+          status "doesn't affect your global gcc installation."
+          slc_key="https://linux.web.cern.ch/linux/scientific6/docs/repository/"
+          slc_key+="cern/slc6X/i386/RPM-GPG-KEY-cern"
+          slc_key_out="$TEMPDIR/RPM-GPG-KEY-cern"
+          run sudo wget "$slc_key" -O "$slc_key_out"
+          run sudo rpm --import "$slc_key_out"
+
+          repo_fname="/etc/yum.repos.d/slc${slc_version}-devtoolset.repo"
+          if [ -e "$repo_fname" ]; then
+            fail "Expected $repo_fname not to exist; aborting."
+          fi
+
+          repo_url="https://linux.web.cern.ch/linux/scientific${slc_version}/"
+          repo_url+="/docs/repository/cern/devtoolset/"
+          repo_url+="slc${slc_version}-devtoolset.repo"
+          run sudo wget -O "$repo_fname" "$repo_url"
+          run sudo yum install devtoolset-2-gcc-c++ devtoolset-2-binutils
+        fi
+        extra_flags=("--with-cc=/opt/rh/devtoolset-2/root/usr/bin/gcc")
+      fi
+    else
+      fail "
+This doesn't appear to be a deb-based distro or an rpm-based one.  Not going to
+be able to install dependencies.  Please install dependencies manually and rerun
+with --no-deps-check."
+    fi
+    status "Operating system dependencies are all set."
+  else
+    status "Not checking whether operating system dependencies are installed."
+  fi
+
+  function delete_if_already_exists() {
+    if "$DRYRUN"; then return; fi
+
+    local directory="$1"
+    if [ -d "$directory" ]; then
+      if [ ${#directory} -lt 8 ]; then
+        fail "
+Not deleting $directory; name is suspiciously short.  Something is wrong."
+      fi
+
+      continue_or_exit "OK to delete $directory?"
+      run rm -rf "$directory"
+    fi
+  }
+
+  # In general, the zip github builds for tag foo unzips to ngx_pagespeed-foo,
+  # but it looks like they special case vVERSION tags to ngx_pagespeed-VERSION
+  if [[ "$NPS_VERSION" =~ ^[0-9]*[.][0-9]*[.][0-9]*[.][0-9]*$ ]]; then
+    # We've been given a numeric version number.  This has an associated tag
+    # in the form vVERSION-beta.
+    tag_name="v${NPS_VERSION}-beta"
+    nps_downloaded_fname="ngx_pagespeed-${NPS_VERSION}-beta"
+  else
+    # We've been given a tag name, like latest-beta.  Download that directly.
+    tag_name="$NPS_VERSION"
+    nps_downloaded_fname="ngx_pagespeed-${NPS_VERSION}"
+  fi
+
+  install_dir="this-only-makes-sense-in-devel-mode"
+  if "$USE_GIT_CHECKOUT"; then
+    # We're either doing a --devel build, or someone is running us from an
+    # existing git checkout.
+    nps_module_dir="$PWD"
+    install_dir="$nps_module_dir"
+    if "$ALREADY_CHECKED_OUT"; then
+      run cd "$nps_module_dir"
+    else
+      status "Checking out ngx_pagespeed..."
+      run git clone "git@github.com:pagespeed/ngx_pagespeed.git" \
+                    "$nps_module_dir"
+      run cd "$nps_module_dir"
+      run git checkout "$tag_name"
+    fi
+    submodules_dir="$nps_module_dir/testing-dependencies"
+    if "$DEVEL"; then
+      status "Downloading dependencies..."
+      run git submodule update --init --recursive
+      if [[ "$CONTINUOUS_INTEGRATION" != true ]]; then
+        status "Switching submodules over to git protocol."
+        # This lets us push to github by public key.
+        for config in $(find .git/ -name config) ; do
+          run sed -i s~https://github.com/~git@github.com:~ $config ;
+        done
+      fi
+    fi
+  else
+    nps_baseurl="https://github.com/pagespeed/ngx_pagespeed/archive"
+    nps_downloaded="$TEMPDIR/$nps_downloaded_fname.zip"
+    status "Downloading ngx_pagespeed..."
+    run wget "$nps_baseurl/$tag_name.zip" -O "$nps_downloaded"
+    nps_module_dir="$BUILDDIR/$nps_downloaded_fname"
+    delete_if_already_exists "$nps_module_dir"
+    status "Extracting ngx_pagespeed..."
+    run unzip -q "$nps_downloaded" -d "$BUILDDIR"
+    run cd "$nps_module_dir"
+  fi
+
+  MOD_PAGESPEED_DIR=""
+  PSOL_BINARY=""
+  if "$PSOL_FROM_SOURCE"; then
+    MOD_PAGESPEED_DIR="$PWD/testing-dependencies/mod_pagespeed"
+    git submodule update --init --recursive -- "$MOD_PAGESPEED_DIR"
+    run pushd "$MOD_PAGESPEED_DIR"
+
+    if "$DEVEL"; then
+      if [ ! -d "$HOME/apache2" ]; then
+        run install/build_development_apache.sh 2.2 prefork
+      fi
+      cd devel
+      run make apache_debug_psol
+      PSOL_BINARY="$MOD_PAGESPEED_DIR/out/$BUILD_TYPE/pagespeed_automatic.a"
+    else
+      run install/build_psol.sh --skip_tests --skip_packaging
+      PSOL_BINARY="$MOD_PAGESPEED_DIR/pagespeed/automatic/pagespeed_automatic.a"
+    fi
+    run popd
+  else
+    # Now we need to figure out what precompiled version of PSOL to build
+    # ngx_pagespeed against.
+    if "$DRYRUN"; then
+      psol_url="https://psol.example.com/cant-get-psol-version-in-dry-run.tar.gz"
+    elif [ -e PSOL_BINARY_URL ]; then
+      # Releases after 1.11.33.4 there is a PSOL_BINARY_URL file that tells us
+      # where to look.
+      psol_url="$(scripts/format_binary_url.sh PSOL_BINARY_URL)"
+      if [[ "$psol_url" != https://* ]]; then
+        fail "Got bad psol binary location information: $psol_url"
+      fi
+    else
+      # For past releases we have to grep it from the config file.  The url has
+      # always looked like this, and the config file has contained it since
+      # before we started tagging our ngx_pagespeed releases.
+      psol_url="$(grep -o \
+          "https://dl.google.com/dl/page-speed/psol/[0-9.]*.tar.gz" config)"
+      if [ -z "$psol_url" ]; then
+        fail "Couldn't find PSOL url in $PWD/config"
+      fi
+    fi
+
+    status "Downloading PSOL binary..."
+    run wget "$psol_url"
+
+    status "Extracting PSOL..."
+    run tar -xzf $(basename "$psol_url")  # extracts to psol/
+  fi
+
+  if "$DYNAMIC_MODULE"; then
+    add_module="--add-dynamic-module=$nps_module_dir"
+  else
+    add_module="--add-module=$nps_module_dir"
+  fi
+  configure_args=("$add_module" "${extra_flags[@]}")
+
+  if "$DEVEL"; then
+    configure_args=("${configure_args[@]}"
+                    "--prefix=$install_dir/nginx"
+                    "--add-module=$submodules_dir/ngx_cache_purge"
+                    "--add-module=$submodules_dir/ngx_devel_kit"
+                    "--add-module=$submodules_dir/set-misc-nginx-module"
+                    "--add-module=$submodules_dir/headers-more-nginx-module"
+                    "--with-ipv6"
+                    "--with-http_v2_module")
+    if [ "$BUILD_TYPE" = "Debug" ]; then
+      configure_args=("${configure_args[@]}" "--with-debug")
+    fi
+  fi
+
+  echo
+  if ! "$BUILD_NGINX"; then
+    # Just prepare the module for them to install.
+    status "ngx_pagespeed is ready to be built against nginx."
+    echo "When running ./configure:"
+    if "$PSOL_FROM_SOURCE"; then
+      echo "  Set the following environment variables:"
+      echo "    MOD_PAGESPEED_DIR=$MOD_PAGESPEED_DIR"
+      echo "    PSOL_BINARY=$PSOL_BINARY"
+    fi
+    echo "  Give ./configure the following arguments:"
+    echo "    $(quote_arguments "${configure_args[@]}")"
+    echo
+    if [ ${#extra_flags[@]} -eq 0 ]; then
+      echo "If this is for integration with an already-built nginx, make sure"
+      echo "to include any other arguments you originally passed to"
+      echo "./configure.  You can see these with 'nginx -V'."
+    else
+      echo "Note: because we need to set $(quote_arguments "${extra_flags[@]}")"
+      echo "on this platform, if you want to integrate ngx_pagespeed with an"
+      echo "already-built nginx you're going to need to rebuild your nginx with"
+      echo "those flags set."
+    fi
+  else
+    if "$DEVEL"; then
+      # Use the nginx we loaded as a submodule
+      nginx_dir="$submodules_dir/nginx"
+      configure_location="auto"
+    else
+      # Download and build the specified nginx version.
+      nginx_leaf="nginx-${NGINX_VERSION}.tar.gz"
+      nginx_fname="$TEMPDIR/$nginx_leaf"
+      status "Downloading nginx..."
+      run wget "http://nginx.org/download/$nginx_leaf" -O "$nginx_fname"
+      nginx_dir="$BUILDDIR/nginx-${NGINX_VERSION}/"
+      delete_if_already_exists "$nginx_dir"
+      status "Extracting nginx..."
+      run tar -xzf "$nginx_fname" --directory "$BUILDDIR"
+      configure_location="."
+    fi
+    run cd "$nginx_dir"
+
+    configure=("$configure_location/configure" "${configure_args[@]}")
+    if ! "$ASSUME_YES"; then
+      echo "About to build nginx.  Do you have any additional ./configure"
+      echo "arguments you would like to set?  For example, if you would like"
+      echo "to build nginx with https support give --with-http_ssl_module"
+      echo "If you don't have any, just press enter."
+      read -p "> " additional_configure_args
+      if [ -n "$additional_configure_args" ]; then
+        # Split additional_configure_args respecting any internal quotation.
+        # Otherwise things like --with-cc-opt='-foo -bar' won't work.
+        eval additional_configure_args=("$additional_configure_args")
+        configure=("${configure[@]}" "${additional_configure_args[@]}")
+      fi
+    fi
+    echo "About to configure nginx with:"
+    echo "   $(quote_arguments "${configure[@]}")"
+    continue_or_exit "Does this look right?"
+    MOD_PAGESPEED_DIR="$MOD_PAGESPEED_DIR" \
+      PSOL_BINARY="$PSOL_BINARY" \
+      run "${configure[@]}"
+
+    if ! "$DEVEL"; then
+      continue_or_exit "Build nginx?"
+    fi
+    run make
+
+    if "$DEVEL"; then
+      run make install
+
+      status "Nginx installed with ngx_pagespeed, and set up for testing."
+      # TODO(jefftk): pull these out into separate scripts.
+      echo "To run tests, pick a pair of ports like 8050 and 8051 and run:"
+      echo "  cd $nps_module_dir"
+      echo "  RUN_TESTS=true \\"
+      echo "  USE_VALGRIND=false \\"
+      echo "  TEST_NATIVE_FETCHER=false \\"
+      echo "  TEST_SERF_FETCHER=true \\"
+      echo "  test/run_tests.sh \\"
+      echo "    $MOD_PAGESPEED_DIR \\"
+      echo "    $install_dir/nginx/sbin/nginx"
+      echo
+      echo "To rebuild after changes:"
+      echo "  First, if you change things in PSOL or update it:"
+      echo "    cd $MOD_PAGESPEED_DIR/devel"
+      echo "    make apache_debug_psol"
+      echo "  Then, whether or not you updated PSOL, rebuild nginx:"
+      echo "    cd $install_dir/nginx"
+      echo "    make && make install"
+    else
+      continue_or_exit "Install nginx?"
+      run sudo make install
+
+      echo
+      if "$DYNAMIC_MODULE"; then
+        echo "Nginx installed with ngx_pagespeed support available as a"
+        echo "loadable module."
+        echo
+        echo "To load the ngx_pagespeed module, you'll need to add:"
+        echo "  load_module \"modules/ngx_pagespeed.so\";"
+        echo "at the top of your main nginx configuration file."
+      else
+        echo "Nginx installed with ngx_pagespeed support compiled-in."
+      fi
+      echo
+      echo "If this is a new installation you probably need an init script to"
+      echo "manage starting and stopping the nginx service.  See:"
+      echo "  http://wiki.nginx.org/InitScripts"
+      echo
+      echo "You'll also need to configure ngx_pagespeed if you haven't yet:"
+      echo "  https://developers.google.com/speed/pagespeed/module/configuration"
+    fi
+  fi
+  if "$DRYRUN"; then
+    echo_color "$YELLOW" "[this was a dry run; your system is unchanged]"
+  fi
+}
+
+# Start running things from a call at the end so if this script is executed
+# after a partial download it doesn't do anything.
+build_ngx_pagespeed "$@"

scripts/format_binary_url.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+#
+# Copyright 2016 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.
+
+set -e
+set -u
+
+if [ $# -ne 1 ]; then
+  echo "Usage: $(basename $0) <url_file>" >&2
+  exit 1
+fi
+
+url_file=$1
+
+if [ ! -e "$url_file" ]; then
+  echo "Url file '$url_file' missing!" >&2
+fi
+
+# The size names must match install/build_psol.sh in mod_pagespeed
+if [ "$(uname -m)" = x86_64 ]; then
+  bit_size_name=x64
+else
+  bit_size_name=ia32
+fi
+
+sed -e 's/$BIT_SIZE_NAME\b/'$bit_size_name'/g' $url_file

scripts/pagespeed_libraries_generator.sh

@@ -0,0 +1,36 @@
+#!/bin/bash
+#
+# Copyright 2013 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.
+#
+# Converts pagespeed_libraries.conf from Apache-format to Nginx-format,
+# supporting the canonicalize_javascript_libraries filter.
+# Inspired by https://github.com/pagespeed/ngx_pagespeed/issues/532
+#
+# Usage:
+#   scripts/pagespeed_libraries_generator.sh > pagespeed_libraries.conf
+#
+# Then have nginx include that configuration file and enable the filter
+# canonicalize_javascript_libraries.
+#
+# Author: vid@zippykid.com (Vid Luther)
+#         jefftk@google.com (Jeff Kaufman)
+
+URL="https://github.com/pagespeed/mod_pagespeed/raw/master/"
+URL+="net/instaweb/genfiles/conf/pagespeed_libraries.conf"
+curl -L -s -S "$URL" \
+    | grep ModPagespeedLibrary \
+    | while read library size hash url ; do
+  echo "  pagespeed Library $size $hash $url;"
+done

scripts/prepare_psol.sh

@@ -1,91 +0,0 @@
-#!/bin/bash
-#
-# Copyright 2012 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: jefftk@google.com (Jeff Kaufman)
-#
-# Usage:
-#   scripts/prepare_psol.sh /path/to/mod_pagespeed/src
-#
-# Creates a directory psol/ and copies headers and a few source files from a
-# depot_tools (glient) checkout into psol/include.  Along with creating
-# binaries, this is a step in preparing psol.tar.gz for distribution.
-#
-
-set -u  # check for undefined variables
-set -e  # exit on failed commands
-
-if [ "$(basename "$PWD")" != "ngx_pagespeed" ] ; then
-  echo "$(basename $0) must be invoked from the ngx_pagespeed directory"
-  exit 1
-fi
-
-if [ $# -ne 1 ] ; then
-  echo "Usage: $(basename $0) /path/to/mod_pagespeed/src"
-  exit 1
-fi
-
-MOD_PAGESPEED_SRC="$1"
-
-if [ "$(basename "$(dirname "$MOD_PAGESPEED_SRC")")/$( \
-        basename "$MOD_PAGESPEED_SRC")" != "mod_pagespeed/src" ] ; then
-  echo "Usage: $(basename $0) /path/to/mod_pagespeed/src"
-  exit 1
-fi
-
-if [ -e psol ] ; then
-  echo "A psol/ directory already exists.  Move it somewhere else and rerun."
-  exit 1
-fi
-mkdir psol/
-# Copy over the .h files, plus a few selected .cc and .c files.
-rsync -arvz "$MOD_PAGESPEED_SRC/" "psol/include/" --prune-empty-dirs \
-  --exclude=".svn" \
-  --exclude=".git" \
-  --include='*.h' \
-  --include='*/' \
-  --include="apr_thread_compatible_pool.cc" \
-  --include="serf_url_async_fetcher.cc" \
-  --include="apr_mem_cache.cc" \
-  --include="key_value_codec.cc" \
-  --include="apr_memcache2.c" \
-  --include="loopback_route_fetcher.cc" \
-  --include="add_headers_fetcher.cc" \
-  --include="console_css_out.cc" \
-  --include="console_out.cc" \
-  --include="dense_hash_map" \
-  --include="dense_hash_set" \
-  --include="sparse_hash_map" \
-  --include="sparse_hash_set" \
-  --include="sparsetable" \
-  --exclude='*'
-mkdir -p psol/lib/Debug/linux/ia32
-mkdir -p psol/lib/Debug/linux/x64
-mkdir -p psol/lib/Release/linux/ia32
-mkdir -p psol/lib/Release/linux/x64
-
-# Log that we did this.
-SVN_REVISION="$(svn info $MOD_PAGESPEED_SRC | grep Revision | awk '{print $2}')"
-SVN_TAG="$(svn info $MOD_PAGESPEED_SRC | grep URL |  awk -F/ '{print $(NF-1)}')"
-
-DATE="$(date +%F)"
-echo "${DATE}: Copied from mod_pagespeed ${SVN_TAG}@r${SVN_REVISION} ($USER)" \
-  >> psol/include_history.txt
-
-echo
-echo "Output is in psol/include.  Now put binaries in psol/lib following"
-echo "https://github.com/pagespeed/ngx_pagespeed/wiki/Building-Release-Binaries"
-echo "and then you can distribute PSOL."
-

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());
 
@@ -98,15 +98,13 @@ namespace net_instaweb {
 namespace log_message_handler {
 
 
-const int kDebugLogLevel = -2;
-
 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,38 +12,193 @@
  * 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/rewriter/public/rewrite_driver.h"
-
+#include "net/instaweb/rewriter/public/rewrite_options.h"
-#include "net/instaweb/http/public/response_headers.h"
 #include "net/instaweb/rewriter/public/rewrite_stats.h"
-#include "net/instaweb/util/public/google_message_handler.h"
+#include "pagespeed/kernel/base/google_message_handler.h"
-#include "net/instaweb/util/public/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)
+                           const RequestContextPtr& request_ctx,
+                           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),
+      need_flush_(false),
       done_called_(false),
       last_buf_sent_(false),
-      pipe_fd_(pipe_fd),
+      references_(2),
-      references_(2) {
+      base_fetch_type_(base_fetch_type),
+      preserve_caching_headers_(preserve_caching_headers),
+      detached_(false),
+      suppress_(false) {
   if (pthread_mutex_init(&mutex_, NULL)) CHECK(0);
-  PopulateRequestHeaders();
+  __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() {
@@ -54,45 +209,6 @@ void NgxBaseFetch::Unlock() {
   pthread_mutex_unlock(&mutex_);
 }
 
-void NgxBaseFetch::PopulateRequestHeaders() {
-  CopyHeadersFromTable<RequestHeaders>(&request_->headers_in.headers,
-                                       request_headers());
-}
-
-void NgxBaseFetch::PopulateResponseHeaders() {
-  CopyHeadersFromTable<ResponseHeaders>(&request_->headers_out.headers,
-                                        response_headers());
-
-  response_headers()->set_status_code(request_->headers_out.status);
-
-  // Manually copy over the content type because it's not included in
-  // request_->headers_out.headers.
-  response_headers()->Add(
-      HttpAttributes::kContentType,
-      ngx_psol::str_to_string_piece(request_->headers_out.content_type));
-
-  // TODO(oschaaf): ComputeCaching should be called in setupforhtml()?
-  response_headers()->ComputeCaching();
-}
-
-template<class HeadersT>
-void NgxBaseFetch::CopyHeadersFromTable(ngx_list_t* headers_from,
-                                        HeadersT* headers_to) {
-  // http_version is the version number of protocol; 1.1 = 1001. See
-  // NGX_HTTP_VERSION_* in ngx_http_request.h
-  headers_to->set_major_version(request_->http_version / 1000);
-  headers_to->set_minor_version(request_->http_version % 1000);
-
-  ngx_table_elt_t* header;
-  NgxListIterator it(&headers_from->part);
-  while ((header = it.Next()) != NULL) {
-    StringPiece key = ngx_psol::str_to_string_piece(header->key);
-    StringPiece value = ngx_psol::str_to_string_piece(header->value);
-
-    headers_to->Add(key, value);
-  }
-}
-
 bool NgxBaseFetch::HandleWrite(const StringPiece& sp,
                                MessageHandler* handler) {
   Lock();
@@ -101,13 +217,22 @@ bool NgxBaseFetch::HandleWrite(const StringPiece& sp,
   return true;
 }
 
+// should only be called in nginx thread
 ngx_int_t NgxBaseFetch::CopyBufferToNginx(ngx_chain_t** link_ptr) {
-  if (done_called_ && last_buf_sent_) {
+  CHECK(!(done_called_ && last_buf_sent_))
-    return NGX_DECLINED;
+        << "CopyBufferToNginx() was called after the last buffer was sent";
+
+  // there is no buffer to send
+  if (!done_called_ && buffer_.empty()) {
+    *link_ptr = NULL;
+    return NGX_AGAIN;
   }
 
-  int rc = ngx_psol::string_piece_to_buffer_chain(
+  int rc = string_piece_to_buffer_chain(request_->pool, buffer_, link_ptr,
-      request_->pool, buffer_, link_ptr, done_called_ /* send_last_buf */);
+                                        done_called_ /* send_last_buf */,
+                                        need_flush_);
+  need_flush_ = false;
+
   if (rc != NGX_OK) {
     return rc;
   }
@@ -117,87 +242,119 @@ ngx_int_t NgxBaseFetch::CopyBufferToNginx(ngx_chain_t** link_ptr) {
 
   if (done_called_) {
     last_buf_sent_ = true;
+    return NGX_OK;
   }
 
-  return NGX_OK;
+  return NGX_AGAIN;
 }
 
 // There may also be a race condition if this is called between the last Write()
 // and Done() such that we're sending an empty buffer with last_buf set, which I
 // think nginx will reject.
 ngx_int_t NgxBaseFetch::CollectAccumulatedWrites(ngx_chain_t** link_ptr) {
+  ngx_int_t rc;
   Lock();
-  ngx_int_t rc = CopyBufferToNginx(link_ptr);
+  rc = CopyBufferToNginx(link_ptr);
   Unlock();
-
-  if (rc == NGX_DECLINED) {
-    *link_ptr = NULL;
-    return NGX_OK;
-  }
   return rc;
 }
 
 ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
-  Lock();
+  // 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();
-  Unlock();
+
-  return ngx_psol::copy_response_headers_to_ngx(request_, *pagespeed_headers);
+  if (content_length_known()) {
+     headers_out->content_length = NULL;
+     headers_out->content_length_n = content_length();
   }
 
-void NgxBaseFetch::RequestCollection() {
+  return copy_response_headers_to_ngx(request_, *pagespeed_headers,
-  int rc;
+                                      preserve_caching_headers_);
-  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?
-
-    } else {
-      perror("NgxBaseFetch::RequestCollection");
-      break;
 }
+
+void NgxBaseFetch::RequestCollection(char type) {
+  if (suppress_) {
+    return;
+  }
+
+  // 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();
   }
 }
 
 void NgxBaseFetch::HandleHeadersComplete() {
+  int status_code = response_headers()->status_code();
+  bool status_ok = (status_code != 0) && (status_code < 400);
+
+  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);
     }
+  }
 
-  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.
+  Lock();
+  need_flush_ = true;
+  Unlock();
+  RequestCollection(kFlush);  // A new part of the response body is available
   return true;
 }
 
-void NgxBaseFetch::Release() {
+int NgxBaseFetch::DecrementRefCount() {
-  DecrefAndDeleteIfUnreferenced();
+  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();
-
+  RequestCollection(kDone);
-  close(pipe_fd_);  // Indicates to nginx that we're done with the rewrite.
-  pipe_fd_ = -1;
-
   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
+//  - When HandleHeadersComplete(), HandleFlush(), or HandleDone() is called by
-//    watching so nginx knows to call CollectAccumulatedWrites() to pick up the
+//    PSOL, events are written to NgxEventConnection which will end up being
-//    rewritten html.
+//    handled by ReadCallback() on nginx's thread.
-//  - When Done() is called the base fetch closes the pipe, which tells nginx to
+//    When applicable, request processing will be continued via a call to
-//    make a final call to CollectAccumulatedWrites().
+//    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
+// This class is referred to in three places: the proxy fetch, nginx's request,
-// request.  It must stay alive until both are finished.  The proxy fetch will
+// and pending events written to the associated NgxEventConnection. It must stay
-// call Done() to indicate this; nginx will call Release().  Once both Done()
+// alive until the proxy fetch and nginx request are finished, and no more
-// and Release() have been called this class will delete itself.
+// 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,26 +58,45 @@ 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/rewriter/public/rewrite_options.h"
-#include "net/instaweb/util/public/string.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);
+               const RequestContextPtr& request_ctx,
+               PreserveCachingHeaders preserve_caching_headers,
+               NgxBaseFetchType base_fetch_type,
+               const RewriteOptions* options);
   virtual ~NgxBaseFetch();
 
-  // Copies the request headers out of request_->headers_in->headers.
+  // Statically initializes event_connection, require for PSOL and nginx to
-  void PopulateRequestHeaders();
+  // communicate.
+  static bool Initialize(ngx_cycle_t* cycle);
 
-  // Copies the response headers out of request_->headers_out->headers.
+  // Attempts to finish up request processing queued up in the named pipe and
-  void PopulateResponseHeaders();
+  // 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
@@ -82,8 +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.
+  // Called by nginx to decrement the refcount.
-  void Release();
+  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_; }
+
+  bool IsCachedResultValid(const ResponseHeaders& headers) override;
 
  private:
   virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler);
@@ -91,18 +139,15 @@ class NgxBaseFetch : public AsyncFetch {
   virtual void HandleHeadersComplete();
   virtual void HandleDone(bool success);
 
-  // Helper method for PopulateRequestHeaders and PopulateResponseHeaders.
-  template<class HeadersT>
-  void CopyHeadersFromTable(ngx_list_t* headers_from, HeadersT* headers_to);
-
   // Indicate to nginx that we would like it to call
   // CollectAccumulatedWrites().
-  void RequestCollection();
+  void RequestCollection(char type);
 
   // Lock must be acquired first.
   // Returns:
-  //   NGX_DECLINED: nothing to send, short circuit.  Buffer not allocated.
+  //   NGX_ERROR: failure
-  //   NGX_OK, NGX_ERROR: success, failure
+  //   NGX_AGAIN: still has buffer to send, need to checkout link_ptr
+  //   NGX_OK: done, HandleDone has been called
   // Allocates an nginx buffer, copies our buffer_ contents into it, clears
   // buffer_.
   ngx_int_t CopyBufferToNginx(ngx_chain_t** link_ptr);
@@ -112,18 +157,32 @@ 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 need_flush_;
   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_;
+  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_caching_headers.cc

@@ -23,17 +23,15 @@
 
 namespace net_instaweb {
 
-bool NgxCachingHeaders::Lookup(const GoogleString& key,
+bool NgxCachingHeaders::Lookup(const StringPiece& key,
                                StringPieceVector* values) {
   ngx_table_elt_t* header;
   NgxListIterator it(&(request_->headers_out.headers.part));
   while ((header = it.Next()) != NULL) {
-    if (header->hash != 0 &&
+    if (header->hash != 0 && key == str_to_string_piece(header->key)) {
-        key == ngx_psol::str_to_string_piece(header->key)) {
       // This will be called multiple times if there are multiple headers with
       // this name.  Each time it will append to values.
-      SplitStringPieceToVector(
+      SplitStringPieceToVector(str_to_string_piece(header->value), ",", values,
-          ngx_psol::str_to_string_piece(header->value), ",", values,
                                true /* omit empty strings */);
     }
   }

src/ngx_caching_headers.h

@@ -37,7 +37,7 @@ class NgxCachingHeaders : public CachingHeaders {
         request_(request) {
   }
 
-  virtual bool Lookup(const GoogleString& key, StringPieceVector* values);
+  virtual bool Lookup(const StringPiece& key, StringPieceVector* values);
 
   virtual bool IsLikelyStaticResourceType() const {
     DCHECK(false);  // not called in our use-case.

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

@@ -24,38 +24,240 @@
 //  - The read handler parses the response. Add the response to the buffer at
 //    last.
 
+// TODO(oschaaf): Currently the first applicable connection is picked from the
+// pool when re-using connections. Perhaps it would be worth it to pick the one
+// that was active the longest time ago to keep a larger pool available.
+// TODO(oschaaf): style: reindent namespace according to google C++ style guide
+// TODO(oschaaf): Retry mechanism for failures on a re-used k-a connection.
+// Currently we don't think it's going to be an issue, see the comments at
+// https://github.com/pagespeed/ngx_pagespeed/pull/781.
+
+extern "C" {
+#include <nginx.h>
+}
+
 #include "ngx_fetch.h"
-#include "net/instaweb/util/public/basictypes.h"
+
 #include "base/logging.h"
 
 #include <algorithm>
 #include <string>
+#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 "pagespeed/kernel/base/basictypes.h"
-#include "net/instaweb/util/public/message_handler.h"
+#include "pagespeed/kernel/base/condvar.h"
-#include "net/instaweb/util/public/pool.h"
+#include "pagespeed/kernel/base/message_handler.h"
-#include "net/instaweb/util/public/pool_element.h"
+#include "pagespeed/kernel/base/pool.h"
-#include "net/instaweb/util/public/statistics.h"
+#include "pagespeed/kernel/base/pool_element.h"
-#include "net/instaweb/util/public/string_writer.h"
+#include "pagespeed/kernel/base/scoped_ptr.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/statistics.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/string_writer.h"
-#include "net/instaweb/util/public/timer.h"
+#include "pagespeed/kernel/base/string_util.h"
-#include "net/instaweb/util/public/writer.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 {
+
+NgxConnection::NgxConnectionPool NgxConnection::connection_pool;
+PthreadMutex NgxConnection::connection_pool_mutex;
+// Default keepalive 60s.
+const int64 NgxConnection::keepalive_timeout_ms = 60000;
+const GoogleString NgxConnection::ka_header =
+    StrCat("keep-alive ",
+           Integer64ToString(NgxConnection::keepalive_timeout_ms));
+
+NgxConnection::NgxConnection(MessageHandler* handler,
+                             int max_keepalive_requests) {
+  c_ = NULL;
+  max_keepalive_requests_ = max_keepalive_requests;
+  handler_ = handler;
+  // max_keepalive_requests specifies the number of http requests that are
+  // allowed to be performed over a single connection. So, a
+  // max_keepalive_requests of 1 effectively disables keepalive.
+  keepalive_ = max_keepalive_requests_ > 1;
+}
+
+NgxConnection::~NgxConnection() {
+  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,
+                                      MessageHandler* handler,
+                                      int max_keepalive_requests) {
+  NgxConnection* nc;
+  {
+    ScopedMutex lock(&NgxConnection::connection_pool_mutex);
+
+    for (NgxConnectionPool::iterator p = connection_pool.begin();
+         p != connection_pool.end(); ++p) {
+      nc = *p;
+
+      if (ngx_memn2cmp(static_cast<u_char*>(nc->sockaddr_),
+                       reinterpret_cast<u_char*>(pc->sockaddr),
+                       nc->socklen_, pc->socklen) == 0) {
+        CHECK(nc->c_->idle) << "Pool should only contain idle connections!";
+
+        nc->c_->idle = 0;
+        nc->c_->log = pc->log;
+        nc->c_->read->log = pc->log;
+        nc->c_->write->log = pc->log;
+        if (nc->c_->pool != NULL) {
+          nc->c_->pool->log = pc->log;
+        }
+
+        if (nc->c_->read->timer_set) {
+          ngx_del_timer(nc->c_->read);
+        }
+        connection_pool.Remove(nc);
+
+        ngx_log_error(NGX_LOG_DEBUG, pc->log, 0,
+                      "NgxFetch: re-using connection %p (pool size: %l)",
+                      nc, connection_pool.size());
+        return nc;
+      }
+    }
+  }
+
+  int rc = ngx_event_connect_peer(pc);
+  if (rc == NGX_ERROR || rc == NGX_DECLINED || rc == NGX_BUSY) {
+    return NULL;
+  }
+
+  // NgxConnection deletes itself if NgxConnection::Close()
+  nc = new NgxConnection(handler, max_keepalive_requests);
+  nc->SetSock(reinterpret_cast<u_char*>(pc->sockaddr), pc->socklen);
+  nc->c_ = pc->connection;
+  return nc;
+}
+
+void NgxConnection::Close() {
+  bool removed_from_pool = false;
+
+  {
+    ScopedMutex lock(&NgxConnection::connection_pool_mutex);
+    for (NgxConnectionPool::iterator p = connection_pool.begin();
+         p != connection_pool.end(); ++p) {
+      if (*p == this) {
+        // When we get here, that means that the connection either has timed
+        // 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)",
+                      this, connection_pool.size());
+        removed_from_pool = true;
+        break;
+      }
+    }
+  }
+
+  max_keepalive_requests_--;
+
+  if (c_->read->timer_set) {
+    ngx_del_timer(c_->read);
+  }
+
+  if (c_->write->timer_set) {
+    ngx_del_timer(c_->write);
+  }
+
+  if (!keepalive_ || max_keepalive_requests_ <= 0 || removed_from_pool) {
+    ngx_close_connection(c_);
+    c_ = NULL;
+    delete this;
+    return;
+  }
+
+  ngx_add_timer(c_->read, static_cast<ngx_msec_t>(
+      NgxConnection::keepalive_timeout_ms));
+
+  c_->data = this;
+  c_->read->handler = NgxConnection::IdleReadHandler;
+  c_->write->handler = NgxConnection::IdleWriteHandler;
+  c_->idle = 1;
+
+  // This connection should not be associated with current fetch.
+  c_->log = ngx_cycle->log;
+  c_->read->log = ngx_cycle->log;
+  c_->write->log = ngx_cycle->log;
+  if (c_->pool != NULL) {
+    c_->pool->log = ngx_cycle->log;
+  }
+
+  // Allow this connection to be re-used, by adding it to the connection pool.
+  {
+    ScopedMutex lock(&NgxConnection::connection_pool_mutex);
+    connection_pool.Add(this);
+    ngx_log_error(NGX_LOG_DEBUG, c_->log, 0,
+                  "NgxFetch: Added connection %p (pool size: %l - "
+                  " max_keepalive_requests_ %d)",
+                  this, connection_pool.size(), max_keepalive_requests_);
+  }
+}
+
+void NgxConnection::IdleWriteHandler(ngx_event_t* ev) {
+  ngx_connection_t* c = static_cast<ngx_connection_t*>(ev->data);
+  u_char buf[1];
+  int n = c->recv(c, buf, 1);
+  if (c->write->timedout) {
+    DCHECK(false) << "NgxFetch: write timeout not expected." << n;
+  }
+  if (n == NGX_AGAIN) {
+    return;
+  }
+}
+
+void NgxConnection::IdleReadHandler(ngx_event_t* ev) {
+  ngx_connection_t* c = static_cast<ngx_connection_t*>(ev->data);
+  NgxConnection* nc = static_cast<NgxConnection*>(c->data);
+
+  if (c->read->timedout) {
+    nc->set_keepalive(false);
+    nc->Close();
+    return;
+  }
+
+  char buf[1];
+  int n;
+
+  // not a timeout event, we should check connection
+  n = recv(c->fd, buf, 1, MSG_PEEK);
+  if (n == -1 && ngx_socket_errno == NGX_EAGAIN) {
+    if (ngx_handle_read_event(c->read, 0) != NGX_OK) {
+      nc->set_keepalive(false);
+      nc->Close();
+      return;
+    }
+
+    return;
+  }
+
+  nc->set_keepalive(false);
+  nc->Close();
+}
+
 NgxFetch::NgxFetch(const GoogleString& url,
                    AsyncFetch* async_fetch,
                    MessageHandler* message_handler,
-                     ngx_msec_t timeout_ms,
                    ngx_log_t* log)
     : str_url_(url),
       fetcher_(NULL),
@@ -66,7 +268,9 @@ namespace net_instaweb {
       fetch_start_ms_(0),
       fetch_end_ms_(0),
       done_(false),
-        content_length_(0) {
+      content_length_(-1),
+      content_length_known_(false),
+      resolver_ctx_(NULL) {
   ngx_memzero(&url_, sizeof(url_));
   log_ = log;
   pool_ = NULL;
@@ -79,17 +283,24 @@ namespace net_instaweb {
     ngx_del_timer(timeout_event_);
   }
   if (connection_ != NULL) {
-      ngx_close_connection(connection_);
+    connection_->Close();
+    connection_ = NULL;
   }
   if (pool_ != NULL) {
     ngx_destroy_pool(pool_);
+    pool_ = NULL;
   }
 }
 
 // This function is called by NgxUrlAsyncFetcher::StartFetch.
 bool NgxFetch::Start(NgxUrlAsyncFetcher* fetcher) {
   fetcher_ = fetcher;
-    return Init();
+  bool ok = Init();
+  if (ok) {
+    ngx_log_error(NGX_LOG_DEBUG, log_, 0, "NgxFetch %p: initialized",
+                  this);
+  }  // else Init() will have emitted a reason
+  return ok;
 }
 
 // Create the pool, parse the url, add the timeout event and
@@ -104,7 +315,9 @@ namespace net_instaweb {
   }
 
   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;
   }
 
@@ -115,20 +328,23 @@ namespace net_instaweb {
                               "NgxFetch: ngx_pcalloc failed for timeout");
     return false;
   }
+
   timeout_event_->data = this;
-    timeout_event_->handler = NgxFetchTimeout;
+  timeout_event_->handler = NgxFetch::TimeoutHandler;
   timeout_event_->log = log_;
 
   ngx_add_timer(timeout_event_, fetcher_->fetch_timeout_);
-    r_ = static_cast<ngx_http_request_t*>(ngx_pcalloc(pool_,
+  r_ = static_cast<ngx_http_request_t*>(
-                                           sizeof(ngx_http_request_t)));
+      ngx_pcalloc(pool_, sizeof(ngx_http_request_t)));
+
   if (r_ == NULL) {
     message_handler_->Message(kError,
                               "NgxFetch: ngx_pcalloc failed for timer");
     return false;
   }
-    status_ = static_cast<ngx_http_status_t*>(ngx_pcalloc(pool_,
+  status_ = static_cast<ngx_http_status_t*>(
-                                              sizeof(ngx_http_status_t)));
+      ngx_pcalloc(pool_, sizeof(ngx_http_status_t)));
+
   if (status_ == NULL) {
     message_handler_->Message(kError,
                               "NgxFetch: ngx_pcalloc failed for status");
@@ -138,19 +354,26 @@ namespace net_instaweb {
   // The host is either a domain name or an IP address.  First check
   // if it's a valid IP address and only if that fails fall back to
   // using the DNS resolver.
-    GoogleString s_ipaddress(reinterpret_cast<char*>(url_.host.data),
+
-                             url_.host.len);
+  // Maybe we have a Proxy.
+  ngx_url_t* tmp_url = &url_;
+  if (fetcher_->proxy_.url.len != 0) {
+    tmp_url = &fetcher_->proxy_;
+  }
+
+  GoogleString s_ipaddress(reinterpret_cast<char*>(tmp_url->host.data),
+                           tmp_url->host.len);
   ngx_memzero(&sin_, sizeof(sin_));
   sin_.sin_family = AF_INET;
-    sin_.sin_port = htons(url_.port);
+  sin_.sin_port = htons(tmp_url->port);
   sin_.sin_addr.s_addr = inet_addr(s_ipaddress.c_str());
 
   if (sin_.sin_addr.s_addr == INADDR_NONE) {
     // inet_addr returned INADDR_NONE, which means the hostname
     // isn't a valid IP address.  Check DNS.
     ngx_resolver_ctx_t temp;
-      temp.name.data = url_.host.data;
+    temp.name.data = tmp_url->host.data;
-      temp.name.len = url_.host.len;
+    temp.name.len = tmp_url->host.len;
     resolver_ctx_ = ngx_resolve_start(fetcher_->resolver_, &temp);
     if (resolver_ctx_ == NULL || resolver_ctx_ == NGX_NO_RESOLVER) {
       // TODO(oschaaf): this spams the log, but is useful in the fetcher's
@@ -159,13 +382,21 @@ namespace net_instaweb {
           kError, "NgxFetch: Couldn't start resolving, "
           "is there a proper resolver configured in nginx.conf?");
       return false;
+    } else {
+      ngx_log_error(NGX_LOG_DEBUG, log_, 0,
+                    "NgxFetch %p: start resolve for: %s",
+                    this, s_ipaddress.c_str());
     }
 
     resolver_ctx_->data = this;
-      resolver_ctx_->name.data = url_.host.data;
+    resolver_ctx_->name.data = tmp_url->host.data;
-      resolver_ctx_->name.len = url_.host.len;
+    resolver_ctx_->name.len = tmp_url->host.len;
+
+#if (nginx_version < 1005008)
     resolver_ctx_->type = NGX_RESOLVE_A;
-      resolver_ctx_->handler = NgxFetchResolveDone;
+#endif
+
+    resolver_ctx_->handler = NgxFetch::ResolveDoneHandler;
     resolver_ctx_->timeout = fetcher_->resolver_timeout_;
 
     if (ngx_resolve_name(resolver_ctx_) != NGX_OK) {
@@ -189,6 +420,9 @@ namespace net_instaweb {
 // 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",
+                this, success ? "OK":"FAIL");
+
   if (async_fetch_ == NULL) {
     LOG(FATAL)
         << "BUG: NgxFetch callback called more than once on same fetch"
@@ -197,16 +431,40 @@ namespace net_instaweb {
     return;
   }
 
+  release_resolver();
+
   if (timeout_event_ && timeout_event_->timer_set) {
     ngx_del_timer(timeout_event_);
     timeout_event_ = NULL;
   }
-    if (connection_) {
+
-      ngx_close_connection(connection_);
+  if (connection_ != NULL) {
-      connection_ = NULL;
+    // Connection will be re-used only on responses that specify
+    // 'Connection: keep-alive' in their headers.
+    bool keepalive = false;
+
+    if (success) {
+      ConstStringStarVector v;
+      if (async_fetch_->response_headers()->Lookup(
+              StringPiece(HttpAttributes::kConnection), &v)) {
+        for (size_t i = 0; i < v.size(); i++) {
+          if (*v[i] == "keep-alive") {
+            keepalive = true;
+            break;
+          } else if (*v[i] == "close") {
+            break;
+          }
+        }
+      }
+      ngx_log_error(NGX_LOG_DEBUG, log_, 0,
+                    "NgxFetch %p: connection %p attempt keep-alive: %s",
+                    this, connection_, keepalive ? "Yes":"No");
     }
 
-    async_fetch_->Done(success);
+    connection_->set_keepalive(keepalive);
+    connection_->Close();
+    connection_ = NULL;
+  }
 
   if (fetcher_ != NULL) {
     if (fetcher_->track_original_content_length()
@@ -217,7 +475,7 @@ namespace net_instaweb {
     }
     fetcher_->FetchComplete(this);
   }
-
+  async_fetch_->Done(success);
   async_fetch_ = NULL;
 }
 
@@ -255,64 +513,85 @@ namespace net_instaweb {
     return false;
   }
   str_url_.copy(reinterpret_cast<char*>(url_.url.data), str_url_.length(), 0);
-    size_t scheme_offset;
-    u_short port;
-    if (ngx_strncasecmp(url_.url.data, reinterpret_cast<u_char*>(
-                                     const_cast<char*>("http://")), 7) == 0) {
-      scheme_offset = 7;
-      port = 80;
-    } else if (ngx_strncasecmp(url_.url.data, reinterpret_cast<u_char*>(
-                                    const_cast<char*>("https://")), 8) == 0) {
-      scheme_offset = 8;
-      port = 443;
-    } else {
-      scheme_offset = 0;
-      port = 80;
-    }
 
-    url_.url.data += scheme_offset;
+  return NgxUrlAsyncFetcher::ParseUrl(&url_, pool_);
-    url_.url.len -= scheme_offset;
-    url_.default_port = port;
-    // TODO(oschaaf): no_resolve was set to 0, which is why url_.port
-    // would always be '0' after parsing it. See:
-    // http://lxr.evanmiller.org/http/source/core/ngx_inet.c#L875
-    url_.no_resolve = 0;
-    url_.uri_part = 1;
-
-    if (ngx_parse_url(pool_, &url_) == NGX_OK) {
-      return true;
-    }
-    return false;
 }
 
 // Issue a request after the resolver is done
-  void NgxFetch::NgxFetchResolveDone(ngx_resolver_ctx_t* resolver_ctx) {
+void NgxFetch::ResolveDoneHandler(ngx_resolver_ctx_t* resolver_ctx) {
   NgxFetch* fetch = static_cast<NgxFetch*>(resolver_ctx->data);
+  NgxUrlAsyncFetcher* fetcher = fetch->fetcher_;
+
   if (resolver_ctx->state != NGX_OK) {
     if (fetch->timeout_event() != NULL && fetch->timeout_event()->timer_set) {
       ngx_del_timer(fetch->timeout_event());
       fetch->set_timeout_event(NULL);
     }
     fetch->message_handler()->Message(
-          kWarning, "NgxFetch: failed to resolve host [%.*s]",
+        kWarning, "NgxFetch %p: failed to resolve host [%.*s]", fetch,
         static_cast<int>(resolver_ctx->name.len), resolver_ctx->name.data);
     fetch->CallbackDone(false);
-      ngx_resolve_name_done(resolver_ctx);
     return;
   }
+
+  ngx_uint_t i;
+  // Find the first ipv4 address. We don't support ipv6 yet.
+  for (i = 0; i < resolver_ctx->naddrs; i++) {
+    // Old versions of nginx and tengine have a different definition of addrs,
+    // work around to make sure we are using the right type (ngx_addr_t*).
+    ngx_addr_t* ngx_addrs = reinterpret_cast<ngx_addr_t*>(resolver_ctx->addrs);
+    if (typeid(*ngx_addrs) == typeid(*resolver_ctx->addrs)) {
+      if (reinterpret_cast<struct sockaddr_in*>(ngx_addrs[i].sockaddr)
+              ->sin_family == AF_INET) {
+        break;
+      }
+    } else {
+      // We're using an old version that uses in_addr_t* for addrs.
+      break;
+    }
+  }
+
+  // If no suitable ipv4 address was found, we fail.
+  if (i == resolver_ctx->naddrs) {
+    if (fetch->timeout_event() != NULL && fetch->timeout_event()->timer_set) {
+      ngx_del_timer(fetch->timeout_event());
+      fetch->set_timeout_event(NULL);
+    }
+    fetch->message_handler()->Message(
+        kWarning, "NgxFetch %p: no suitable address for host [%.*s]", fetch,
+        static_cast<int>(resolver_ctx->name.len), resolver_ctx->name.data);
+    fetch->CallbackDone(false);
+  }
+
   ngx_memzero(&fetch->sin_, sizeof(fetch->sin_));
+
+#if (nginx_version < 1005008)
+  fetch->sin_.sin_addr.s_addr = resolver_ctx->addrs[i];
+#else
+  struct sockaddr_in* sin;
+
+  sin = reinterpret_cast<struct sockaddr_in*>(
+      resolver_ctx->addrs[i].sockaddr);
+
+  fetch->sin_.sin_family = sin->sin_family;
+  fetch->sin_.sin_addr.s_addr = sin->sin_addr.s_addr;
+#endif
+
   fetch->sin_.sin_family = AF_INET;
   fetch->sin_.sin_port = htons(fetch->url_.port);
-    fetch->sin_.sin_addr.s_addr = resolver_ctx->addrs[0];
+
+  // Maybe we have Proxy
+  if (0 != fetcher->proxy_.url.len) {
+    fetch->sin_.sin_port = htons(fetcher->proxy_.port);
+  }
 
   char* ip_address = inet_ntoa(fetch->sin_.sin_addr);
 
-    fetch->message_handler()->Message(
+  ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-        kInfo, "NgxFetch: Resolved host [%.*s] to [%s]",
+                "NgxFetch %p: Resolved host [%V] to [%s]", fetch,
-        static_cast<int>(resolver_ctx->name.len), resolver_ctx->name.data,
+                &resolver_ctx->name, ip_address);
-        ip_address);
 
-    ngx_resolve_name_done(resolver_ctx);
+  fetch->release_resolver();
 
   if (fetch->InitRequest() != NGX_OK) {
     fetch->message_handler()->Message(kError, "NgxFetch: InitRequest failed");
@@ -320,7 +599,7 @@ namespace net_instaweb {
   }
 }
 
-  // prepare the send data for this fetch, and hook write event.
+// Prepare the request data for this fetch, and hook the write event.
 int NgxFetch::InitRequest() {
   in_ = ngx_create_temp_buf(pool_, 4096);
   if (in_ == NULL) {
@@ -335,7 +614,20 @@ namespace net_instaweb {
   bool have_host = false;
   GoogleString port;
 
-    size = sizeof("GET ") - 1 + url_.uri.len + sizeof(" HTTP/1.0\r\n") - 1;
+  response_handler = NgxFetch::HandleStatusLine;
+  int rc = Connect();
+  if (rc == NGX_AGAIN || rc == NGX_OK) {
+    if (connection_->keepalive()) {
+      request_headers->Add(HttpAttributes::kConnection,
+                           NgxConnection::ka_header);
+    }
+    const char* method = request_headers->method_string();
+    size_t method_len = strlen(method);
+
+    size = (method_len +
+            1 /* for the space */ +
+            url_.uri.len +
+            sizeof(" HTTP/1.0\r\n") - 1);
 
     for (int i = 0; i < request_headers->NumAttributes(); i++) {
       // if no explicit host header is given in the request headers,
@@ -346,8 +638,7 @@ namespace net_instaweb {
 
       // name: value\r\n
       size += request_headers->Name(i).length()
-           + request_headers->Value(i).length()
+          + request_headers->Value(i).length() + 4;  // 4 for ": \r\n"
-           + 4;  // for ": \r\n"
     }
 
     if (!have_host) {
@@ -363,7 +654,8 @@ namespace net_instaweb {
       return NGX_ERROR;
     }
 
-    out_->last = ngx_cpymem(out_->last, "GET ", 4);
+    out_->last = ngx_cpymem(out_->last, method, method_len);
+    out_->last = ngx_cpymem(out_->last, " ", 1);
     out_->last = ngx_cpymem(out_->last, url_.uri.data, url_.uri.len);
     out_->last = ngx_cpymem(out_->last, " HTTP/1.0\r\n", 11);
 
@@ -386,16 +678,14 @@ namespace net_instaweb {
     }
     *(out_->last++) = CR;
     *(out_->last++) = LF;
-
-    response_handler = NgxFetchHandleStatusLine;
-    int rc = Connect();
     if (rc == NGX_AGAIN) {
       return NGX_OK;
+    }
   } else if (rc < NGX_OK) {
     return rc;
   }
-
+  CHECK(rc == NGX_OK);
-    NgxFetchWrite(connection_->write);
+  NgxFetch::ConnectionWriteHandler(connection_->c_->write);
   return NGX_OK;
 }
 
@@ -412,99 +702,114 @@ namespace net_instaweb {
   pc.log = fetcher_->log_;
   pc.rcvbuf = -1;
 
-    int rc = ngx_event_connect_peer(&pc);
-    connection_ = pc.connection;
-    connection_->write->handler = NgxFetchWrite;
-    connection_->read->handler = NgxFetchRead;
-    connection_->data = this;
 
-    // TODO(junmin): set connect timeout when rc == NGX_AGAIN
+  connection_ = NgxConnection::Connect(&pc, message_handler(),
-    return rc;
+                                       fetcher_->max_keepalive_requests_);
+  ngx_log_error(NGX_LOG_DEBUG, fetcher_->log_, 0,
+                "NgxFetch %p Connect() connection %p for [%s]",
+                this, connection_, str_url());
+
+  if (connection_ == NULL) {
+    return NGX_ERROR;
+  }
+
+  connection_->c_->write->handler = NgxFetch::ConnectionWriteHandler;
+  connection_->c_->read->handler = NgxFetch::ConnectionReadHandler;
+  connection_->c_->data = this;
+
+  // Timer set in Init() is still in effect.
+  return NGX_OK;
 }
 
 // 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::NgxFetchWrite(ngx_event_t* wev) {
+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_;
-
+  bool ok = true;
-    while (out->pos < out->last) {
+  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 "
+                  "send result %d", fetch, n);
+
     if (n >= 0) {
       out->pos += n;
     } else if (n == NGX_AGAIN) {
-        // TODO(junmin): set write event timeout
+      break;
-        if (ngx_handle_write_event(c->write, 0) != NGX_OK) {
-          fetch->CallbackDone(false);
-        }
-        return;
     } else {
-        c->error = 1;
+      ok = false;
-        fetch->CallbackDone(false);
+      break;
-        return;
     }
   }
 
-    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;
 }
 
-  void NgxFetch::NgxFetchRead(ngx_event_t* rev) {
+// 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);
 
+    ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
+                  "NgxFetch %p: ConnectionReadHandler "
+                  "recv result %d", fetch, n);
+
     if (n == NGX_AGAIN) {
       break;
-      }
+    } else if (n == 0) {
-
+      // If the content length was not known, we assume that we have read
-      if (n == 0) {
+      // all if we at least parsed the headers.
-        // connection is closed prematurely by remote server,
+      // If we do know the content length, having a mismatch on the bytes read
-        // or the content-length was 0
+      // will be interpreted as an error.
-        fetch->CallbackDone(fetch->content_length_ == 0);
+      ok = (fetch->content_length_known_ && fetch->content_length_ == fetch->bytes_received_)
-        return;
+          || 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)) {
+      ok = fetch->response_handler(c);
-          fetch->CallbackDone(false);
+      if (fetch->done_ || !ok) {
-          return;
-        }
-
-        if (fetch->done_) {
-          fetch->CallbackDone(true);
-          return;
-        }
-      }
-
-      if (!rev->ready) {
         break;
       }
     }
-
-    if (fetch->done_) {
-      fetch->CallbackDone(true);
-      return;
   }
 
-    if (ngx_handle_read_event(rev, 0) != NGX_OK) {
+  if (!ok) {
+    fetch->CallbackDone(false);
+  } else if (fetch->done_) {
+    fetch->CallbackDone(true);
+  } else if (ngx_handle_read_event(rev, 0) != NGX_OK) {
     fetch->CallbackDone(false);
   }
-
-    // TODO(junmin): set read event timeout
 }
 
 // Parse the status line: "HTTP/1.1 200 OK\r\n"
-  bool NgxFetch::NgxFetchHandleStatusLine(ngx_connection_t* c) {
+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", 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.
   ngx_int_t n = ngx_http_parse_status_line(fetch->r_, fetch->in_,
@@ -516,65 +821,102 @@ namespace net_instaweb {
   } 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->set_response_handler(NgxFetchHandleHeader);
+
+  fetch->in_->pos += n;
+  fetch->set_response_handler(NgxFetch::HandleHeader);
+  if ((fetch->in_->last - fetch->in_->pos) > 0) {
     return fetch->response_handler(c);
   }
+  return true;
+}
 
 // Parse the HTTP headers
-  bool NgxFetch::NgxFetchHandleHeader(ngx_connection_t* c) {
+bool NgxFetch::HandleHeader(ngx_connection_t* c) {
   NgxFetch* fetch = static_cast<NgxFetch*>(c->data);
   char* data = reinterpret_cast<char*>(fetch->in_->pos);
   size_t size = fetch->in_->last - fetch->in_->pos;
   size_t n = fetch->parser_.ParseChunk(StringPiece(data, size),
                                        fetch->message_handler_);
+
+  ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
+                "NgxFetch %p: Handle headers", fetch);
+
   if (n > size) {
     return false;
   } else if (fetch->parser_.headers_complete()) {
-      int64 content_length = -1;
+    // TODO(oschaaf): We should also check if the request method was HEAD
-      fetch->async_fetch_->response_headers()->FindContentLength(
+    // - but I don't think PSOL uses that at this point.
-          &content_length);
+    if (fetch->get_status_code() == 304 || fetch->get_status_code() == 204) {
-      fetch->content_length_ = content_length;
+      fetch->done_ = true;
-      if (fetch->fetcher_->track_original_content_length()) {
+    } else if (fetch->async_fetch_->response_headers()->FindContentLength(
+            &fetch->content_length_)) {
+      if (fetch->content_length_ < 0) {
+        fetch->message_handler_->Message(
+            kError, "Negative content-length in response header");
+        return false;
+      } else {
+        fetch->content_length_known_ = true;
+        if (fetch->content_length_ == 0) {
+          fetch->done_ = true;
+        }
+      }
+    }
+
+    if (fetch->fetcher_->track_original_content_length()
+        && fetch->content_length_known_) {
       fetch->async_fetch_->response_headers()->SetOriginalContentLength(
-            content_length);
+          fetch->content_length_);
     }
 
     fetch->in_->pos += n;
-      fetch->set_response_handler(NgxFetchHandleBody);
+    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;
+  }
   return true;
 }
 
 // Read the response body
-  bool NgxFetch::NgxFetchHandleBody(ngx_connection_t* c) {
+bool NgxFetch::HandleBody(ngx_connection_t* c) {
   NgxFetch* fetch = static_cast<NgxFetch*>(c->data);
   char* data = reinterpret_cast<char*>(fetch->in_->pos);
   size_t size = fetch->in_->last - fetch->in_->pos;
-    if (size == 0) {
-      return true;
-    }
 
-    fetch->bytes_received_add(static_cast<int64>(size));
+  fetch->bytes_received_add(size);
+
+  ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
+                "NgxFetch %p: Handle body (%d bytes)", fetch, size);
+
   if ( fetch->async_fetch_->Write(StringPiece(data, size),
                                   fetch->message_handler()) ) {
-      fetch->content_length_ -= size;
+    if (fetch->bytes_received_ == fetch->content_length_) {
-      if (fetch->content_length_ <= 0) {
       fetch->done_ = true;
     }
+    fetch->in_->pos += size;
+  } else {
+    ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
+                  "NgxFetch %p: async fetch write failure", fetch);
+    return false;
+  }
   return true;
 }
-    return false;
-  }
 
-  void NgxFetch::NgxFetchTimeout(ngx_event_t* tev) {
+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", fetch);
   fetch->CallbackDone(false);
 }
 
@@ -583,7 +925,7 @@ namespace net_instaweb {
   ConstStringStarVector v;
   RequestHeaders* request_headers = async_fetch_->request_headers();
   if (request_headers->Lookup(HttpAttributes::kUserAgent, &v)) {
-      for (int i = 0, n = v.size(); i < n; i++) {
+    for (size_t i = 0, n = v.size(); i < n; i++) {
       if (i != 0) {
         user_agent += " ";
       }
@@ -598,11 +940,12 @@ namespace net_instaweb {
     user_agent += "NgxNativeFetcher";
   }
   GoogleString version = StrCat(
-        " ", kModPagespeedSubrequestUserAgent,
+      " (", kModPagespeedSubrequestUserAgent,
-        "/" MOD_PAGESPEED_VERSION_STRING "-" LASTCHANGE_STRING);
+      "/" MOD_PAGESPEED_VERSION_STRING "-" LASTCHANGE_STRING ")");
   if (!StringPiece(user_agent).ends_with(version)) {
     user_agent += version;
   }
   request_headers->Add(HttpAttributes::kUserAgent, user_agent);
 }
+
 }  // namespace net_instaweb

src/ngx_fetch.h

@@ -16,49 +16,107 @@
 
 // Author: x.dinic@gmail.com (Junmin Xiong)
 //
+// PageSpeed needs some way to talk to the internet and request resources.  For
+// example, if it's optimizing www.example.com/index.html and it sees html with
+// <img src="//images.example.com/cat.jpg"> and images.example.com is authorized
+// for rewriting in the config, then it needs to fetch cat.jpg from
+// images.example.com and optimize it.  In apache (always) and nginx (by
+// default) we use a fetcher called "serf".  This works fine, but it does run
+// its own event loop.  To be more efficient, this is a "native" fetcher that
+// uses nginx's event loop.
+//
 // The fetch is started by the main thread. It will fetch the remote resource
 // from the specific url asynchronously.
 
-#ifndef NET_INSTAWEB_NGX_FETCHER_H_
+#ifndef NET_INSTAWEB_NGX_FETCH_H_
-#define NET_INSTAWEB_NGX_FETCHER_H_
+#define NET_INSTAWEB_NGX_FETCH_H_
 
 extern "C" {
 #include <ngx_config.h>
 #include <ngx_core.h>
 #include <ngx_http.h>
-
-  typedef bool (*response_handler_pt)(ngx_connection_t* 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 "pagespeed/kernel/base/basictypes.h"
-#include "net/instaweb/http/public/response_headers_parser.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 {
+
+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,
            AsyncFetch* async_fetch,
            MessageHandler* message_handler,
-               ngx_msec_t timeout_ms,
            ngx_log_t* log);
   ~NgxFetch();
 
-      // Start the fetch
+  // Start the fetch.
   bool Start(NgxUrlAsyncFetcher* fetcher);
-      // Show the completed url, for logging purpose
+  // Show the completed url, for logging purposes.
   const char* str_url();
-      // This fetch task is done. Call the done of async_fetch.
+  // This fetch task is done. Call Done() on the async_fetch. It will copy the
-      // It will copy the buffer to cache.
+  // buffer to cache.
   void CallbackDone(bool success);
 
-      // Show the bytes received
+  // Show the bytes received.
   size_t bytes_received();
   void bytes_received_add(int64 x);
   int64 fetch_start_ms();
@@ -66,24 +124,28 @@ namespace net_instaweb {
   int64 fetch_end_ms();
   void set_fetch_end_ms(int64 end_ms);
   MessageHandler* message_handler();
+
   int get_major_version() {
     return static_cast<int>(status_->http_version / 1000);
   }
-
   int get_minor_version() {
     return static_cast<int>(status_->http_version % 1000);
   }
-
   int get_status_code() {
     return static_cast<int>(status_->code);
   }
-
   ngx_event_t* timeout_event() {
     return timeout_event_;
-      };
+  }
   void set_timeout_event(ngx_event_t* x) {
     timeout_event_ = x;
-      };
+  }
+  void release_resolver() {
+    if (resolver_ctx_ != NULL && resolver_ctx_ != NGX_NO_RESOLVER) {
+      ngx_resolve_name_done(resolver_ctx_);
+      resolver_ctx_ = NULL;
+    }
+  }
 
  private:
   response_handler_pt response_handler;
@@ -98,24 +160,21 @@ namespace net_instaweb {
     response_handler = handler;
   }
   // Only the Static functions could be used in callbacks.
-      static void NgxFetchResolveDone(ngx_resolver_ctx_t* ctx);
+  static void ResolveDoneHandler(ngx_resolver_ctx_t* ctx);
+  // Write the request.
+  static void ConnectionWriteHandler(ngx_event_t* wev);
+  // Wait for the response.
+  static void ConnectionReadHandler(ngx_event_t* rev);
+  // Read and parse the first status line.
+  static bool HandleStatusLine(ngx_connection_t* c);
+  // Read and parse the HTTP headers.
+  static bool HandleHeader(ngx_connection_t* c);
+  // Read the response body.
+  static bool HandleBody(ngx_connection_t* c);
+  // Cancel the fetch when it's timeout.
+  static void TimeoutHandler(ngx_event_t* tev);
 
-      // Write the request
+  // Add the pagespeed User-Agent.
-      static void NgxFetchWrite(ngx_event_t* wev);
-
-      // Wait for the response
-      static void NgxFetchRead(ngx_event_t* rev);
-      // Read and parse the first status line
-      static bool NgxFetchHandleStatusLine(ngx_connection_t* c);
-      // Read and parse the HTTP headers
-      static bool NgxFetchHandleHeader(ngx_connection_t* c);
-      // Read the response body
-      static bool NgxFetchHandleBody(ngx_connection_t* c);
-
-      // Cancel the fetch when it's timeout
-      static void NgxFetchTimeout(ngx_event_t* tev);
-
-      // Add the pagespeed User-Agent
   void FixUserAgent();
   void FixHost();
 
@@ -125,12 +184,12 @@ namespace net_instaweb {
   AsyncFetch* async_fetch_;
   ResponseHeadersParser parser_;
   MessageHandler* message_handler_;
-      size_t bytes_received_;
+  int64 bytes_received_;
   int64 fetch_start_ms_;
   int64 fetch_end_ms_;
-      int64 timeout_ms_;
   bool done_;
   int64 content_length_;
+  bool content_length_known_;
 
   struct sockaddr_in sin_;
   ngx_log_t* log_;
@@ -140,7 +199,7 @@ namespace net_instaweb {
   ngx_http_request_t* r_;
   ngx_http_status_t* status_;
   ngx_event_t* timeout_event_;
-      ngx_connection_t* connection_;
+  NgxConnection* connection_;
   ngx_resolver_ctx_t* resolver_ctx_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxFetch);
@@ -148,4 +207,4 @@ namespace net_instaweb {
 
 }  // namespace net_instaweb
 
-#endif  // NET_INSTAWEB_NGX_FETCHER_H_
+#endif  // NET_INSTAWEB_NGX_FETCH_H_

src/ngx_gzip_setter.cc

@@ -0,0 +1,403 @@
+/*
+ * 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: kspoelstra@we-amp.com (Kees Spoelstra)
+
+#include "ngx_gzip_setter.h"
+
+#include <ngx_conf_file.h>
+
+namespace net_instaweb {
+
+NgxGZipSetter g_gzip_setter;
+
+extern "C" {
+  // These functions replace the setters for:
+  //   gzip
+  //   gzip_types
+  //   gzip_http_version
+  //   gzip_vary
+  //
+  // If these functions are called it means there is an explicit gzip
+  // configuration. The gzip configuration set by pagespeed is then rolled
+  // back and pagespeed will stop enabling gzip automatically.
+  char* ngx_gzip_redirect_conf_set_flag_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_conf_set_flag_slot(cf, cmd, conf);
+    return ret;
+  }
+
+  char* ngx_gzip_redirect_http_types_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_http_types_slot(cf, cmd, conf);
+    return ret;
+  }
+
+  char* ngx_gzip_redirect_conf_set_enum_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_conf_set_enum_slot(cf, cmd, conf);
+    return ret;
+  }
+  char* ngx_gzip_redirect_conf_set_bitmask_slot(
+      ngx_conf_t* cf, ngx_command_t* cmd, void* conf) {
+    if (g_gzip_setter.enabled()) {
+      g_gzip_setter.RollBackAndDisable(cf);
+    }
+    char* ret = ngx_conf_set_bitmask_slot(cf, cmd, conf);
+    return ret;
+  }
+}
+
+NgxGZipSetter::NgxGZipSetter() : enabled_(0) { }
+NgxGZipSetter::~NgxGZipSetter() { }
+
+// Helper functions to determine signature.
+bool HasLocalConfig(ngx_command_t* command) {
+  return (!(command->type & (NGX_DIRECT_CONF|NGX_MAIN_CONF)) &&
+          command->conf == NGX_HTTP_LOC_CONF_OFFSET);
+}
+bool IsNgxFlagCommand(ngx_command_t* command) {
+  return (command->set == ngx_conf_set_flag_slot &&
+          HasLocalConfig(command));
+}
+bool IsNgxHttpTypesCommand(ngx_command_t* command) {
+  return (command->set == ngx_http_types_slot &&
+          HasLocalConfig(command));
+}
+bool IsNgxEnumCommand(ngx_command_t* command) {
+  return (command->set == ngx_conf_set_enum_slot &&
+          HasLocalConfig(command));
+}
+bool IsNgxBitmaskCommand(ngx_command_t* command) {
+  return (command->set == ngx_conf_set_bitmask_slot &&
+          HasLocalConfig(command));
+}
+
+// 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
+// a rollback if expicit configuration is found.
+// If commands are not found the method will inform the user by logging.
+void NgxGZipSetter::Init(ngx_conf_t* cf) {
+#if (NGX_HTTP_GZIP)
+  bool gzip_signature_mismatch = false;
+  bool other_signature_mismatch = false;
+  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++) {
+        ngx_command_t* current_command =& ngx_modules[m]->commands[c];
+
+        // We look for the gzip command, and the exact signature we trust
+        // this means configured as an config location offset
+        // and a ngx_flag_t setter.
+        // Also see:
+        //   ngx_conf_handler in ngx_conf_file.c
+        //   ngx_http_gzip_filter_commands in ngx_http_gzip_filter.c
+        if (gzip_command_.command_ == NULL &&
+            STR_EQ_LITERAL(current_command->name, "gzip")) {
+          if (IsNgxFlagCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_flag_slot;
+            gzip_command_.command_ = current_command;
+            gzip_command_.module_ = ngx_modules[m];
+            enabled_ = 1;
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip, signature mismatch");
+            gzip_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_http_version_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_http_version")) {
+          if (IsNgxEnumCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_enum_slot;
+            gzip_http_version_command_.command_ = current_command;
+            gzip_http_version_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_http_version, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_proxied_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_proxied")) {
+          if (IsNgxBitmaskCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_bitmask_slot;
+            gzip_proxied_command_.command_ = current_command;
+            gzip_proxied_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_proxied, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_http_types_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_types")) {
+          if (IsNgxHttpTypesCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_http_types_slot;
+            gzip_http_types_command_.command_ = current_command;
+            gzip_http_types_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_types, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+
+        if (!gzip_vary_command_.command_ &&
+            STR_EQ_LITERAL(current_command->name, "gzip_vary")) {
+          if (IsNgxFlagCommand(current_command)) {
+            current_command->set = ngx_gzip_redirect_conf_set_flag_slot;
+            gzip_vary_command_.command_ = current_command;
+            gzip_vary_command_.module_ = ngx_modules[m];
+          } else {
+            ngx_conf_log_error(
+                NGX_LOG_WARN, cf, 0,
+                "pagespeed: cannot set gzip_vary, signature mismatch");
+            other_signature_mismatch = true;
+          }
+        }
+      }
+    }
+  }
+  if (gzip_signature_mismatch) {
+    return;  // Already logged error.
+  } else if (!enabled_) {
+    // Looked through all the available commands and didn't find the "gzip" one.
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: cannot set gzip, command not found");
+    return;
+  } else if (other_signature_mismatch) {
+    return;  // Already logged error.
+  } else if (!gzip_vary_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_vary");
+    return;
+  } else if (!gzip_http_types_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_types");
+    return;
+  } else if (!gzip_http_version_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_http_version");
+    return;
+  } else if (!gzip_proxied_command_.command_) {
+    ngx_conf_log_error(
+        NGX_LOG_WARN, cf, 0, "pagespeed: missing gzip_proxied");
+    return;
+  } else {
+    return;  // Success.
+  }
+#else
+  ngx_conf_log_error(
+      NGX_LOG_WARN, cf, 0, "pagespeed: gzip not compiled into nginx");
+  return;
+#endif
+}
+
+void* ngx_command_ctx::GetConfPtr(ngx_conf_t* cf) {
+  return GetModuleConfPtr(cf) + command_->offset;
+}
+
+char* ngx_command_ctx::GetModuleConfPtr(ngx_conf_t* cf) {
+  return reinterpret_cast<char*>(
+      ngx_http_conf_get_module_loc_conf(cf, (*(module_))));
+}
+
+void NgxGZipSetter::SetNgxConfFlag(ngx_conf_t* cf,
+                                   ngx_command_ctx* command_ctx,
+                                   ngx_flag_t value) {
+  ngx_flag_t* flag = reinterpret_cast<ngx_flag_t*>(command_ctx->GetConfPtr(cf));
+  *flag = value;
+  // Save the flag position for possible rollback.
+  ngx_flags_set_.push_back(flag);
+}
+
+void NgxGZipSetter::SetNgxConfEnum(ngx_conf_t* cf,
+                                   ngx_command_ctx* command_ctx,
+                                   ngx_uint_t value) {
+  ngx_uint_t* enum_to_set =
+      reinterpret_cast<ngx_uint_t*>(command_ctx->GetConfPtr(cf));
+  *enum_to_set = value;
+  ngx_uint_set_.push_back(enum_to_set);
+}
+
+void NgxGZipSetter::SetNgxConfBitmask(ngx_conf_t* cf,
+                                      ngx_command_ctx* command_ctx,
+                                      ngx_uint_t value) {
+  ngx_uint_t* enum_to_set =
+      reinterpret_cast<ngx_uint_t*>(command_ctx->GetConfPtr(cf));
+  *enum_to_set = value;
+  ngx_uint_set_.push_back(enum_to_set);
+}
+
+// These are the content types we want to compress.
+ngx_str_t gzip_http_types[] = {
+  ngx_string("application/ecmascript"),
+  ngx_string("application/javascript"),
+  ngx_string("application/json"),
+  ngx_string("application/pdf"),
+  ngx_string("application/postscript"),
+  ngx_string("application/x-javascript"),
+  ngx_string("image/svg+xml"),
+  ngx_string("text/css"),
+  ngx_string("text/csv"),
+  // ngx_string("text/html"),  // This is the default implied value.
+  ngx_string("text/javascript"),
+  ngx_string("text/plain"),
+  ngx_string("text/xml"),
+  ngx_null_string  // Indicates end of array.
+};
+
+gzs_enable_result NgxGZipSetter::SetGZipForLocation(ngx_conf_t* cf,
+                                                    bool value) {
+  if (!enabled_) {
+    return kEnableGZipNotEnabled;
+  }
+  if (gzip_command_.command_) {
+    SetNgxConfFlag(cf, &gzip_command_, value);
+  }
+  return kEnableGZipOk;
+}
+
+void NgxGZipSetter::EnableGZipForLocation(ngx_conf_t* cf) {
+  if (!enabled_) {
+    return;
+  }
+
+  // When we get called twice for the same location{}, we ignore the second call
+  // to prevent adding duplicate gzip http types and so on.
+  ngx_flag_t* flag =
+      reinterpret_cast<ngx_flag_t*>(gzip_command_.GetConfPtr(cf));
+  if (*flag == 1) {
+    return;
+  }
+  SetGZipForLocation(cf, true);
+  if (gzip_vary_command_.command_) {
+    SetNgxConfFlag(cf, &gzip_vary_command_, 1);
+  }
+  if (gzip_http_version_command_.command_) {
+    SetNgxConfEnum(cf, &gzip_http_version_command_, NGX_HTTP_VERSION_10);
+  }
+  if (gzip_proxied_command_.command_) {
+    SetNgxConfBitmask(
+        cf, &gzip_http_version_command_, NGX_HTTP_GZIP_PROXIED_ANY);
+  }
+
+  // This is actually the most prone to future API changes, because gzip_types
+  // is not a simple type like ngx_flag_t. The signature check should be enough
+  // to prevent problems.
+  AddGZipHTTPTypes(cf);
+  return;
+}
+
+void NgxGZipSetter::AddGZipHTTPTypes(ngx_conf_t* cf) {
+  if (gzip_http_types_command_.command_) {
+    // Following should not happen, but if it does return gracefully.
+    if (cf->args->nalloc < 2) {
+      ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                         "pagespeed: unexpected small cf->args in gzip_types");
+      return;
+    }
+
+    ngx_command_t* command = gzip_http_types_command_.command_;
+    char* gzip_conf = reinterpret_cast<char* >(
+        gzip_http_types_command_.GetModuleConfPtr(cf));
+
+    // Backup the old settings.
+    ngx_str_t old_elt0 = reinterpret_cast<ngx_str_t*>(cf->args->elts)[0];
+    ngx_str_t old_elt1 = reinterpret_cast<ngx_str_t*>(cf->args->elts)[1];
+    ngx_uint_t old_nelts = cf->args->nelts;
+
+    // Setup first arg.
+    ngx_str_t gzip_types_string = ngx_string("gzip_types");
+    reinterpret_cast<ngx_str_t*>(cf->args->elts)[0] = gzip_types_string;
+    cf->args->nelts = 2;
+
+    ngx_str_t* http_types = gzip_http_types;
+    while (http_types->data) {
+      ngx_str_t d;
+      // We allocate the http type on the configuration pool and actually
+      // leak this if we rollback. This does not seem to be a big problem,
+      // because nginx also allocates tokens in ngx_conf_file.c and does not
+      // free them. This way they can be used safely by configurations.
+      // We must use a copy of gzip_http_types array here because nginx will
+      // manipulate the values.
+      // TODO(kspoelstra): better would be to allocate once on init and not
+      // every time we enable gzip. This needs further investigation, sharing
+      // tokens might be problematic.
+      // For now I think it is not a large problem. This might add up in case
+      // of a large multi server/location config with a lot of "pagespeed on"
+      // directives.
+      // Estimates are 300-400KB for 1000 times "pagespeed on".
+      d.data = reinterpret_cast<u_char*>(
+          ngx_pnalloc(cf->pool, http_types->len + 1));
+      snprintf(reinterpret_cast<char*>(d.data), http_types->len + 1, "%s",
+               reinterpret_cast<const char*>(http_types->data));
+      d.len = http_types->len;
+      reinterpret_cast<ngx_str_t*>(cf->args->elts)[1] = d;
+      // Call the original setter.
+      ngx_http_types_slot(cf, command, gzip_conf);
+      http_types++;
+    }
+
+    // Restore args.
+    cf->args->nelts = old_nelts;
+    reinterpret_cast<ngx_str_t*>(cf->args->elts)[1] = old_elt1;
+    reinterpret_cast<ngx_str_t*>(cf->args->elts)[0] = old_elt0;
+
+    // Backup configuration location for rollback.
+    ngx_httptypes_set_.push_back(gzip_conf + command->offset);
+  }
+}
+
+void NgxGZipSetter::RollBackAndDisable(ngx_conf_t* cf) {
+  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) {
+    *(*i)=NGX_CONF_UNSET;
+  }
+  for (std::vector<ngx_uint_t*>::iterator i = ngx_uint_set_.begin();
+       i != ngx_uint_set_.end(); ++i) {
+    *(*i)=NGX_CONF_UNSET_UINT;
+  }
+  for (std::vector<void*>::iterator i = ngx_httptypes_set_.begin();
+       i != ngx_httptypes_set_.end(); ++i) {
+    ngx_array_t** type_array = reinterpret_cast<ngx_array_t**>(*i);
+    ngx_array_destroy(*type_array);
+    *type_array = NULL;
+  }
+  enabled_ = 0;
+}
+
+}  // namespace net_instaweb

src/ngx_gzip_setter.h

@@ -0,0 +1,124 @@
+/*
+ * 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: kspoelstra@we-amp.com (Kees Spoelstra)
+
+/*
+ * NgxGZipSetter sets up gzip for pagespeed
+ * with the following configuration:
+ * gzip  on;
+ * gzip_vary on;
+ * gzip_types application/ecmascript;
+ * gzip_types application/javascript;
+ * gzip_types application/json;
+ * gzip_types application/pdf;
+ * gzip_types application/postscript;
+ * gzip_types application/x-javascript;
+ * gzip_types image/svg+xml;
+ * gzip_types text/css;
+ * gzip_types text/csv;
+ * gzip_types text/javascript;
+ * gzip_types text/plain;
+ * gzip_types text/xml;
+ * gzip_http_version 1.0;
+ *
+ * If there is an explicit gzip configuration in the nginx.conf
+ * pagespeed will rollback the set configuration and let the
+ * user decide what the configuration will be.
+ *
+ * It manipulates the configuration by manipulating ngx_flag_t
+ * and ngx_uint_t settings directly and using the nginx setter for
+ * gzip_http_types.
+ * This is probably a safe way to do it. If this mechanism
+ * changes all non nginx module setup & configuration will
+ * fail.
+ */
+
+#ifndef NGX_GZIP_SETTER_H_
+#define NGX_GZIP_SETTER_H_
+
+extern "C" {
+#include <ngx_config.h>
+#include <ngx_core.h>
+#include <ngx_http.h>
+}
+
+#include <vector>
+
+#include "ngx_pagespeed.h"
+
+#include "pagespeed/kernel/base/basictypes.h"
+
+namespace net_instaweb {
+
+// We need this class because configuration for gzip is in different modules, so
+// just saving the command will not work.
+class ngx_command_ctx {
+ public:
+  ngx_command_ctx() : command_(NULL), module_(NULL) { }
+  void* GetConfPtr(ngx_conf_t* cf);
+  char* GetModuleConfPtr(ngx_conf_t* cf);
+  ngx_command_t* command_;
+  ngx_module_t* module_;
+};
+
+enum gzs_enable_result {
+  kEnableGZipOk,
+  kEnableGZipPartial,
+  kEnableGZipNotEnabled
+};
+
+class NgxGZipSetter {
+  std::vector<ngx_flag_t*> ngx_flags_set_;
+  std::vector<ngx_uint_t*> ngx_uint_set_;
+  std::vector<void*> ngx_httptypes_set_;
+  ngx_command_ctx gzip_command_;
+  ngx_command_ctx gzip_http_types_command_;
+  ngx_command_ctx gzip_proxied_command_;
+  ngx_command_ctx gzip_vary_command_;
+  ngx_command_ctx gzip_http_version_command_;
+  bool enabled_;
+
+ public:
+  NgxGZipSetter();
+  ~NgxGZipSetter();
+  void Init(ngx_conf_t* cf);
+
+  void SetNgxConfFlag(ngx_conf_t* cf,
+                      ngx_command_ctx* command_ctx,
+                      ngx_flag_t value);
+  void SetNgxConfEnum(ngx_conf_t* cf,
+                      ngx_command_ctx* command_ctx,
+                      ngx_uint_t value);
+  void SetNgxConfBitmask(ngx_conf_t* cf,
+                         ngx_command_ctx* command_ctx,
+                         ngx_uint_t value);
+  void EnableGZipForLocation(ngx_conf_t* cf);
+  gzs_enable_result SetGZipForLocation(ngx_conf_t* cf, bool value);
+  void AddGZipHTTPTypes(ngx_conf_t* cf);
+  void RollBackAndDisable(ngx_conf_t* cf);
+
+  bool enabled() { return enabled_; }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NgxGZipSetter);
+};
+
+extern NgxGZipSetter g_gzip_setter;
+
+}  // namespace net_instaweb
+
+#endif  // NGX_GZIP_SETTER_H_

src/ngx_list_iterator.h

@@ -33,7 +33,7 @@ namespace net_instaweb {
 
 class NgxListIterator {
  public:
-  NgxListIterator(ngx_list_part_t* part);
+  explicit NgxListIterator(ngx_list_part_t* part);
 
   // Return the next element of the list if there is one, NULL otherwise.
   ngx_table_elt_t* Next();

src/ngx_message_handler.cc

@@ -18,13 +18,13 @@
 
 #include <signal.h>
 
-#include "apr_time.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 {
 
@@ -56,11 +56,9 @@ extern "C" {
 
 namespace net_instaweb {
 
-NgxMessageHandler::NgxMessageHandler(AbstractMutex* mutex)
+NgxMessageHandler::NgxMessageHandler(Timer* timer, AbstractMutex* mutex)
-    : mutex_(mutex),
+    : SystemMessageHandler(timer, mutex),
-      buffer_(NULL),
       log_(NULL) {
-  SetPidString(static_cast<int64>(getpid()));
 }
 
 // Installs a signal handler for common crash signals, that tries to print
@@ -73,14 +71,6 @@ void NgxMessageHandler::InstallCrashHandler(ngx_log_t* log) {
   signal(SIGSEGV, signal_handler);
 }
 
-bool NgxMessageHandler::Dump(Writer* writer) {
-  // Can't dump before SharedCircularBuffer is set up.
-  if (buffer_ == NULL) {
-    return false;
-  }
-  return buffer_->Dump(writer, &handler_);
-}
-
 ngx_uint_t NgxMessageHandler::GetNgxLogLevel(MessageType type) {
   switch (type) {
     case kInfo:
@@ -98,65 +88,29 @@ ngx_uint_t NgxMessageHandler::GetNgxLogLevel(MessageType type) {
   return NGX_LOG_ALERT;
 }
 
-void NgxMessageHandler::set_buffer(SharedCircularBuffer* buff) {
+void NgxMessageHandler::MessageSImpl(MessageType type,
-  ScopedMutex lock(mutex_.get());
+                                     const GoogleString& message) {
-  buffer_ = buff;
-}
-
-void NgxMessageHandler::MessageVImpl(MessageType type, const char* msg,
-                                     va_list args) {
-  ngx_uint_t log_level = GetNgxLogLevel(type);
-  GoogleString formatted_message = Format(msg, args);
   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);
+  }
+  AddMessageToBuffer(type, message);
 }
 
-  // Prepare a log message for the SharedCircularBuffer only.
+void NgxMessageHandler::FileMessageSImpl(
-  // Prepend time and severity to message.
+    MessageType type, const char* file, int line, const GoogleString& message) {
-  // Format is [time] [severity] [pid] message.
-  GoogleString message;
-  char time_buffer[APR_CTIME_LEN + 1];
-  const char* time = time_buffer;
-  apr_status_t status = apr_ctime(time_buffer, apr_time_now());
-  if (status != APR_SUCCESS) {
-    time = "?";
-  }
-  StrAppend(&message, "[", time, "] ",
-            "[", MessageTypeToString(type), "] ");
-  StrAppend(&message, pid_string_, " ", formatted_message, "\n");
-  {
-    ScopedMutex lock(mutex_.get());
-    if (buffer_ != NULL) {
-      buffer_->Write(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);
   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);
   }
-}
+  AddMessageToBuffer(type, file, line, message);
-
-// TODO(sligocki): It'd be nice not to do so much string copying.
-GoogleString NgxMessageHandler::Format(const char* msg, va_list args) {
-  GoogleString buffer;
-
-  // Ignore the name of this routine: it formats with vsnprintf.
-  // See base/stringprintf.cc.
-  StringAppendV(&buffer, msg, args);
-  return buffer;
 }
 
 }  // namespace net_instaweb

src/ngx_message_handler.h

@@ -18,65 +18,48 @@
 #define NGX_MESSAGE_HANDLER_H_
 
 extern "C" {
+  #include <ngx_auto_config.h>
+#if (NGX_THREADS)
+  #include <ngx_thread.h>
+#endif
   #include <ngx_core.h>
   #include <ngx_log.h>
 }
 
 #include <cstdarg>
 
-#include "net/instaweb/util/public/basictypes.h"
+#include "pagespeed/kernel/base/basictypes.h"
-#include "net/instaweb/util/public/google_message_handler.h"
+#include "pagespeed/kernel/base/message_handler.h"
-#include "net/instaweb/util/public/message_handler.h"
+#include "pagespeed/kernel/base/string.h"
-#include "net/instaweb/util/public/scoped_ptr.h"
+#include "pagespeed/kernel/base/string_util.h"
-#include "net/instaweb/util/public/string.h"
+#include "pagespeed/system/system_message_handler.h"
-#include "net/instaweb/util/public/string_util.h"
 
 namespace net_instaweb {
 
 class AbstractMutex;
-class SharedCircularBuffer;
 class Timer;
-class Writer;
 
 // Implementation of a message handler that uses ngx_log_error()
 // logging to emit messages, with a fallback to GoogleMessageHandler
-class NgxMessageHandler : public GoogleMessageHandler {
+class NgxMessageHandler : public SystemMessageHandler {
  public:
-  explicit NgxMessageHandler(AbstractMutex* mutex);
+  explicit NgxMessageHandler(Timer* timer, AbstractMutex* mutex);
 
   // Installs a signal handler for common crash signals that tries to print
   // out a backtrace.
   static void InstallCrashHandler(ngx_log_t* log);
 
-  // When NgxRewriteDriver instantiates the NgxMessageHandlers, the
-  // SharedCircularBuffer and ngx_log_t are not available yet. These
-  // will later be set in RootInit/Childinit
-  // Messages logged before that will be passed on to handler_;
-  void set_buffer(SharedCircularBuffer* buff);
   void set_log(ngx_log_t* log) { log_ = log; }
-
+  ngx_log_t* log() { return log_; }
-  void SetPidString(const int64 pid) {
-    pid_string_ = StrCat("[", Integer64ToString(pid), "]");
-  }
-  // Dump contents of SharedCircularBuffer.
-  bool Dump(Writer* writer);
 
  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,
+  virtual void FileMessageSImpl(MessageType type, const char* file,
-                                int line, const char* msg, va_list args);
+                                int line, const GoogleString& message);
 
  private:
   ngx_uint_t GetNgxLogLevel(MessageType type);
-  GoogleString Format(const char* msg, va_list args);
-
-  scoped_ptr<AbstractMutex> mutex_;
-  GoogleString pid_string_;
-  // handler_ is used as a fallback when we can not use ngx_log_errort
-  // It's also used when calling Dump on the internal SharedCircularBuffer
-  GoogleMessageHandler handler_;
-  SharedCircularBuffer* buffer_;
   ngx_log_t* log_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxMessageHandler);

src/ngx_pagespeed.h

@@ -34,8 +34,8 @@ extern "C" {
 }
 
 #include "base/logging.h"
-#include "net/instaweb/http/public/response_headers.h"
+#include "pagespeed/kernel/base/string_util.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/http/response_headers.h"
 
 namespace net_instaweb {
 
@@ -43,17 +43,16 @@ class GzipInflater;
 class NgxBaseFetch;
 class ProxyFetch;
 class RewriteDriver;
-
+class RequestHeaders;
-}  // namespace net_instaweb
+class ResponseHeaders;
-
+class InPlaceResourceRecorder;
-namespace ngx_psol {
 
 // Allocate chain links and buffers from the supplied pool, and copy over the
 // data from the string piece.  If the string piece is empty, return
 // NGX_DECLINED immediately unless send_last_buf.
 ngx_int_t string_piece_to_buffer_chain(
     ngx_pool_t* pool, StringPiece sp,
-    ngx_chain_t** link_ptr, bool send_last_buf);
+    ngx_chain_t** link_ptr, bool send_last_buf, bool send_flush);
 
 StringPiece str_to_string_piece(ngx_str_t s);
 
@@ -75,24 +74,65 @@ StringPiece str_to_string_piece(ngx_str_t s);
 // Allocate memory out of the pool for the string piece, and copy the contents
 // over.  Returns NULL if we can't get memory.
 char* string_piece_to_pool_string(ngx_pool_t* pool, StringPiece sp);
-ngx_int_t copy_response_headers_to_ngx(
+
-    ngx_http_request_t* r,
+enum PreserveCachingHeaders {
-    const net_instaweb::ResponseHeaders& pagespeed_headers);
+  kPreserveAllCachingHeaders,  // Cache-Control, ETag, Last-Modified, etc
+  kPreserveOnlyCacheControl,   // Only Cache-Control.
+  kDontPreserveHeaders,
+};
 
 typedef struct {
-  net_instaweb::ProxyFetch* proxy_fetch;
+  NgxBaseFetch* base_fetch;
-  net_instaweb::NgxBaseFetch* base_fetch;
+
-  net_instaweb::RewriteDriver* driver;
-  bool data_received;
-  int pipe_fd;
-  ngx_connection_t* pagespeed_connection;
   ngx_http_request_t* r;
-  bool is_resource_fetch;
+
-  bool sent_headers;
+  bool html_rewrite;
-  bool write_pending;
+  bool in_place;
-  net_instaweb::GzipInflater* inflater_;
+
+  PreserveCachingHeaders preserve_caching_headers;
+
+  // for html rewrite
+  ProxyFetch* proxy_fetch;
+  GzipInflater* inflater_;
+
+  // for in place resource
+  RewriteDriver* driver;
+  InPlaceResourceRecorder* recorder;
+  ResponseHeaders* ipro_response_headers;
+
+  // 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;
+  bool follow_flushes;
 } ps_request_ctx_t;
 
-}  // namespace ngx_psol
+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);
+
+void copy_response_headers_from_ngx(const ngx_http_request_t* r,
+                                    ResponseHeaders* headers);
+
+ngx_int_t copy_response_headers_to_ngx(
+    ngx_http_request_t* r,
+    const ResponseHeaders& pagespeed_headers,
+    PreserveCachingHeaders preserve_caching_headers);
+
+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_request_context.cc

@@ -1,82 +0,0 @@
-/*
- * Copyright 2013 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.
- */
-
-#include "ngx_request_context.h"
-
-extern "C" {
-  #include <ngx_http.h>
-}
-
-#include "ngx_pagespeed.h"
-
-#include "base/logging.h"
-#include "net/instaweb/http/public/meta_data.h"
-
-namespace net_instaweb {
-
-NgxRequestContext::NgxRequestContext(AbstractMutex* logging_mutex,
-                                     Timer* timer,
-                                     ngx_http_request_t* r)
-    : RequestContext(logging_mutex, timer),
-      local_port_(-1) {
-  // Note that at the time we create a RequestContext we have full
-  // access to the nginx internal request structure.  However,
-  // due to Cloning and (I believe) Detaching, we can initiate fetches after
-  // http_http_request_t* has been retired.  So deep-copy the bits we need
-  // at the time we create our RequestContext.
-
-  // Save our own IP as well, LoopbackRouteFetcher will need it.
-
-  // Based on ngx_http_variable_server_port.
-  bool port_set = false;
-#if (NGX_HAVE_INET6)
-  if (r->connection->local_sockaddr->sa_family == AF_INET6) {
-    local_port_ = ntohs(reinterpret_cast<struct sockaddr_in6*>(
-        r->connection->local_sockaddr)->sin6_port);
-    port_set = true;
-  }
-#endif
-  if (!port_set) {
-    local_port_ = ntohs(reinterpret_cast<struct sockaddr_in*>(
-        r->connection->local_sockaddr)->sin_port);
-  }
-
-  ngx_str_t  s;
-  u_char addr[NGX_SOCKADDR_STRLEN];
-  s.len = NGX_SOCKADDR_STRLEN;
-  s.data = addr;
-  ngx_int_t rc = ngx_connection_local_sockaddr(r->connection, &s, 0);
-  if (rc != NGX_OK) {
-    s.len = 0;
-  }
-  local_ip_ =  ngx_psol::str_to_string_piece(s).as_string();
-}
-
-NgxRequestContext::~NgxRequestContext() {
-}
-
-NgxRequestContext* NgxRequestContext::DynamicCast(RequestContext* rc) {
-  if (rc == NULL) {
-    return NULL;
-  }
-  NgxRequestContext* out = dynamic_cast<NgxRequestContext*>(rc);
-  DCHECK(out != NULL) << "Invalid request conversion. Do not rely on RTTI for "
-                      << "functional behavior. Ngx handling flows must use "
-                      << "NgxRequestContexts.";
-  return out;
-}
-
-}  // namespace net_instaweb

src/ngx_request_context.h

@@ -1,64 +0,0 @@
-/*
- * Copyright 2013 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@google.com (Otto van der Schaaf)
-//
-// I tried to keep this as close to ApacheRequestContext as possible.
-// Captures the NGINX request details in our request context, including
-// the port (used for loopback fetches).
-
-#ifndef NGX_REQUEST_CONTEXT_H_
-#define NGX_REQUEST_CONTEXT_H_
-
-#include "ngx_pagespeed.h"
-
-#include "net/instaweb/http/public/request_context.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/string.h"
-#include "net/instaweb/util/public/string_util.h"
-
-
-namespace net_instaweb {
-
-class AbstractMutex;
-class Timer;
-
-class NgxRequestContext : public RequestContext {
- public:
-  NgxRequestContext(AbstractMutex* logging_mutex,
-                    Timer* timer,
-                    ngx_http_request_t* ps_request_context);
-
-  // Returns rc as an NgxRequestContext* if it is one and CHECK
-  // fails if it is not. Returns NULL if rc is NULL.
-  static NgxRequestContext* DynamicCast(RequestContext* rc);
-
-  int local_port() const { return local_port_; }
-  const GoogleString& local_ip() const { return local_ip_; }
-
- protected:
-  virtual ~NgxRequestContext();
-
- private:
-  int local_port_;
-  GoogleString local_ip_;
-
-  DISALLOW_COPY_AND_ASSIGN(NgxRequestContext);
-};
-
-}  // namespace net_instaweb
-
-#endif  // NGX_REQUEST_CONTEXT_H_

src/ngx_rewrite_driver_factory.cc

@@ -24,33 +24,32 @@
 #include "ngx_message_handler.h"
 #include "ngx_rewrite_options.h"
 #include "ngx_server_context.h"
-#include "ngx_thread_system.h"
 #include "ngx_url_async_fetcher.h"
-#include "pthread_shared_mem.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/rewriter/public/static_asset_manager.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 "pagespeed/kernel/base/google_message_handler.h"
-#include "net/instaweb/util/public/shared_circular_buffer.h"
+#include "pagespeed/kernel/base/null_shared_mem.h"
-#include "net/instaweb/util/public/shared_mem_statistics.h"
+#include "pagespeed/kernel/base/posix_timer.h"
-#include "net/instaweb/util/public/slow_worker.h"
+#include "pagespeed/kernel/base/stdio_file_system.h"
-#include "net/instaweb/util/public/stdio_file_system.h"
+#include "pagespeed/kernel/base/string.h"
-#include "net/instaweb/util/public/string.h"
+#include "pagespeed/kernel/base/string_util.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/thread_system.h"
-#include "net/instaweb/util/public/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 {
 
@@ -63,36 +62,29 @@ class UrlAsyncFetcher;
 class UrlFetcher;
 class Writer;
 
-const char NgxRewriteDriverFactory::kStaticAssetPrefix[] =
+class SharedCircularBuffer;
-    "/ngx_pagespeed_static/";
-
-namespace {
-
-const char kShutdownCount[] = "child_shutdown_count";
-
-}  // namespace
 
 NgxRewriteDriverFactory::NgxRewriteDriverFactory(
-    NgxThreadSystem* ngx_thread_system)
+    const ProcessContext& process_context,
-    : SystemRewriteDriverFactory(ngx_thread_system),
+    SystemThreadSystem* system_thread_system, StringPiece hostname, int port)
-      ngx_thread_system_(ngx_thread_system),
+    : SystemRewriteDriverFactory(process_context, system_thread_system,
-      // TODO(oschaaf): mod_pagespeed ifdefs this:
+        NULL /* default shared memory runtime */, hostname, port),
-      shared_mem_runtime_(new ngx::PthreadSharedMem()),
-      main_conf_(NULL),
       threads_started_(false),
-      use_per_vhost_statistics_(false),
+      ngx_message_handler_(
-      is_root_process_(true),
+          new NgxMessageHandler(timer(), thread_system()->NewMutex())),
-      ngx_message_handler_(new NgxMessageHandler(thread_system()->NewMutex())),
       ngx_html_parse_message_handler_(
-          new NgxMessageHandler(thread_system()->NewMutex())),
+          new NgxMessageHandler(timer(), thread_system()->NewMutex())),
-      install_crash_handler_(false),
-      message_buffer_size_(0),
-      shared_circular_buffer_(NULL),
-      statistics_frozen_(false),
-      ngx_url_async_fetcher_(NULL),
       log_(NULL),
       resolver_timeout_(NGX_CONF_UNSET_MSEC),
-      use_native_fetcher_(false) {
+      use_native_fetcher_(false),
+      // 100 Aligns to nginx's server-side default.
+      native_fetcher_max_keepalive_requests_(100),
+      ngx_shared_circular_buffer_(NULL),
+      hostname_(hostname.as_string()),
+      port_(port),
+      process_script_variables_mode_(ProcessScriptVariablesMode::kOff),
+      process_script_variables_set_(false),
+      shut_down_(false) {
   InitializeDefaultOptions();
   default_options()->set_beacon_url("/ngx_pagespeed_beacon");
   SystemRewriteOptions* system_options = dynamic_cast<SystemRewriteOptions*>(
@@ -101,85 +93,35 @@ NgxRewriteDriverFactory::NgxRewriteDriverFactory(
   system_options->set_avoid_renaming_introspective_javascript(true);
   set_message_handler(ngx_message_handler_);
   set_html_parse_message_handler(ngx_html_parse_message_handler_);
-
-  // see https://code.google.com/p/modpagespeed/issues/detail?id=672
-  int thread_limit = 1;
-  caches_.reset(
-      new SystemCaches(this, shared_mem_runtime_.get(), thread_limit));
 }
 
 NgxRewriteDriverFactory::~NgxRewriteDriverFactory() {
   ShutDown();
-
+  ngx_shared_circular_buffer_ = NULL;
-  CHECK(uninitialized_server_contexts_.empty() || is_root_process_);
   STLDeleteElements(&uninitialized_server_contexts_);
-  shared_mem_statistics_.reset(NULL);
 }
 
 Hasher* NgxRewriteDriverFactory::NewHasher() {
   return new MD5Hasher;
 }
 
-UrlAsyncFetcher* NgxRewriteDriverFactory::DefaultAsyncUrlFetcher() {
+UrlAsyncFetcher* NgxRewriteDriverFactory::AllocateFetcher(
-  const char* fetcher_proxy = "";
+    SystemRewriteOptions* config) {
-  if (main_conf_ != NULL) {
-    fetcher_proxy = main_conf_->fetcher_proxy().c_str();
-  }
-
-  UrlAsyncFetcher* fetcher = NULL;
-
   if (use_native_fetcher_) {
-    ngx_url_async_fetcher_ =
+    NgxUrlAsyncFetcher* fetcher = new NgxUrlAsyncFetcher(
-        new net_instaweb::NgxUrlAsyncFetcher(
+        config->fetcher_proxy().c_str(),
-            fetcher_proxy,
         log_,
         resolver_timeout_,
-            25000,
+        config->blocking_fetch_timeout_ms(),
         resolver_,
+        native_fetcher_max_keepalive_requests_,
         thread_system(),
         message_handler());
-    fetcher = ngx_url_async_fetcher_;
+    ngx_url_async_fetchers_.push_back(fetcher);
-  } else {
-    net_instaweb::SerfUrlAsyncFetcher* serf_fetcher =
-        new net_instaweb::SerfUrlAsyncFetcher(
-            fetcher_proxy,
-            NULL,
-            thread_system(),
-            statistics(),
-            timer(),
-            2500,
-            message_handler());
-    fetcher = serf_fetcher;
-  }
-
-  SystemRewriteOptions* system_options = dynamic_cast<SystemRewriteOptions*>(
-      default_options());
-  if (rate_limit_background_fetches_) {
-    // Unfortunately, we need stats for load-shedding.
-    if (system_options->statistics_enabled()) {
-      // TODO(oschaaf): mps bases this multiplier on the configured
-      // num_rewrite_threads_ which we don't have (yet).
-      int multiplier = 4;
-      fetcher = new RateControllingUrlAsyncFetcher(
-          fetcher,
-          500 * multiplier /* max queue size */,
-          multiplier /* requests/host */,
-          500 * multiplier /* queued per host */,
-          thread_system(),
-          statistics());
-      if (ngx_url_async_fetcher_ == NULL) {
-        defer_cleanup(new Deleter<SerfUrlAsyncFetcher>(
-            static_cast<net_instaweb::SerfUrlAsyncFetcher*>(fetcher)));
-      } else  {
-        defer_cleanup(new Deleter<net_instaweb::NgxUrlAsyncFetcher>(
-            ngx_url_async_fetcher_));
-      }
-    } else {
-      message_handler()->Message(
-          kError, "Can't enable fetch rate-limiting without statistics");
-    }
-  }
     return fetcher;
+  } else {
+    return SystemRewriteDriverFactory::AllocateFetcher(config);
+  }
 }
 
 MessageHandler* NgxRewriteDriverFactory::DefaultHtmlParseMessageHandler() {
@@ -203,44 +145,19 @@ NamedLockManager* NgxRewriteDriverFactory::DefaultLockManager() {
   return NULL;
 }
 
-void NgxRewriteDriverFactory::SetupCaches(ServerContext* server_context) {
-  // TODO(anupama): Remove duplication wrt mod_pagespeed code.
-  caches_->SetupCaches(server_context);
-
-  server_context->set_enable_property_cache(true);
-  PropertyCache* pcache = server_context->page_property_cache();
-  const PropertyCache::Cohort* cohort =
-    pcache->AddCohort(RewriteDriver::kBeaconCohort);
-  server_context->set_beacon_cohort(cohort);
-
-  cohort = pcache->AddCohort(RewriteDriver::kDomCohort);
-  server_context->set_dom_cohort(cohort);
-}
-
 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;
 }
 
-
+RewriteOptions* NgxRewriteDriverFactory::NewRewriteOptionsForQuery() {
-void NgxRewriteDriverFactory::InitStaticAssetManager(
+  return new NgxRewriteOptions(thread_system());
-    StaticAssetManager* static_asset_manager) {
-  static_asset_manager->set_library_url_prefix(kStaticAssetPrefix);
-}
-
-void NgxRewriteDriverFactory::PrintMemCacheStats(GoogleString* out) {
-  // TODO(morlovich): Port the client code to proper API, so it gets
-  // shm stats, too.
-  caches_->PrintCacheStats(SystemCaches::kIncludeMemcached, out);
-}
-
-bool NgxRewriteDriverFactory::InitNgxUrlAsyncFetcher() {
-  if (ngx_url_async_fetcher_ == NULL) {
-    return true;
-  }
-  log_ = ngx_cycle->log;
-  return ngx_url_async_fetcher_->Init();
 }
 
 bool NgxRewriteDriverFactory::CheckResolver() {
@@ -250,52 +167,46 @@ bool NgxRewriteDriverFactory::CheckResolver() {
   return true;
 }
 
-void NgxRewriteDriverFactory::StopCacheActivity() {
+NgxServerContext* NgxRewriteDriverFactory::MakeNgxServerContext(
-  RewriteDriverFactory::StopCacheActivity();
+    StringPiece hostname, int port) {
-  caches_->StopCacheActivity();
+  NgxServerContext* server_context = new NgxServerContext(this, hostname, port);
-}
-
-NgxServerContext* NgxRewriteDriverFactory::MakeNgxServerContext() {
-  NgxServerContext* server_context = new NgxServerContext(this);
   uninitialized_server_contexts_.insert(server_context);
   return server_context;
 }
 
+ServerContext* NgxRewriteDriverFactory::NewDecodingServerContext() {
+  ServerContext* sc = new NgxServerContext(this, hostname_, port_);
+  InitStubDecodingServerContext(sc);
+  return sc;
+}
+
 ServerContext* NgxRewriteDriverFactory::NewServerContext() {
   LOG(DFATAL) << "MakeNgxServerContext should be used instead";
   return NULL;
 }
 
 void NgxRewriteDriverFactory::ShutDown() {
-  StopCacheActivity();
+  if (!shut_down_) {
-  if (!is_root_process_) {
+    shut_down_ = true;
-    Variable* child_shutdown_count = statistics()->GetVariable(kShutdownCount);
+    SystemRewriteDriverFactory::ShutDown();
-    child_shutdown_count->Add(1);
+  }
 }
 
-  RewriteDriverFactory::ShutDown();
+void NgxRewriteDriverFactory::ShutDownMessageHandlers() {
-  caches_->ShutDown(message_handler());
-
   ngx_message_handler_->set_buffer(NULL);
   ngx_html_parse_message_handler_->set_buffer(NULL);
-
+  for (NgxMessageHandlerSet::iterator p =
-  if (is_root_process_) {
+           server_context_message_handlers_.begin();
-    // Cleanup statistics.
+       p != server_context_message_handlers_.end(); ++p) {
-    // TODO(morlovich): This looks dangerous with async.
+    (*p)->set_buffer(NULL);
-    if (shared_mem_statistics_.get() != NULL) {
-      shared_mem_statistics_->GlobalCleanup(message_handler());
-    }
-    if (shared_circular_buffer_ != NULL) {
-      shared_circular_buffer_->GlobalCleanup(message_handler());
-    }
   }
+  server_context_message_handlers_.clear();
 }
 
 void NgxRewriteDriverFactory::StartThreads() {
   if (threads_started_) {
     return;
   }
-  ngx_thread_system_->PermitThreadStarting();
   // TODO(jefftk): use a native nginx timer instead of running our own thread.
   // See issue #111.
   SchedulerThread* thread = new SchedulerThread(thread_system(), scheduler());
@@ -305,107 +216,43 @@ void NgxRewriteDriverFactory::StartThreads() {
   threads_started_ = true;
 }
 
-void NgxRewriteDriverFactory::ParentOrChildInit(ngx_log_t* log) {
+void NgxRewriteDriverFactory::SetMainConf(NgxRewriteOptions* main_options) {
-  if (install_crash_handler_) {
+  // 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 (may_install_crash_handler && install_crash_handler()) {
     NgxMessageHandler::InstallCrashHandler(log);
   }
   ngx_message_handler_->set_log(log);
   ngx_html_parse_message_handler_->set_log(log);
-  SharedCircularBufferInit(is_root_process_);
 }
 
-// TODO(jmarantz): make this per-vhost.
+void NgxRewriteDriverFactory::SetCircularBuffer(
-void NgxRewriteDriverFactory::SharedCircularBufferInit(bool is_root) {
+    SharedCircularBuffer* buffer) {
-  // Set buffer size to 0 means turning it off
+  ngx_shared_circular_buffer_ = buffer;
-  if (shared_mem_runtime() != NULL && (message_buffer_size_ != 0)) {
+  ngx_message_handler_->set_buffer(buffer);
-    // TODO(jmarantz): it appears that filename_prefix() is not actually
+  ngx_html_parse_message_handler_->set_buffer(buffer);
-    // established at the time of this construction, calling into question
-    // whether we are naming our shared-memory segments correctly.
-    shared_circular_buffer_.reset(new SharedCircularBuffer(
-        shared_mem_runtime(),
-        message_buffer_size_,
-        filename_prefix().as_string(),
-        "foo.com" /*hostname_identifier()*/));
-    if (shared_circular_buffer_->InitSegment(is_root, message_handler())) {
-      ngx_message_handler_->set_buffer(shared_circular_buffer_.get());
-      ngx_html_parse_message_handler_->set_buffer(
-          shared_circular_buffer_.get());
-    }
-  }
 }
 
-void NgxRewriteDriverFactory::RootInit(ngx_log_t* log) {
+void NgxRewriteDriverFactory::SetServerContextMessageHandler(
-  net_instaweb::log_message_handler::Install(log);
+    ServerContext* server_context, ngx_log_t* log) {
-
+  NgxMessageHandler* handler = new NgxMessageHandler(
-  ParentOrChildInit(log);
+      timer(), thread_system()->NewMutex());
-
+  handler->set_log(log);
-  // Let SystemCaches know about the various paths we have in configuration
+  // The ngx_shared_circular_buffer_ will be NULL if MessageBufferSize hasn't
-  // first, as well as the memcached instances.
+  // been raised from its default of 0.
-  for (NgxServerContextSet::iterator p = uninitialized_server_contexts_.begin(),
+  handler->set_buffer(ngx_shared_circular_buffer_);
-           e = uninitialized_server_contexts_.end(); p != e; ++p) {
+  server_context_message_handlers_.insert(handler);
-    NgxServerContext* server_context = *p;
+  defer_cleanup(new Deleter<NgxMessageHandler>(handler));
-    caches_->RegisterConfig(server_context->config());
+  server_context->set_message_handler(handler);
-  }
-
-  caches_->RootInit();
-}
-
-void NgxRewriteDriverFactory::ChildInit(ngx_log_t* log) {
-  is_root_process_ = false;
-
-  ParentOrChildInit(log);
-  if (shared_mem_statistics_.get() != NULL) {
-    shared_mem_statistics_->Init(false, message_handler());
-  }
-
-  caches_->ChildInit();
-  for (NgxServerContextSet::iterator p = uninitialized_server_contexts_.begin(),
-           e = uninitialized_server_contexts_.end(); p != e; ++p) {
-    NgxServerContext* server_context = *p;
-    server_context->ChildInit();
-  }
-
-  uninitialized_server_contexts_.clear();
-}
-
-// Initializes global statistics object if needed, using factory to
-// help with the settings if needed.
-// Note: does not call set_statistics() on the factory.
-Statistics* NgxRewriteDriverFactory::MakeGlobalSharedMemStatistics(
-    const NgxRewriteOptions& options) {
-  if (shared_mem_statistics_.get() == NULL) {
-    shared_mem_statistics_.reset(AllocateAndInitSharedMemStatistics(
-        "global", options));
-  }
-  DCHECK(!statistics_frozen_);
-  statistics_frozen_ = true;
-  SetStatistics(shared_mem_statistics_.get());
-  return shared_mem_statistics_.get();
-}
-
-SharedMemStatistics* NgxRewriteDriverFactory::
-AllocateAndInitSharedMemStatistics(
-    const StringPiece& name,
-    const NgxRewriteOptions& options) {
-  GoogleString log_filename;
-  bool logging_enabled = false;
-  if (!options.log_dir().empty()) {
-    // Only enable statistics logging if a log_dir() is actually specified.
-    log_filename = StrCat(options.log_dir(), "/stats_log_", name);
-    logging_enabled = options.statistics_logging_enabled();
-  }
-  // Note that we create the statistics object in the parent process, and
-  // it stays around in the kids but gets reinitialized for them
-  // inside ChildInit(), called from pagespeed_child_init.
-  SharedMemStatistics* stats = new SharedMemStatistics(
-      options.statistics_logging_interval_ms(),
-      options.statistics_logging_max_file_size_kb(),
-      log_filename, logging_enabled,
-      StrCat(filename_prefix(), name), shared_mem_runtime(), message_handler(),
-      file_system(), timer());
-  InitStats(stats);
-  stats->Init(true, message_handler());
-  return stats;
 }
 
 void NgxRewriteDriverFactory::InitStats(Statistics* statistics) {
@@ -416,8 +263,26 @@ void NgxRewriteDriverFactory::InitStats(Statistics* statistics) {
 
   // Init Ngx-specific stats.
   NgxServerContext::InitStats(statistics);
+  InPlaceResourceRecorder::InitStats(statistics);
+}
 
-  statistics->AddVariable(kShutdownCount);
+void NgxRewriteDriverFactory::PrepareForkedProcess(const char* name) {
+  ngx_pid = ngx_getpid();  // Needed for logging to have the right PIDs.
+  SystemRewriteDriverFactory::PrepareForkedProcess(name);
+}
+
+void NgxRewriteDriverFactory::NameProcess(const char* name) {
+  SystemRewriteDriverFactory::NameProcess(name);
+
+  // Superclass set status with prctl.  Nginx has a helper function for setting
+  // argv[0] as well, so let's use that.  We'll show up as:
+  //
+  //    nginx: pagespeed $name
+
+  char name_for_setproctitle[32];
+  snprintf(name_for_setproctitle, sizeof(name_for_setproctitle),
+           "pagespeed %s", name);
+  ngx_setproctitle(name_for_setproctitle);
 }
 
 }  // namespace net_instaweb

src/ngx_rewrite_driver_factory.h

@@ -20,6 +20,10 @@
 #define NGX_REWRITE_DRIVER_FACTORY_H_
 
 extern "C" {
+  #include <ngx_auto_config.h>
+#if (NGX_THREADS)
+  #include <ngx_thread.h>
+#endif
   #include <ngx_core.h>
   #include <ngx_http.h>
   #include <ngx_config.h>
@@ -28,130 +32,73 @@ extern "C" {
 
 #include <set>
 
-#include "apr_pools.h"
+#include "pagespeed/kernel/base/md5_hasher.h"
-#include "net/instaweb/system/public/system_rewrite_driver_factory.h"
+#include "pagespeed/kernel/base/scoped_ptr.h"
-#include "net/instaweb/util/public/md5_hasher.h"
+#include "pagespeed/system/system_rewrite_driver_factory.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.
 
 namespace net_instaweb {
 
-class AbstractSharedMem;
 class NgxMessageHandler;
 class NgxRewriteOptions;
 class NgxServerContext;
-class NgxThreadSystem;
 class NgxUrlAsyncFetcher;
 class SharedCircularBuffer;
 class SharedMemRefererStatistics;
-class SharedMemStatistics;
 class SlowWorker;
-class StaticAssetManager;
 class Statistics;
-class SystemCaches;
+class SystemThreadSystem;
+
+enum ProcessScriptVariablesMode {
+  kOff,
+  kLegacyRestricted,
+  kAll
+};
 
 class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
  public:
-  static const char kStaticAssetPrefix[];
-
   // We take ownership of the thread system.
-  explicit NgxRewriteDriverFactory(NgxThreadSystem* ngx_thread_system);
+  explicit NgxRewriteDriverFactory(
+      const ProcessContext& process_context,
+      SystemThreadSystem* system_thread_system, StringPiece hostname, int port);
   virtual ~NgxRewriteDriverFactory();
   virtual Hasher* NewHasher();
-  virtual UrlAsyncFetcher* DefaultAsyncUrlFetcher();
+  virtual UrlAsyncFetcher* AllocateFetcher(SystemRewriteOptions* config);
   virtual MessageHandler* DefaultHtmlParseMessageHandler();
   virtual MessageHandler* DefaultMessageHandler();
   virtual FileSystem* DefaultFileSystem();
   virtual Timer* DefaultTimer();
   virtual NamedLockManager* DefaultLockManager();
-  virtual void SetupCaches(ServerContext* server_context);
   // Create a new RewriteOptions.  In this implementation it will be an
-  // NgxRewriteOptions.
+  // NgxRewriteOptions, and it will have CoreFilters explicitly set.
   virtual RewriteOptions* NewRewriteOptions();
-  // Initializes the StaticAssetManager.
+  virtual RewriteOptions* NewRewriteOptionsForQuery();
-  virtual void InitStaticAssetManager(
+  virtual ServerContext* NewDecodingServerContext();
-      StaticAssetManager* static_asset_manager);
-  // Print out details of all the connections to memcached servers.
-  void PrintMemCacheStats(GoogleString* out);
-  bool InitNgxUrlAsyncFetcher();
   // Check resolver configured or not.
   bool CheckResolver();
 
-  // Release all the resources. It also calls the base class ShutDown to
-  // release the base class resources.
   // Initializes all the statistics objects created transitively by
   // NgxRewriteDriverFactory, including nginx-specific and
   // platform-independent statistics.
   static void InitStats(Statistics* statistics);
+  NgxServerContext* MakeNgxServerContext(StringPiece hostname, int port);
+  virtual ServerContext* NewServerContext();
   virtual void ShutDown();
-  virtual void StopCacheActivity();
-  NgxServerContext* MakeNgxServerContext();
-  ServerContext* NewServerContext();
-  AbstractSharedMem* shared_mem_runtime() const {
-    return shared_mem_runtime_.get();
-  }
-
-  SystemCaches* caches() { return caches_.get(); }
 
   // Starts pagespeed threads if they've not been started already.  Must be
   // called after the caller has finished any forking it intends to do.
   void StartThreads();
-  // This helper method contains init procedures invoked by both RootInit()
-  // and ChildInit()
-  void ParentOrChildInit(ngx_log_t* log);
-  // For shared memory resources the general setup we follow is to have the
-  // first running process (aka the root) create the necessary segments and
-  // fill in their shared data structures, while processes created to actually
-  // handle requests attach to already existing shared data structures.
-  //
-  // During normal server startup[1], RootInit() is called from the nginx hooks
-  // in the root process for the first task, and then ChildInit() is called in
-  // any child process.
-  //
-  // Keep in mind, however, that when fork() is involved a process may
-  // effectively see both calls, in which case the 'ChildInit' call would
-  // come second and override the previous root status. Both calls are also
-  // invoked in the debug single-process mode.
-  //
-  // [1] Besides normal startup, nginx also uses a temporary process to
-  // syntax check the config file. That basically looks like a complete
-  // normal startup and shutdown to the code.
-  bool is_root_process() const { return is_root_process_; }
-  void RootInit(ngx_log_t* log);
-  void ChildInit(ngx_log_t* log);
-  void SharedCircularBufferInit(bool is_root);
-  // Build global shared-memory statistics.  This is invoked if at least
-  // one server context (global or VirtualHost) enables statistics.
-  Statistics* MakeGlobalSharedMemStatistics(const NgxRewriteOptions& options);
 
-  // Creates and ::Initializes a shared memory statistics object.
+  void SetServerContextMessageHandler(ServerContext* server_context,
-  SharedMemStatistics* AllocateAndInitSharedMemStatistics(
+                                      ngx_log_t* log);
-      const StringPiece& name, const NgxRewriteOptions& options);
 
   NgxMessageHandler* ngx_message_handler() { return ngx_message_handler_; }
-  void set_main_conf(NgxRewriteOptions* main_conf) {  main_conf_ = main_conf; }
 
-  bool use_per_vhost_statistics() const {
+  virtual void NonStaticInitStats(Statistics* statistics) {
-    return use_per_vhost_statistics_;
+    InitStats(statistics);
-  }
-  void set_use_per_vhost_statistics(bool x) {
-    use_per_vhost_statistics_ = x;
-  }
-  bool install_crash_handler() const {
-    return install_crash_handler_;
-  }
-  void set_install_crash_handler(bool x) {
-    install_crash_handler_ = x;
-  }
-  bool message_buffer_size() const {
-    return message_buffer_size_;
-  }
-  void set_message_buffer_size(int x) {
-    message_buffer_size_ = x;
   }
+
+  void SetMainConf(NgxRewriteOptions* main_conf);
+
   void set_resolver(ngx_resolver_t* resolver) {
     resolver_ = resolver;
   }
@@ -165,52 +112,61 @@ class NgxRewriteDriverFactory : public SystemRewriteDriverFactory {
   void set_use_native_fetcher(bool x) {
     use_native_fetcher_ = x;
   }
-  void set_rate_limit_background_fetches(bool x) {
+  int native_fetcher_max_keepalive_requests() {
-    rate_limit_background_fetches_ = x;
+    return native_fetcher_max_keepalive_requests_;
+  }
+  void set_native_fetcher_max_keepalive_requests(int x) {
+    native_fetcher_max_keepalive_requests_ = x;
+  }
+  ProcessScriptVariablesMode process_script_variables() {
+    return process_script_variables_mode_;
   }
 
-  // We use a beacon handler to collect data for critical images,
+  void LoggingInit(ngx_log_t* log, bool may_install_crash_handler);
-  // css, etc., so filters should be configured accordingly.
+
-  //
+  virtual void ShutDownMessageHandlers();
-  // TODO(jefftk): move to SystemRewriteDriverFactory
+
-  virtual bool UseBeaconResultsInFilters() const {
+  virtual void SetCircularBuffer(SharedCircularBuffer* buffer);
+
+  bool SetProcessScriptVariables(ProcessScriptVariablesMode mode) {
+    if (!process_script_variables_set_) {
+      process_script_variables_mode_ = mode;
+      process_script_variables_set_ = true;
       return true;
     }
+    return false;
+  }
+
+  virtual void PrepareForkedProcess(const char* name);
+
+  virtual void NameProcess(const char* name);
 
  private:
-  NgxThreadSystem* ngx_thread_system_;
   Timer* timer_;
-  scoped_ptr<AbstractSharedMem> shared_mem_runtime_;
-
-  // main_conf will have only options set in the main block.  It may be NULL,
-  // and we do not take ownership.
-  NgxRewriteOptions* main_conf_;
-  typedef std::set<NgxServerContext*> NgxServerContextSet;
-  NgxServerContextSet uninitialized_server_contexts_;
-
-  // Manages all our caches & lock managers.
-  scoped_ptr<SystemCaches> caches_;
 
   bool threads_started_;
-  // If true, we'll have a separate statistics object for each vhost
-  // (along with a global aggregate), rather than just a single object
-  // aggregating all of them.
-  bool use_per_vhost_statistics_;
-  bool is_root_process_;
   NgxMessageHandler* ngx_message_handler_;
   NgxMessageHandler* ngx_html_parse_message_handler_;
-  bool install_crash_handler_;
-  int message_buffer_size_;
-  scoped_ptr<SharedCircularBuffer> shared_circular_buffer_;
-  scoped_ptr<SharedMemStatistics> shared_mem_statistics_;
-  bool statistics_frozen_;
 
-  NgxUrlAsyncFetcher* ngx_url_async_fetcher_;
+  std::vector<NgxUrlAsyncFetcher*> ngx_url_async_fetchers_;
   ngx_log_t* log_;
   ngx_msec_t resolver_timeout_;
   ngx_resolver_t* resolver_;
   bool use_native_fetcher_;
-  bool rate_limit_background_fetches_;
+  int native_fetcher_max_keepalive_requests_;
+
+  typedef std::set<NgxMessageHandler*> NgxMessageHandlerSet;
+  NgxMessageHandlerSet server_context_message_handlers_;
+
+  // Owned by the superclass.
+  // TODO(jefftk): merge the nginx and apache ways of doing this.
+  SharedCircularBuffer* ngx_shared_circular_buffer_;
+
+  GoogleString hostname_;
+  int port_;
+  ProcessScriptVariablesMode process_script_variables_mode_;
+  bool process_script_variables_set_;
+  bool shut_down_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxRewriteDriverFactory);
 };

src/ngx_rewrite_options.cc

@@ -30,19 +30,68 @@ 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 "pagespeed/kernel/base/message_handler.h"
-#include "net/instaweb/util/public/timer.h"
+#include "pagespeed/kernel/base/timer.h"
+#include "pagespeed/system/system_caches.h"
 
 namespace net_instaweb {
 
 namespace {
 
-const char kNgxPagespeedStatisticsHandlerPath[] = "/ngx_pagespeed_statistics";
+const char kStatisticsPath[] = "StatisticsPath";
+const char kGlobalStatisticsPath[] = "GlobalStatisticsPath";
+const char kConsolePath[] = "ConsolePath";
+const char kMessagesPath[] = "MessagesPath";
+const char kAdminPath[] = "AdminPath";
+const char kGlobalAdminPath[] = "GlobalAdminPath";
+
+// These options are copied from mod_instaweb.cc, where APACHE_CONFIG_OPTIONX
+// indicates that they can not be set at the directory/location level. They set
+// options in the RewriteDriverFactory, so they're entirely global and do not
+// appear in RewriteOptions.  They are not alphabetized on purpose, but rather
+// left in the same order as in mod_instaweb.cc in case we end up needing to
+// compare.
+// TODO(oschaaf): this duplication is a short term solution.
+const char* const server_only_options[] = {
+  "FetcherTimeoutMs",
+  "FetchProxy",
+  "ForceCaching",
+  "GeneratedFilePrefix",
+  "ImgMaxRewritesAtOnce",
+  "InheritVHostConfig",
+  "InstallCrashHandler",
+  "MessageBufferSize",
+  "NumRewriteThreads",
+  "NumExpensiveRewriteThreads",
+  "StaticAssetPrefix",
+  "TrackOriginalContentLength",
+  "UsePerVHostStatistics",  // TODO(anupama): What to do about "No longer used"
+  "BlockingRewriteRefererUrls",
+  "CreateSharedMemoryMetadataCache",
+  "LoadFromFile",
+  "LoadFromFileMatch",
+  "LoadFromFileRule",
+  "LoadFromFileRuleMatch",
+  "UseNativeFetcher",
+  "NativeFetcherMaxKeepaliveRequests"
+};
+
+// Options that can only be used in the main (http) option scope.
+const char* const main_only_options[] = {
+  "UseNativeFetcher",
+  "NativeFetcherMaxKeepaliveRequests"
+};
 
 }  // namespace
 
 RewriteOptions::Properties* NgxRewriteOptions::ngx_properties_ = NULL;
 
+NgxRewriteOptions::NgxRewriteOptions(const StringPiece& description,
+                                     ThreadSystem* thread_system)
+    : SystemRewriteOptions(description, thread_system) {
+  Init();
+}
+
 NgxRewriteOptions::NgxRewriteOptions(ThreadSystem* thread_system)
     : SystemRewriteOptions(thread_system) {
   Init();
@@ -51,30 +100,43 @@ NgxRewriteOptions::NgxRewriteOptions(ThreadSystem* thread_system)
 void NgxRewriteOptions::Init() {
   DCHECK(ngx_properties_ != NULL)
       << "Call NgxRewriteOptions::Initialize() before construction";
+  clear_inherited_scripts_ = false;
   InitializeOptions(ngx_properties_);
-
-  // Nginx-specific default.
-  // TODO(sligocki): Get rid of this line and let both Apache and Nginx use
-  // /pagespeed_statistics as the handler.
-  statistics_handler_path_.set_default(kNgxPagespeedStatisticsHandlerPath);
 }
 
 void NgxRewriteOptions::AddProperties() {
-  // Nothing ngx-specific for now.
+  // Nginx-specific options.
+  add_ngx_option(
+      "", &NgxRewriteOptions::statistics_path_, "nsp", kStatisticsPath,
+      kServerScope, "Set the statistics path. Ex: /ngx_pagespeed_statistics",
+      false);
+  add_ngx_option(
+      "", &NgxRewriteOptions::global_statistics_path_, "ngsp",
+      kGlobalStatisticsPath, kProcessScopeStrict,
+      "Set the global statistics path. Ex: /ngx_pagespeed_global_statistics",
+      false);
+  add_ngx_option(
+      "", &NgxRewriteOptions::console_path_, "ncp", kConsolePath, kServerScope,
+      "Set the console path. Ex: /pagespeed_console", false);
+  add_ngx_option(
+      "", &NgxRewriteOptions::messages_path_, "nmp", kMessagesPath,
+      kServerScope, "Set the messages path.  Ex: /ngx_pagespeed_message",
+      false);
+  add_ngx_option(
+      "", &NgxRewriteOptions::admin_path_, "nap", kAdminPath,
+      kServerScope, "Set the admin path.  Ex: /pagespeed_admin", false);
+  add_ngx_option(
+      "", &NgxRewriteOptions::global_admin_path_, "ngap", kGlobalAdminPath,
+      kProcessScopeStrict,
+      "Set the global admin path.  Ex: /pagespeed_global_admin",
+      false);
 
   MergeSubclassProperties(ngx_properties_);
-  // We create a dummy NgxRewriteOptions object here so that we can
+
-  // call InitializeSignaturesAndDefaults on it, and in turn get the
+  // Default properties are global but to set them the current API requires
-  // global defaults (for say, X-Page-Speed header value) setup correctly.
+  // a RewriteOptions instance and we're in a static method.
   NgxRewriteOptions dummy_config(NULL);
-  dummy_config.InitializeSignaturesAndDefaults();
+  dummy_config.set_default_x_header_value(kModPagespeedVersion);
-}
-
-void NgxRewriteOptions::InitializeSignaturesAndDefaults() {
-  // Calls to foo_.DoNotUseForSignatureComputation() would go here.
-
-  // Set default header value.
-  set_default_x_header_value(kModPagespeedVersion);
 }
 
 void NgxRewriteOptions::Initialize() {
@@ -95,6 +157,39 @@ bool NgxRewriteOptions::IsDirective(StringPiece config_directive,
   return StringCaseEqual(config_directive, compare_directive);
 }
 
+RewriteOptions::OptionScope NgxRewriteOptions::GetOptionScope(
+    StringPiece option_name) {
+  ngx_uint_t i;
+  ngx_uint_t size = sizeof(main_only_options) / sizeof(char*);
+  for (i = 0; i < size; i++) {
+    if (StringCaseEqual(main_only_options[i], option_name)) {
+      return kProcessScopeStrict;
+    }
+  }
+
+  size = sizeof(server_only_options) / sizeof(char*);
+  for (i = 0; i < size; i++) {
+    if (StringCaseEqual(server_only_options[i], option_name)) {
+      return kServerScope;
+    }
+  }
+
+  // This could be made more efficient if RewriteOptions provided a map allowing
+  // access of options by their name. It's not too much of a worry at present
+  // since this is just during initialization.
+  for (OptionBaseVector::const_iterator it = all_options().begin();
+       it != all_options().end(); ++it) {
+    RewriteOptions::OptionBase* option = *it;
+    if (option->option_name() == option_name) {
+      // We treat kLegacyProcessScope as kProcessScopeStrict, failing to start
+      // if an option is out of place.
+      return option->scope() == kLegacyProcessScope ? kProcessScopeStrict
+                                                    : option->scope();
+    }
+  }
+  return kDirectoryScope;
+}
+
 RewriteOptions::OptionSettingResult NgxRewriteOptions::ParseAndSetOptions0(
     StringPiece directive, GoogleString* msg, MessageHandler* handler) {
   if (IsDirective(directive, "on")) {
@@ -109,31 +204,63 @@ RewriteOptions::OptionSettingResult NgxRewriteOptions::ParseAndSetOptions0(
   return RewriteOptions::kOptionOk;
 }
 
-
 RewriteOptions::OptionSettingResult
-    NgxRewriteOptions::ParseAndSetOptionFromEnum1(
+    NgxRewriteOptions::ParseAndSetOptionFromName1(
-        OptionEnum directive, StringPiece arg,
+        StringPiece name, StringPiece arg,
         GoogleString* msg, MessageHandler* handler) {
   // FileCachePath needs error checking.
-  if (directive == kFileCachePath) {
+  if (StringCaseEqual(name, kFileCachePath)) {
     if (!StringCaseStartsWith(arg, "/")) {
       *msg = "must start with a slash";
       return RewriteOptions::kOptionValueInvalid;
     }
   }
 
-  // TODO(jefftk): port these (no enums for them yet, even!)
+  return SystemRewriteOptions::ParseAndSetOptionFromName1(
-  //  DangerPermitFetchFromUnknownHosts, FetchWithGzip, ForceCaching
+      name, arg, msg, handler);
-
-  return SystemRewriteOptions::ParseAndSetOptionFromEnum1(
-      directive, arg, msg, handler);
 }
 
+template <class DriverFactoryT>
+RewriteOptions::OptionSettingResult ParseAndSetOptionHelper(
+    StringPiece option_value,
+    DriverFactoryT* driver_factory,
+    void (DriverFactoryT::*set_option_method)(bool)) {
+  bool parsed_value;
+  if (StringCaseEqual(option_value, "on") ||
+      StringCaseEqual(option_value, "true")) {
+    parsed_value = true;
+  } else if (StringCaseEqual(option_value, "off") ||
+             StringCaseEqual(option_value, "false")) {
+    parsed_value = false;
+  } else {
+    return RewriteOptions::kOptionValueInvalid;
+  }
+
+  (driver_factory->*set_option_method)(parsed_value);
+  return RewriteOptions::kOptionOk;
+}
+
+namespace {
+
+const char* ps_error_string_for_option(
+    ngx_pool_t* pool, StringPiece directive, StringPiece warning) {
+  GoogleString msg =
+      StrCat("\"", directive, "\" ", warning);
+  char* s = string_piece_to_pool_string(pool, msg);
+  if (s == NULL) {
+    return "failed to allocate memory";
+  }
+  return s;
+}
+
+}  // namespace
+
 // Very similar to apache/mod_instaweb::ParseDirective.
-const char*
+const char* NgxRewriteOptions::ParseAndSetOptions(
-NgxRewriteOptions::ParseAndSetOptions(
     StringPiece* args, int n_args, ngx_pool_t* pool, MessageHandler* handler,
-    NgxRewriteDriverFactory* driver_factory) {
+    NgxRewriteDriverFactory* driver_factory,
+    RewriteOptions::OptionScope scope, ngx_conf_t* cf,
+    ProcessScriptVariablesMode script_mode) {
   CHECK_GE(n_args, 1);
 
   StringPiece directive = args[0];
@@ -144,111 +271,186 @@ NgxRewriteOptions::ParseAndSetOptions(
     directive.remove_prefix(mod_pagespeed.size());
   }
 
+  if (GetOptionScope(directive) > scope) {
+    return ps_error_string_for_option(
+        pool, directive, "cannot be set at this scope.");
+  }
+
+  bool compile_scripts = false;
+
+  if (script_mode != ProcessScriptVariablesMode::kOff) {
+    // In the old mode we only allowed a few, so restrict to those.
+    compile_scripts =
+        StringCaseStartsWith(directive, "LoadFromFile") ||
+        StringCaseEqual(directive, "EnableFilters") ||
+        StringCaseEqual(directive, "DisableFilters") ||
+        StringCaseEqual(directive, "DownstreamCachePurgeLocationPrefix") ||
+        StringCaseEqual(directive, "DownstreamCachePurgeMethod") ||
+        StringCaseEqual(directive,
+                        "DownstreamCacheRewrittenPercentageThreshold") ||
+        StringCaseEqual(directive, "ShardDomain");
+    // In the new behaviour we also allow scripting of query- and directory-
+    // scoped options.
+    compile_scripts |=
+        script_mode == ProcessScriptVariablesMode::kAll &&
+        (GetOptionScope(directive) <= RewriteOptions::kDirectoryScope ||
+         (StringCaseEqual(directive, "Allow") ||
+          StringCaseEqual(directive, "BlockingRewriteRefererUrls") ||
+          StringCaseEqual(directive, "Disallow") ||
+          StringCaseEqual(directive, "DistributableFilters") ||
+          StringCaseEqual(directive, "Domain") ||
+          StringCaseEqual(directive, "ExperimentVariable") ||
+          StringCaseEqual(directive, "ExperimentSpec") ||
+          StringCaseEqual(directive, "ForbidFilters") ||
+          StringCaseEqual(directive, "RetainComment") ||
+          StringCaseEqual(directive, "CustomFetchHeader") ||
+          StringCaseEqual(directive, "MapOriginDomain") ||
+          StringCaseEqual(directive, "MapProxyDomain") ||
+          StringCaseEqual(directive, "MapRewriteDomain") ||
+          StringCaseEqual(directive, "UrlValuedAttribute") ||
+          StringCaseEqual(directive, "Library")));
+  }
+
+  ScriptLine* script_line;
+  script_line = NULL;
+
+  if (n_args == 1 && StringCaseEqual(directive, "ClearInheritedScripts")) {
+    clear_inherited_scripts_ = true;
+    return NGX_CONF_OK;
+  }
+
+  if (compile_scripts) {
+    CHECK(cf != NULL);
+    int i;
+    // Skip the first arg which is always 'pagespeed'
+    for (i = 1; i < n_args; i++) {
+      ngx_str_t script_source;
+
+      script_source.len = args[i].as_string().length();
+      std::string tmp = args[i].as_string();
+      script_source.data = reinterpret_cast<u_char*>(
+          const_cast<char*>(tmp.c_str()));
+
+      if (ngx_http_script_variables_count(&script_source) > 0) {
+        ngx_http_script_compile_t* sc =
+            reinterpret_cast<ngx_http_script_compile_t*>(
+                ngx_pcalloc(cf->pool, sizeof(ngx_http_script_compile_t)));
+        sc->cf = cf;
+        sc->source = &script_source;
+        sc->lengths = reinterpret_cast<ngx_array_t**>(
+            ngx_pcalloc(cf->pool, sizeof(ngx_array_t*)));
+        sc->values = reinterpret_cast<ngx_array_t**>(
+            ngx_pcalloc(cf->pool, sizeof(ngx_array_t*)));
+        sc->variables = 1;
+        sc->complete_lengths = 1;
+        sc->complete_values = 1;
+        if (ngx_http_script_compile(sc) != NGX_OK) {
+          return ps_error_string_for_option(
+              pool, directive, "Failed to compile script variables");
+        } else {
+          if (script_line == NULL) {
+            script_line = new ScriptLine(args, n_args, scope);
+          }
+          script_line->AddScriptAndArgIndex(sc, i);
+        }
+      }
+    }
+
+    if (script_line != NULL) {
+      script_lines_.push_back(RefCountedPtr<ScriptLine>(script_line));
+      // We have found script variables in the current configuration line, and
+      // prepared the associated rewriteoptions for that.
+      // We will defer parsing, validation and processing of this line to
+      // request time. That means we are done handling this configuration line.
+      return NGX_CONF_OK;
+    }
+  }
+
   GoogleString msg;
   OptionSettingResult result;
   if (n_args == 1) {
     result = ParseAndSetOptions0(directive, &msg, handler);
   } else if (n_args == 2) {
     StringPiece arg = args[1];
-    // TODO(morlovich): Remove these special hacks, and handle these via
+    if (IsDirective(directive, "UseNativeFetcher")) {
-    // ParseAndSetOptionFromEnum1.
+      result = ParseAndSetOptionHelper<NgxRewriteDriverFactory>(
-    if (IsDirective(directive, "UsePerVHostStatistics")) {
+          arg, driver_factory,
-        // TODO(oschaaf): mod_pagespeed has a nicer way to do this.
+          &NgxRewriteDriverFactory::set_use_native_fetcher);
-        if (IsDirective(arg, "on")) {
+    } else if (IsDirective(directive, "NativeFetcherMaxKeepaliveRequests")) {
-          driver_factory->set_use_per_vhost_statistics(true);
+      int max_keepalive_requests;
-          result = RewriteOptions::kOptionOk;
+      if (StringToInt(arg, &max_keepalive_requests) &&
-        } else if (IsDirective(arg, "off")) {
+          max_keepalive_requests > 0) {
-          driver_factory->set_use_per_vhost_statistics(false);
+        driver_factory->set_native_fetcher_max_keepalive_requests(
+            max_keepalive_requests);
         result = RewriteOptions::kOptionOk;
       } else {
         result = RewriteOptions::kOptionValueInvalid;
       }
-      } else if (IsDirective(directive, "InstallCrashHandler")) {
+    } else if (StringCaseEqual("ProcessScriptVariables", args[0])) {
-        // TODO(oschaaf): mod_pagespeed has a nicer way to do this.
+      if (scope == RewriteOptions::kProcessScopeStrict) {
-        if (IsDirective(arg, "on")) {
+        ProcessScriptVariablesMode mode;
-          driver_factory->set_install_crash_handler(true);
+        if (StringCaseEqual(arg, "all")) {
-          result = RewriteOptions::kOptionOk;
+          mode = ProcessScriptVariablesMode::kAll;
-        } else if (IsDirective(arg, "off")) {
+        } else if (StringCaseEqual(arg, "on")) {
-          driver_factory->set_install_crash_handler(false);
+          mode = ProcessScriptVariablesMode::kLegacyRestricted;
-          result = RewriteOptions::kOptionOk;
+        } else if (StringCaseEqual(arg, "off")) {
+          mode = ProcessScriptVariablesMode::kOff;
         } else {
-          result = RewriteOptions::kOptionValueInvalid;
+          return const_cast<char*>(
+              "pagespeed ProcessScriptVariables: invalid value");
         }
-      } else if (IsDirective(directive, "MessageBufferSize")) {
+        if (driver_factory->SetProcessScriptVariables(mode)) {
-        // TODO(oschaaf): mod_pagespeed has a nicer way to do this.
-        int message_buffer_size;
-        bool ok = StringToInt(arg.as_string(), &message_buffer_size);
-        if (ok && message_buffer_size >= 0) {
-          driver_factory->set_message_buffer_size(message_buffer_size);
           result = RewriteOptions::kOptionOk;
         } else {
-          result = RewriteOptions::kOptionValueInvalid;
+          return const_cast<char*>(
-        }
+              "pagespeed ProcessScriptVariables: can only be set once");
-      } else if (IsDirective(directive, "UseNativeFetcher")) {
-        // TODO(oschaaf): mod_pagespeed has a nicer way to do this.
-        if (IsDirective(arg, "on")) {
-          driver_factory->set_use_native_fetcher(true);
-          result = RewriteOptions::kOptionOk;
-        } else if (IsDirective(arg, "off")) {
-          driver_factory->set_use_native_fetcher(false);
-          result = RewriteOptions::kOptionOk;
-        } else {
-          result = RewriteOptions::kOptionValueInvalid;
-        }
-      } else if (IsDirective(directive, "RateLimitBackgroundFetches")) {
-        // TODO(oschaaf): mod_pagespeed has a nicer way to do this.
-        if (IsDirective(arg, "on")) {
-          driver_factory->set_rate_limit_background_fetches(true);
-          result = RewriteOptions::kOptionOk;
-        } else if (IsDirective(arg, "off")) {
-          driver_factory->set_rate_limit_background_fetches(false);
-          result = RewriteOptions::kOptionOk;
-        } else {
-          result = RewriteOptions::kOptionValueInvalid;
         }
       } else {
-        result = ParseAndSetOptionFromName1(directive, args[1], &msg, handler);
+        return const_cast<char*>(
+            "ProcessScriptVariables is only allowed at the top level");
+      }
+    } else {
+      result = ParseAndSetOptionFromName1(directive, arg, &msg, handler);
+      if (result == RewriteOptions::kOptionNameUnknown) {
+        result = driver_factory->ParseAndSetOption1(
+            directive,
+            arg,
+            scope >= RewriteOptions::kLegacyProcessScope,
+            &msg,
+            handler);
+      }
     }
   } else if (n_args == 3) {
-    // Short-term special handling, until this moves to common code.
-    // TODO(morlovich): Clean this up.
-    if (StringCaseEqual(directive, "CreateSharedMemoryMetadataCache")) {
-      int64 kb = 0;
-      if (!StringToInt64(args[2], &kb) || kb < 0) {
-        result = RewriteOptions::kOptionValueInvalid;
-        msg = "size_kb must be a positive 64-bit integer";
-      } else {
-        bool ok = driver_factory->caches()->CreateShmMetadataCache(
-            args[1].as_string(), kb, &msg);
-        result = ok ? kOptionOk : kOptionValueInvalid;
-      }
-    } else {
     result = ParseAndSetOptionFromName2(directive, args[1], args[2],
                                         &msg, handler);
+    if (result == RewriteOptions::kOptionNameUnknown) {
+      result = driver_factory->ParseAndSetOption2(
+          directive,
+          args[1],
+          args[2],
+          scope >= RewriteOptions::kLegacyProcessScope,
+          &msg,
+          handler);
     }
   } else if (n_args == 4) {
     result = ParseAndSetOptionFromName3(
         directive, args[1], args[2], args[3], &msg, handler);
   } else {
-    return "unknown option";
+    result = RewriteOptions::kOptionNameUnknown;
   }
 
   switch (result) {
     case RewriteOptions::kOptionOk:
       return NGX_CONF_OK;
     case RewriteOptions::kOptionNameUnknown:
-      return "unknown option";
+      return ps_error_string_for_option(
+          pool, directive, "not recognized or too many arguments");
     case RewriteOptions::kOptionValueInvalid: {
-      GoogleString full_directive = "\"";
+      GoogleString full_directive;
       for (int i = 0 ; i < n_args ; i++) {
         StrAppend(&full_directive, i == 0 ? "" : " ", args[i]);
       }
-      StrAppend(&full_directive, "\": ", msg);
+      return ps_error_string_for_option(pool, full_directive, msg);
-      char* s = ngx_psol::string_piece_to_pool_string(pool, full_directive);
-      if (s == NULL) {
-        return "failed to allocate memory";
-      }
-      return s;
     }
   }
 
@@ -256,8 +458,84 @@ NgxRewriteOptions::ParseAndSetOptions(
   return NULL;
 }
 
+// Execute all entries in the script_lines vector, and hand the result off to
+// ParseAndSetOptions to obtain the final option values.
+bool NgxRewriteOptions::ExecuteScriptVariables(
+    ngx_http_request_t* r, MessageHandler* handler,
+    NgxRewriteDriverFactory* driver_factory) {
+  bool script_error = false;
+
+  if (script_lines_.size() > 0) {
+    std::vector<RefCountedPtr<ScriptLine> >::iterator it;
+    for (it = script_lines_.begin() ; it != script_lines_.end(); ++it) {
+      ScriptLine* script_line = it->get();
+      StringPiece args[NGX_PAGESPEED_MAX_ARGS];
+      std::vector<ScriptArgIndex*>::iterator cs_it;
+      int i;
+
+      for (i = 0; i < script_line->n_args(); i++) {
+        args[i] = script_line->args()[i];
+      }
+
+      for (cs_it = script_line->data().begin();
+           cs_it != script_line->data().end(); cs_it++) {
+        ngx_http_script_compile_t* script;
+        ngx_array_t* values;
+        ngx_array_t* lengths;
+        ngx_str_t value;
+
+        script = (*cs_it)->script();
+        lengths = *script->lengths;
+        values = *script->values;
+
+        if (ngx_http_script_run(r, &value, lengths->elts, 0, values->elts)
+            == NULL) {
+          handler->Message(kError, "ngx_http_script_run error");
+          script_error = true;
+          break;
+        } else  {
+          args[(*cs_it)->index()] = str_to_string_piece(value);
+        }
+      }
+
+      const char* status = ParseAndSetOptions(args, script_line->n_args(),
+          r->pool, handler, driver_factory, script_line->scope(), NULL /*cf*/,
+          ProcessScriptVariablesMode::kOff);
+
+      if (status != NULL) {
+        script_error = true;
+        handler->Message(kWarning,
+            "Error setting option value from script: '%s'", status);
+        break;
+      }
+    }
+  }
+
+  if (script_error) {
+    handler->Message(kWarning,
+        "Script error(s) in configuration, disabling optimization");
+    set_enabled(RewriteOptions::kEnabledOff);
+    return false;
+  }
+
+  return true;
+}
+
+void NgxRewriteOptions::CopyScriptLinesTo(
+    NgxRewriteOptions* destination) const {
+  destination->script_lines_ = script_lines_;
+}
+
+void NgxRewriteOptions::AppendScriptLinesTo(
+    NgxRewriteOptions* destination) const {
+  destination->script_lines_.insert(destination->script_lines_.end(),
+                                    script_lines_.begin(), script_lines_.end());
+}
+
 NgxRewriteOptions* NgxRewriteOptions::Clone() const {
-  NgxRewriteOptions* options = new NgxRewriteOptions(thread_system());
+  NgxRewriteOptions* options = new NgxRewriteOptions(
+      StrCat("cloned from ", description()), thread_system());
+  this->CopyScriptLinesTo(options);
   options->Merge(*this);
   return options;
 }

src/ngx_rewrite_options.h

@@ -27,19 +27,91 @@ extern "C" {
   #include <ngx_http.h>
 }
 
+#include <vector>
+
+#include "ngx_rewrite_driver_factory.h"
+
 #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
 
 namespace net_instaweb {
 
 class NgxRewriteDriverFactory;
 
+class ScriptArgIndex {
+ public:
+  explicit ScriptArgIndex(ngx_http_script_compile_t* script, int index)
+    : script_(script), index_(index) {
+      CHECK(script != NULL);
+      CHECK(index > 0 && index < NGX_PAGESPEED_MAX_ARGS);
+  }
+
+  virtual ~ScriptArgIndex() {}
+
+  ngx_http_script_compile_t* script() { return script_; }
+  int index() { return index_; }
+
+ private:
+  // Not owned.
+  ngx_http_script_compile_t* script_;
+  int index_;
+};
+
+// Refcounted, because the ScriptArgIndexes inside data_ can be shared between
+// different rewriteoptions.
+class ScriptLine : public RefCounted<ScriptLine> {
+ public:
+  explicit ScriptLine(StringPiece* args, int n_args,
+                      RewriteOptions::OptionScope scope)
+    : n_args_(n_args),
+      scope_(scope) {
+
+      for (int i = 0; i < n_args; i++) {
+        args_[i] = args[i];
+      }
+  }
+
+  virtual ~ScriptLine() {
+    STLDeleteElements(&data_);
+    data_.clear();
+  }
+
+  void AddScriptAndArgIndex(ngx_http_script_compile_t* script,
+                            int script_index) {
+    CHECK(script != NULL);
+    CHECK(script_index <  NGX_PAGESPEED_MAX_ARGS);
+    data_.push_back(new ScriptArgIndex(script, script_index));
+  }
+
+  int n_args() { return n_args_;}
+  StringPiece* args() { return args_;}
+  RewriteOptions::OptionScope scope() { return scope_; }
+  std::vector<ScriptArgIndex*>& data() {
+    return data_;
+  }
+
+ private:
+  StringPiece args_[NGX_PAGESPEED_MAX_ARGS];
+  int n_args_;
+  RewriteOptions::OptionScope scope_;
+  std::vector<ScriptArgIndex*> data_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScriptLine);
+};
+
 class NgxRewriteOptions : public SystemRewriteOptions {
  public:
   // See rewrite_options::Initialize and ::Terminate
   static void Initialize();
   static void Terminate();
 
+  NgxRewriteOptions(const StringPiece& description,
+                    ThreadSystem* thread_system);
   explicit NgxRewriteOptions(ThreadSystem* thread_system);
   virtual ~NgxRewriteOptions() { }
 
@@ -54,9 +126,19 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   // on failure.
   //
   // pool is a memory pool for allocating error strings.
+  // cf is only required when compile_scripts is true
+  // when compile_scripts is true, the rewrite_options will be prepared
+  // for replacing any script $variables encountered in args. when false,
+  // script variables will be substituted using the prepared rewrite options.
   const char* ParseAndSetOptions(
       StringPiece* args, int n_args, ngx_pool_t* pool, MessageHandler* handler,
+      NgxRewriteDriverFactory* driver_factory, OptionScope scope,
+      ngx_conf_t* cf, ProcessScriptVariablesMode script_mode);
+  bool ExecuteScriptVariables(
+      ngx_http_request_t* r, MessageHandler* handler,
       NgxRewriteDriverFactory* driver_factory);
+  void CopyScriptLinesTo(NgxRewriteOptions* destination) const;
+  void AppendScriptLinesTo(NgxRewriteOptions* destination) const;
 
   // Make an identical copy of these options and return it.
   virtual NgxRewriteOptions* Clone() const;
@@ -66,6 +148,30 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   static const NgxRewriteOptions* DynamicCast(const RewriteOptions* instance);
   static NgxRewriteOptions* DynamicCast(RewriteOptions* instance);
 
+  const GoogleString& statistics_path() const {
+    return statistics_path_.value();
+  }
+  const GoogleString& global_statistics_path() const {
+    return global_statistics_path_.value();
+  }
+  const GoogleString& console_path() const {
+    return console_path_.value();
+  }
+  const GoogleString& messages_path() const {
+    return messages_path_.value();
+  }
+  const GoogleString& admin_path() const {
+    return admin_path_.value();
+  }
+  const GoogleString& global_admin_path() const {
+    return global_admin_path_.value();
+  }
+  const std::vector<RefCountedPtr<ScriptLine> >& script_lines() const {
+    return script_lines_;
+  }
+  const bool& clear_inherited_scripts() const {
+    return clear_inherited_scripts_;
+  }
 
  private:
   // Helper methods for ParseAndSetOptions().  Each can:
@@ -85,9 +191,8 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   OptionSettingResult ParseAndSetOptions0(
       StringPiece directive, GoogleString* msg, MessageHandler* handler);
 
-  // These are called via RewriteOptions::ParseAndSetOptionFromName[123]
+  virtual OptionSettingResult ParseAndSetOptionFromName1(
-  virtual OptionSettingResult ParseAndSetOptionFromEnum1(
+      StringPiece name, StringPiece arg,
-      OptionEnum name, StringPiece arg,
       GoogleString* msg, MessageHandler* handler);
 
   // We may want to override 2- and 3-argument versions as well in the future,
@@ -102,21 +207,37 @@ class NgxRewriteOptions : public SystemRewriteOptions {
   static Properties* ngx_properties_;
   static void AddProperties();
   void Init();
-  void InitializeSignaturesAndDefaults();
 
   // Add an option to ngx_properties_
-  template<class RewriteOptionsSubclass, class OptionClass>
+  template<class OptionClass>
   static void add_ngx_option(typename OptionClass::ValueType default_value,
-                             OptionClass RewriteOptionsSubclass::*offset,
+                             OptionClass NgxRewriteOptions::*offset,
                              const char* id,
-                             OptionEnum option_enum) {
+                             StringPiece option_name,
-    AddProperty(default_value, offset, id, option_enum, ngx_properties_);
+                             OptionScope scope,
+                             const char* help,
+                             bool safe_to_print) {
+    AddProperty(default_value, offset, id, option_name, scope, help,
+                safe_to_print, ngx_properties_);
   }
 
+  Option<GoogleString> statistics_path_;
+  Option<GoogleString> global_statistics_path_;
+  Option<GoogleString> console_path_;
+  Option<GoogleString> messages_path_;
+  Option<GoogleString> admin_path_;
+  Option<GoogleString> global_admin_path_;
+
+  bool clear_inherited_scripts_;
+  std::vector<RefCountedPtr<ScriptLine> > script_lines_;
+
   // Helper for ParseAndSetOptions.  Returns whether the two directives equal,
   // ignoring case.
   bool IsDirective(StringPiece config_directive, StringPiece compare_directive);
 
+  // Returns a given option's scope.
+  RewriteOptions::OptionScope GetOptionScope(StringPiece option_name);
+
   // TODO(jefftk): support fetch proxy in server and location blocks.
 
   DISALLOW_COPY_AND_ASSIGN(NgxRewriteOptions);

src/ngx_server_context.cc

@@ -18,120 +18,81 @@
 
 #include "ngx_server_context.h"
 
-#include "ngx_request_context.h"
+extern "C" {
+  #include <ngx_http.h>
+}
+
+#include "ngx_pagespeed.h"
+#include "ngx_message_handler.h"
 #include "ngx_rewrite_driver_factory.h"
 #include "ngx_rewrite_options.h"
 #include "net/instaweb/rewriter/public/rewrite_driver.h"
-#include "net/instaweb/rewriter/public/rewrite_stats.h"
+#include "pagespeed/system/add_headers_fetcher.h"
-#include "net/instaweb/system/public/add_headers_fetcher.h"
+#include "pagespeed/system/loopback_route_fetcher.h"
-#include "net/instaweb/system/public/loopback_route_fetcher.h"
+#include "pagespeed/system/system_request_context.h"
-#include "net/instaweb/system/public/system_caches.h"
-#include "net/instaweb/util/public/shared_mem_statistics.h"
-#include "net/instaweb/util/public/split_statistics.h"
-#include "net/instaweb/util/public/statistics.h"
 
 namespace net_instaweb {
 
-const char kCacheFlushCount[] = "cache_flush_count";
+NgxServerContext::NgxServerContext(
-const char kCacheFlushTimestampMs[] = "cache_flush_timestamp_ms";
+    NgxRewriteDriverFactory* factory, StringPiece hostname, int port)
-
+    : SystemServerContext(factory, hostname, port),
-// Statistics histogram names.
+      ngx_http2_variable_index_(NGX_ERROR) {
-const char kHtmlRewriteTimeUsHistogram[] = "Html Time us Histogram";
-
-
-NgxServerContext::NgxServerContext(NgxRewriteDriverFactory* factory)
-    : SystemServerContext(factory),
-      ngx_factory_(factory),
-      initialized_(false) {
 }
 
-NgxServerContext::~NgxServerContext() {
+NgxServerContext::~NgxServerContext() { }
-}
 
 NgxRewriteOptions* NgxServerContext::config() {
   return NgxRewriteOptions::DynamicCast(global_options());
 }
 
-void NgxServerContext::ChildInit() {
+SystemRequestContext* NgxServerContext::NewRequestContext(
-  DCHECK(!initialized_);
+    ngx_http_request_t* r) {
-  if (!initialized_) {
+  // Based on ngx_http_variable_server_port.
-    initialized_ = true;
+  bool port_set = false;
-    set_lock_manager(ngx_factory_->caches()->GetLockManager(config()));
+  int local_port = 0;
-
+#if (NGX_HAVE_INET6)
-    if (split_statistics_.get() != NULL) {
+  if (r->connection->local_sockaddr->sa_family == AF_INET6) {
-      // Readjust the SHM stuff for the new process
+    local_port = ntohs(reinterpret_cast<struct sockaddr_in6*>(
-      local_statistics_->Init(false, message_handler());
+        r->connection->local_sockaddr)->sin6_port);
-
+    port_set = true;
-      // Create local stats for the ServerContext, and fill in its
+  }
-      // statistics() and rewrite_stats() using them; if we didn't do this here
+#endif
-      // they would get set to the factory's by the InitServerContext call
+  if (!port_set) {
-      // below.
+    local_port = ntohs(reinterpret_cast<struct sockaddr_in*>(
-      set_statistics(split_statistics_.get());
+        r->connection->local_sockaddr)->sin_port);
-      local_rewrite_stats_.reset(new RewriteStats(
-          split_statistics_.get(), ngx_factory_->thread_system(),
-          ngx_factory_->timer()));
-      set_rewrite_stats(local_rewrite_stats_.get());
   }
 
-    ngx_factory_->InitServerContext(this);
+  ngx_str_t local_ip;
-    // TODO(oschaaf): in mod_pagespeed, the ServerContext owns
+  u_char addr[NGX_SOCKADDR_STRLEN];
-    // the fetchers, and sets up the UrlAsyncFetcherStats here
+  local_ip.len = NGX_SOCKADDR_STRLEN;
+  local_ip.data = addr;
+  ngx_int_t rc = ngx_connection_local_sockaddr(r->connection, &local_ip, 0);
+  if (rc != NGX_OK) {
+    local_ip.len = 0;
+  }
+
+  SystemRequestContext* ctx = new SystemRequestContext(
+      thread_system()->NewMutex(), timer(),
+      ps_determine_host(r), local_port, str_to_string_piece(local_ip));
+
+  // See if http2 is in use.
+  if (ngx_http2_variable_index_ >= 0) {
+    ngx_http_variable_value_t* val =
+        ngx_http_get_indexed_variable(r, ngx_http2_variable_index_);
+    if (val != NULL && val->valid) {
+      StringPiece str_val(reinterpret_cast<char*>(val->data), val->len);
+      if (str_val == "h2" || str_val == "h2c") {
+        ctx->set_using_http2(true);
+      }
     }
   }
 
-void NgxServerContext::CreateLocalStatistics(
+  return ctx;
-    Statistics* global_statistics) {
-  local_statistics_ =
-      ngx_factory_->AllocateAndInitSharedMemStatistics(
-          hostname_identifier(), *config());
-  split_statistics_.reset(new SplitStatistics(
-      ngx_factory_->thread_system(), local_statistics_, global_statistics));
-  // local_statistics_ was ::InitStat'd by AllocateAndInitSharedMemStatistics,
-  // but we need to take care of split_statistics_.
-  NgxRewriteDriverFactory::InitStats(split_statistics_.get());
 }
 
-void NgxServerContext::InitStats(Statistics* statistics) {
+GoogleString NgxServerContext::FormatOption(StringPiece option_name,
-  // TODO(oschaaf): we need to port the cache flush mechanism
+                                            StringPiece args) {
-  statistics->AddVariable(kCacheFlushCount);
+  return StrCat("pagespeed ", option_name, " ", args, ";");
-  statistics->AddVariable(kCacheFlushTimestampMs);
-  Histogram* html_rewrite_time_us_histogram =
-      statistics->AddHistogram(kHtmlRewriteTimeUsHistogram);
-  // We set the boundary at 2 seconds which is about 2 orders of magnitude
-  // worse than anything we have reasonably seen, to make sure we don't
-  // cut off actual samples.
-  html_rewrite_time_us_histogram->SetMaxValue(2 * Timer::kSecondUs);
-  // TODO(oschaaf): Once the ServerContext owns the fetchers,
-  // initialise UrlAsyncFetcherStats here
-}
-
-void NgxServerContext::ApplySessionFetchers(
-    const RequestContextPtr& request, RewriteDriver* driver) {
-  const NgxRewriteOptions* conf = NgxRewriteOptions::DynamicCast(
-      driver->options());
-  CHECK(conf != NULL);
-  NgxRequestContext* ngx_request = NgxRequestContext::DynamicCast(
-      request.get());
-  if (ngx_request == NULL) {
-    return;  // decoding_driver has a null RequestContext.
-  }
-
-  // Note that these fetchers are applied in the opposite order of how they are
-  // added
-  // TODO(oschaaf): in mod_pagespeed, LoopbackRouteFetcher is not added when
-  // one of these is set:  disable_loopback_routing, slurping_enabled, or
-  // test_proxy.
-
-  // Note the port here is our port, not from the request, since
-  // LoopbackRouteFetcher may decide we should be talking to ourselves.
-  driver->SetSessionFetcher(new LoopbackRouteFetcher(
-      driver->options(), ngx_request->local_ip(),
-      ngx_request->local_port(), driver->async_fetcher()));
-
-  if (driver->options()->num_custom_fetch_headers() > 0) {
-    driver->SetSessionFetcher(new AddHeadersFetcher(driver->options(),
-                                                    driver->async_fetcher()));
-  }
 }
 
 }  // namespace net_instaweb

src/ngx_server_context.h

@@ -21,56 +21,55 @@
 #ifndef NGX_SERVER_CONTEXT_H_
 #define NGX_SERVER_CONTEXT_H_
 
-#include "net/instaweb/system/public/system_server_context.h"
+#include "ngx_message_handler.h"
+#include "pagespeed/system/system_server_context.h"
+
+extern "C" {
+#include <ngx_http.h>
+}
 
 namespace net_instaweb {
 
 class NgxRewriteDriverFactory;
 class NgxRewriteOptions;
-class RewriteStats;
+class SystemRequestContext;
-class SharedMemStatistics;
-class Statistics;
 
 class NgxServerContext : public SystemServerContext {
  public:
-  explicit NgxServerContext(NgxRewriteDriverFactory* factory);
+  NgxServerContext(
+      NgxRewriteDriverFactory* factory, StringPiece hostname, int port);
   virtual ~NgxServerContext();
 
-  // We expect to use ProxyFetch with HTML.
+  // We don't allow ProxyFetch to fetch HTML via MapProxyDomain. We will call
-  virtual bool ProxiesHtml() const { return true; }
+  // 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
   // downcast.
   NgxRewriteOptions* config();
-  // Should be called after the child process is forked.
+
-  void ChildInit();
-  // Initialize this ServerContext to have its own statistics domain.
-  // Must be called after global_statistics has been created and had
-  // ::Initialize called on it.
-  void CreateLocalStatistics(Statistics* global_statistics);
-  static void InitStats(Statistics* statistics);
-  virtual void ApplySessionFetchers(const RequestContextPtr& req,
-                                    RewriteDriver* driver);
-  bool initialized() const { return initialized_; }
-  GoogleString hostname_identifier() { return hostname_identifier_; }
-  void set_hostname_identifier(GoogleString x) { hostname_identifier_ = x; }
   NgxRewriteDriverFactory* ngx_rewrite_driver_factory() { return ngx_factory_; }
+  SystemRequestContext* NewRequestContext(ngx_http_request_t* r);
+
+  NgxMessageHandler* ngx_message_handler() {
+    return dynamic_cast<NgxMessageHandler*>(message_handler());
+  }
+
+  virtual GoogleString FormatOption(StringPiece option_name, StringPiece args);
+
+  void set_ngx_http2_variable_index(ngx_int_t idx) {
+    ngx_http2_variable_index_ = idx;
+  }
+
+  ngx_int_t ngx_http2_variable_index() const {
+    return ngx_http2_variable_index_;
+  }
 
  private:
   NgxRewriteDriverFactory* ngx_factory_;
-  // hostname_identifier_ is used to distinguish the name of shared memory
+  // what index the "http2" var is, or NGX_ERROR.
-  // segments associated with this ServerContext
+  ngx_int_t ngx_http2_variable_index_;
-  GoogleString hostname_identifier_;
-  bool initialized_;
-
-  // Non-NULL if we have per-vhost stats.
-  scoped_ptr<Statistics> split_statistics_;
-
-  // May be NULL. Owned by *split_statistics_.
-  SharedMemStatistics* local_statistics_;
-  // These are non-NULL if we have per-vhost stats.
-  scoped_ptr<RewriteStats> local_rewrite_stats_;
 
   DISALLOW_COPY_AND_ASSIGN(NgxServerContext);
 };

src/ngx_thread_system.cc

@@ -1,46 +0,0 @@
-/*
- * Copyright 2013 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: jefftk@google.com (Jeff Kaufman)
-
-#include "ngx_thread_system.h"
-
-#include "apr_thread_proc.h"
-
-namespace net_instaweb {
-
-class Timer;
-
-NgxThreadSystem::NgxThreadSystem() : may_start_threads_(false) {}
-
-NgxThreadSystem::~NgxThreadSystem() {}
-
-void NgxThreadSystem::PermitThreadStarting() {
-  CHECK(!may_start_threads_);
-  may_start_threads_ = true;
-}
-
-void NgxThreadSystem::BeforeThreadRunHook() {
-  // We disable all signals here, since the nginx worker process is expecting to
-  // catch them and pagespeed doesn't use signals.
-  apr_setup_signal_thread();
-
-  // If this fails you can get a backtrace from gdb by setting a breakpoint on
-  // "pthread_create".
-  CHECK(may_start_threads_);
-}
-
-}  // namespace net_instaweb

src/ngx_thread_system.h

@@ -1,56 +0,0 @@
-/*
- * Copyright 2013 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: jefftk@google.com (Jeff Kaufman)
-//
-// See ApacheThreadSystem.
-//
-//
-
-#ifndef NGX_THREAD_SYSTEM_H_
-#define NGX_THREAD_SYSTEM_H_
-
-#include "base/logging.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/pthread_thread_system.h"
-
-namespace net_instaweb {
-
-class Timer;
-
-class NgxThreadSystem : public PthreadThreadSystem {
- public:
-  NgxThreadSystem();
-  virtual ~NgxThreadSystem();
-
-  // In nginx we may only start threads after forking a worker process.  In
-  // order to enforce this, we call PermitThreadStarting() in the worker process
-  // right after forking, and CHECK-fail if something tries to start a thread
-  // before then.
-  void PermitThreadStarting();
-
- protected:
-  virtual void BeforeThreadRunHook();
-
- private:
-  bool may_start_threads_;
-
-  DISALLOW_COPY_AND_ASSIGN(NgxThreadSystem);
-};
-
-}  // namespace net_instaweb
-
-#endif  // NGX_THREAD_SYSTEM_H_

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 "pagespeed/kernel/base/basictypes.h"
-#include "net/instaweb/util/public/message_handler.h"
+#include "pagespeed/kernel/base/condvar.h"
-#include "net/instaweb/util/public/pool.h"
+#include "pagespeed/kernel/base/message_handler.h"
-#include "net/instaweb/util/public/pool_element.h"
+#include "pagespeed/kernel/base/pool.h"
-#include "net/instaweb/util/public/statistics.h"
+#include "pagespeed/kernel/base/pool_element.h"
-#include "net/instaweb/util/public/string_util.h"
+#include "pagespeed/kernel/base/statistics.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/string_util.h"
-#include "net/instaweb/util/public/timer.h"
+#include "pagespeed/kernel/base/thread_system.h"
-#include "net/instaweb/util/public/writer.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 {
 
@@ -55,6 +55,7 @@ namespace net_instaweb {
                                          ngx_msec_t resolver_timeout,
                                          ngx_msec_t fetch_timeout,
                                          ngx_resolver_t* resolver,
+                                         int max_keepalive_requests,
                                          ThreadSystem* thread_system,
                                          MessageHandler* handler)
     : fetchers_count_(0),
@@ -63,23 +64,30 @@ namespace net_instaweb {
       byte_count_(0),
       thread_system_(thread_system),
       message_handler_(handler),
-      mutex_(NULL) {
+      mutex_(NULL),
+      max_keepalive_requests_(max_keepalive_requests),
+      event_connection_(NULL) {
     resolver_timeout_ = resolver_timeout;
     fetch_timeout_ = fetch_timeout;
-    ngx_memzero(&url_, sizeof(url_));
+    ngx_memzero(&proxy_, sizeof(proxy_));
     if (proxy != NULL && *proxy != '\0') {
-      url_.url.data = reinterpret_cast<u_char*>(const_cast<char*>(proxy));
+      proxy_.url.data = reinterpret_cast<u_char*>(const_cast<char*>(proxy));
-      url_.url.len = ngx_strlen(proxy);
+      proxy_.url.len = ngx_strlen(proxy);
     }
     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",
@@ -87,25 +95,48 @@ 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;
     }
   }
 
+
+  bool NgxUrlAsyncFetcher::ParseUrl(ngx_url_t* url, ngx_pool_t* pool) {
+    size_t scheme_offset;
+    u_short port;
+    if (ngx_strncasecmp(url->url.data, reinterpret_cast<u_char*>(
+                                     const_cast<char*>("http://")), 7) == 0) {
+      scheme_offset = 7;
+      port = 80;
+    } else if (ngx_strncasecmp(url->url.data, reinterpret_cast<u_char*>(
+                                    const_cast<char*>("https://")), 8) == 0) {
+      scheme_offset = 8;
+      port = 443;
+    } else {
+      scheme_offset = 0;
+      port = 80;
+    }
+
+    url->url.data += scheme_offset;
+    url->url.len -= scheme_offset;
+    url->default_port = port;
+    // See: http://lxr.evanmiller.org/http/source/core/ngx_inet.c#L875
+    url->no_resolve = 0;
+    url->uri_part = 1;
+
+    if (ngx_parse_url(pool, url) == NGX_OK) {
+      return true;
+    }
+    return false;
+  }
+
   // If there are still active requests, cancel them.
   void NgxUrlAsyncFetcher::CancelActiveFetches() {
     // TODO(oschaaf): this seems tricky, this may end up calling
@@ -120,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() {
+  bool NgxUrlAsyncFetcher::Init(ngx_cycle_t* cycle) {
-    log_ = ngx_cycle->log;
+    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) {
@@ -132,50 +167,15 @@ namespace net_instaweb {
       }
     }
 
-    int pipe_fds[2];
+    if (proxy_.url.len == 0) {
-    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 (url_.url.len == 0) {
       return true;
     }
 
     // TODO(oschaaf): shouldn't we do this earlier? Do we need to clean
     // up when parsing the url failed?
-    if (ngx_parse_url(pool_, &url_) != NGX_OK) {
+    if (!ParseUrl(&proxy_, pool_)) {
       ngx_log_error(NGX_LOG_ERR, log_, 0,
-          "NgxUrlAsyncFetcher::Init parse proxy[%V] failed", &url_.url);
+          "NgxUrlAsyncFetcher::Init parse proxy[%V] failed", &proxy_.url);
       return false;
     }
     return true;
@@ -183,7 +183,28 @@ namespace net_instaweb {
 
   void NgxUrlAsyncFetcher::ShutDown() {
     shutdown_ = true;
-      SendCmd('S');
+    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
@@ -191,58 +212,35 @@ namespace net_instaweb {
   void NgxUrlAsyncFetcher::Fetch(const GoogleString& url,
                                  MessageHandler* message_handler,
                                  AsyncFetch* async_fetch) {
-    async_fetch = EnableInflation(async_fetch, NULL);
+    // 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, fetch_timeout_, log_);
+          message_handler, log_);
     ScopedMutex lock(mutex_);
     pending_fetches_.Add(fetch);
-    SendCmd('F');
-  }
 
-  // send command to nginx main thread
+    // TODO(oschaaf): thread safety on written vs shutdown.
-  // 'F' : start a fetch
+    // It is possible that shutdown() is called after writing an event? In that
-  // 'S' : shutdown the fetcher
+    // case, this could (rarely) fail when it shouldn't.
-  bool NgxUrlAsyncFetcher::SendCmd(const char command) {
+    bool written = event_connection_->WriteEvent(this);
-    int rc;
+    CHECK(written || shutdown_) << "NgxUrlAsyncFetcher: event write failure";
-    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;
   }
 
   // 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) {
+  void NgxUrlAsyncFetcher::ReadCallback(const ps_event_data& data) {
-    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);
-
-    CHECK(rc == -1 || rc == 0 || rc == 1);
-
-    if (rc == -1 || rc == 0) {
-      // EAGAIN
-      return;
-    }
-
     std::vector<NgxFetch*> to_start;
+    NgxUrlAsyncFetcher* fetcher = reinterpret_cast<NgxUrlAsyncFetcher*>(
+      data.sender);
 
-    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;
@@ -255,29 +253,6 @@ namespace net_instaweb {
     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;
-    }
 
     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 "pagespeed/kernel/base/basictypes.h"
-#include "net/instaweb/util/public/pool.h"
+#include "pagespeed/kernel/base/pool.h"
-#include "net/instaweb/util/public/string.h"
+#include "pagespeed/kernel/base/string.h"
-#include "net/instaweb/util/public/thread_system.h"
+#include "pagespeed/kernel/base/thread_system.h"
 
 
 namespace net_instaweb {
@@ -53,27 +56,27 @@ class NgxUrlAsyncFetcher : public UrlAsyncFetcher {
   NgxUrlAsyncFetcher(
       const char* proxy, ngx_log_t* log, ngx_msec_t resolver_timeout,
       ngx_msec_t fetch_timeout, ngx_resolver_t* resolver,
-      ThreadSystem* thread_system, MessageHandler* handler);
+      int max_keepalive_requests, ThreadSystem* thread_system,
+      MessageHandler* handler);
 
   ~NgxUrlAsyncFetcher();
 
   // It should be called in the module init_process callback function. Do some
   // intializations which can't be done in the master process
-  bool Init();
+  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
@@ -115,13 +118,14 @@ class NgxUrlAsyncFetcher : public UrlAsyncFetcher {
 
  private:
   static void TimeoutHandler(ngx_event_t* tev);
+  static bool ParseUrl(ngx_url_t* url, ngx_pool_t* pool);
   friend class NgxFetch;
 
   NgxFetchPool active_fetches_;
   // Add the pending task to this list
   NgxFetchPool pending_fetches_;
   NgxFetchPool completed_fetches_;
-  ngx_url_t url_;
+  ngx_url_t proxy_;
 
   int fetchers_count_;
   bool shutdown_;
@@ -135,12 +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);
 };
 

src/pthread_shared_mem.cc

@@ -1,257 +0,0 @@
-// Copyright 2011 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: morlovich@google.com (Maksim Orlovich)
-
-#include "pthread_shared_mem.h"
-
-#include <fcntl.h>
-#include <pthread.h>
-#include <sys/mman.h>
-#include <unistd.h>
-#include <cerrno>
-#include <cstddef>
-#include <map>
-#include <utility>
-#include "net/instaweb/util/public/abstract_shared_mem.h"
-#include "net/instaweb/util/public/abstract_mutex.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/util/public/string.h"
-
-namespace net_instaweb {
-
-namespace ngx {
-
-namespace {
-
-// This implementation relies on readonly copies of old memory and shared R/W
-// mappings being kept across a fork. It simply stashes addresses of
-// shared mmap segments into a map where kid processes can pick them up.
-
-// close() a fd logging failure and dealing with EINTR.
-void CheckedClose(int fd, MessageHandler* message_handler) {
-  while (close(fd) != 0) {
-    if (errno != EINTR) {
-      message_handler->Message(kWarning, "Problem closing SHM segment fd:%d",
-                               errno);
-      return;
-    }
-  }
-}
-
-// Unlike PthreadMutex this doesn't own the lock, but rather refers to an
-// external one.
-class PthreadSharedMemMutex : public AbstractMutex {
- public:
-  explicit PthreadSharedMemMutex(pthread_mutex_t* external_mutex)
-      : external_mutex_(external_mutex) {}
-
-  virtual bool TryLock() {
-    return (pthread_mutex_trylock(external_mutex_) == 0);
-  }
-
-  virtual void Lock() {
-    pthread_mutex_lock(external_mutex_);
-  }
-
-  virtual void Unlock() {
-    pthread_mutex_unlock(external_mutex_);
-  }
-
- private:
-  pthread_mutex_t* external_mutex_;
-
-  DISALLOW_COPY_AND_ASSIGN(PthreadSharedMemMutex);
-};
-
-class PthreadSharedMemSegment : public AbstractSharedMemSegment {
- public:
-  // We will be representing memory mapped in the [base, base + size) range.
-  PthreadSharedMemSegment(char* base, size_t size, MessageHandler* handler)
-      : base_(base),
-        size_(size) {
-  }
-
-  virtual ~PthreadSharedMemSegment() {
-  }
-
-  virtual volatile char* Base() {
-    return base_;
-  }
-
-  virtual size_t SharedMutexSize() const {
-    return sizeof(pthread_mutex_t);
-  }
-
-  virtual bool InitializeSharedMutex(size_t offset, MessageHandler* handler) {
-    pthread_mutexattr_t attr;
-    if (pthread_mutexattr_init(&attr) != 0) {
-      handler->Message(kError, "pthread_mutexattr_init failed with errno:%d",
-                       errno);
-      return false;
-    }
-
-    if (pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED) != 0) {
-      pthread_mutexattr_destroy(&attr);
-      handler->Message(
-          kError, "pthread_mutexattr_setpshared failed with errno:%d", errno);
-      return false;
-    }
-
-    if (pthread_mutex_init(MutexPtr(offset), &attr) != 0) {
-      pthread_mutexattr_destroy(&attr);
-      handler->Message(kError, "pthread_mutex_init failed with errno:%d",
-                       errno);
-      return false;
-    }
-
-    pthread_mutexattr_destroy(&attr);
-    return true;
-  }
-
-  virtual AbstractMutex* AttachToSharedMutex(size_t offset) {
-    return new PthreadSharedMemMutex(MutexPtr(offset));
-  }
-
- private:
-  pthread_mutex_t* MutexPtr(size_t offset) {
-    return reinterpret_cast<pthread_mutex_t*>(base_ + offset);
-  }
-
-  char* const base_;
-  const size_t size_;
-
-  DISALLOW_COPY_AND_ASSIGN(PthreadSharedMemSegment);
-};
-
-pthread_mutex_t segment_bases_lock = PTHREAD_MUTEX_INITIALIZER;
-
-}  // namespace
-
-size_t PthreadSharedMem::s_instance_count_ = 0;
-
-PthreadSharedMem::SegmentBaseMap* PthreadSharedMem::segment_bases_ = NULL;
-
-PthreadSharedMem::PthreadSharedMem() {
-  instance_number_ = ++s_instance_count_;
-}
-
-PthreadSharedMem::~PthreadSharedMem() {
-}
-
-size_t PthreadSharedMem::SharedMutexSize() const {
-  return sizeof(pthread_mutex_t);
-}
-
-AbstractSharedMemSegment* PthreadSharedMem::CreateSegment(
-    const GoogleString& name, size_t size, MessageHandler* handler) {
-  GoogleString prefixed_name = PrefixSegmentName(name);
-  // Create the memory
-  int fd = open("/dev/zero", O_RDWR);
-  if (fd == -1) {
-    handler->Message(kError, "Unable to create SHM segment %s, errno=%d.",
-                     prefixed_name.c_str(), errno);
-    return NULL;
-  }
-
-  // map it
-  char* base = reinterpret_cast<char*>(
-                   mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0));
-  CheckedClose(fd, handler);
-  if (base == MAP_FAILED) {
-    return NULL;
-  }
-
-  SegmentBaseMap* bases = AcquireSegmentBases();
-  (*bases)[prefixed_name] = base;
-  UnlockSegmentBases();
-  return new PthreadSharedMemSegment(base, size, handler);
-}
-
-AbstractSharedMemSegment* PthreadSharedMem::AttachToSegment(
-    const GoogleString& name, size_t size, MessageHandler* handler) {
-  GoogleString prefixed_name = PrefixSegmentName(name);
-  SegmentBaseMap* bases = AcquireSegmentBases();
-  SegmentBaseMap::const_iterator i = bases->find(prefixed_name);
-  if (i == bases->end()) {
-    handler->Message(kError, "Unable to find SHM segment %s to attach to.",
-                     prefixed_name.c_str());
-    UnlockSegmentBases();
-    return NULL;
-  }
-  char* base = i->second;
-  UnlockSegmentBases();
-  return new PthreadSharedMemSegment(base, size, handler);
-}
-
-void PthreadSharedMem::DestroySegment(const GoogleString& name,
-                                      MessageHandler* handler) {
-  GoogleString prefixed_name = PrefixSegmentName(name);
-
-  // Note that in the process state children will not see any mutations
-  // we make here, so it acts mostly for checking in that case.
-  SegmentBaseMap* bases = AcquireSegmentBases();
-  SegmentBaseMap::iterator i = bases->find(prefixed_name);
-  if (i != bases->end()) {
-    bases->erase(i);
-    if (bases->empty()) {
-      delete segment_bases_;
-      segment_bases_ = NULL;
-    }
-  } else {
-    handler->Message(kError, "Attempt to destroy unknown SHM segment %s.",
-                     prefixed_name.c_str());
-  }
-  UnlockSegmentBases();
-}
-
-PthreadSharedMem::SegmentBaseMap* PthreadSharedMem::AcquireSegmentBases() {
-  PthreadSharedMemMutex lock(&segment_bases_lock);
-  lock.Lock();
-
-  if (segment_bases_ == NULL) {
-    segment_bases_ = new SegmentBaseMap();
-  }
-
-  return segment_bases_;
-}
-
-void PthreadSharedMem::UnlockSegmentBases() {
-  PthreadSharedMemMutex lock(&segment_bases_lock);
-  lock.Unlock();
-}
-
-GoogleString PthreadSharedMem::PrefixSegmentName(const GoogleString& name) {
-  GoogleString res;
-  StrAppend(&res, "[", IntegerToString(instance_number_), "]", name);
-  return res;
-}
-
-
-void PthreadSharedMem::Terminate() {
-  // Clean up the local memory associated with the maps to shared memory
-  // storage.
-  PthreadSharedMemMutex lock(&segment_bases_lock);
-  lock.Lock();
-  if (segment_bases_ != NULL) {
-    delete segment_bases_;
-    segment_bases_ = NULL;
-  }
-  lock.Unlock();
-}
-
-}  // namespace ngx
-
-}  // namespace net_instaweb

src/pthread_shared_mem.h

@@ -1,88 +0,0 @@
-// Copyright 2011 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: morlovich@google.com (Maksim Orlovich)
-
-#ifndef NET_INSTAWEB_UTIL_PUBLIC_PTHREAD_SHARED_MEM_H_
-#define NET_INSTAWEB_UTIL_PUBLIC_PTHREAD_SHARED_MEM_H_
-
-#include <cstddef>
-#include <map>
-
-#include "net/instaweb/util/public/abstract_shared_mem.h"
-#include "net/instaweb/util/public/basictypes.h"
-#include "net/instaweb/util/public/string.h"
-
-namespace net_instaweb {
-class MessageHandler;
-
-namespace ngx {
-
-// POSIX shared memory support, using mmap/pthread_mutexattr_setpshared
-// Supports both processes and threads, but processes that want to access it
-// must be results of just fork (without exec), and all the CreateSegment
-// calls must occur before the fork.
-//
-// This implementation is also not capable of deallocating segments except
-// at exit, so it should not be used when the set of segments may be dynamic.
-class PthreadSharedMem : public AbstractSharedMem {
- public:
-  PthreadSharedMem();
-  virtual ~PthreadSharedMem();
-
-  virtual size_t SharedMutexSize() const;
-
-  virtual AbstractSharedMemSegment* CreateSegment(
-      const GoogleString& name, size_t size, MessageHandler* handler);
-
-  virtual AbstractSharedMemSegment* AttachToSegment(
-      const GoogleString& name, size_t size, MessageHandler* handler);
-
-  virtual void DestroySegment(const GoogleString& name,
-                              MessageHandler* handler);
-
-  // Frees all lazy-initialized memory used to track shared-memory segments.
-  static void Terminate();
-
- private:
-  typedef std::map<GoogleString, char*> SegmentBaseMap;
-
-  // Accessor for below. Note that the segment_bases_lock will be held at exit.
-  static SegmentBaseMap* AcquireSegmentBases();
-
-  static void UnlockSegmentBases();
-
-  // Prefixes the passed in segment name with the current instance number.
-  GoogleString PrefixSegmentName(const GoogleString& name);
-
-  // The root process stores segment locations here. Child processes will
-  // inherit a readonly copy of this map after the fork. Note that this is
-  // initialized in a thread-unsafe manner, given the above assumptions.
-  static SegmentBaseMap* segment_bases_;
-  // Holds the number of times a PthreadSharedMem has been created.
-  static size_t s_instance_count_;
-  // Used to prefix segment names, so that when two runtimes are active at the
-  // same moment they will not have overlapping segment names. This occurs in
-  // ngx_pagespeed during a configuration reload, where first a new factory is
-  // created, before destroying the old one.
-  size_t instance_number_;
-
-  DISALLOW_COPY_AND_ASSIGN(PthreadSharedMem);
-};
-
-}  // namespace ngx
-
-}  // namespace net_instaweb
-
-#endif  // NET_INSTAWEB_UTIL_PUBLIC_PTHREAD_SHARED_MEM_H_

test/run_tests.sh

@@ -23,10 +23,12 @@
 # Exits with status 2 if command line args are wrong.
 #
 # Usage:
-#   ./run_tests.sh primary_port secondary_port mod_pagespeed_dir
+#   ./run_tests.sh /path/to/mod_pagespeed /path/to/nginx/binary
-# Example:
-#   ./run_tests.sh 8050 8051 /path/to/mod_pagespeed
 #
+# By default the test script uses several ports.  If you have a port conflict
+# and need to override one you can do that by setting the relevant environment
+# variable.  For example:
+#   PRIMARY_PORT=1234 ./run_tests.sh /.../mod_pagespeed /.../nginx/binary
 
 # Normally we test only with the native fetcher off.  Set
 # TEST_NATIVE_FETCHER=true to also test the native fetcher, set
@@ -43,26 +45,34 @@ RUN_TESTS=${RUN_TESTS:-true}
 # true.
 USE_VALGRIND=${USE_VALGRIND:-false}
 
-if [ "$#" -ne 4 ] ; then
+if [ "$#" -ne 2 ] ; then
-  echo "Usage: $0 primary_port secondary_port mod_pagespeed_dir"
+  echo "Usage: $0 mod_pagespeed_dir nginx_executable"
-  echo "  nginx_executable"
   exit 2
 fi
 
-PRIMARY_PORT="$1"
+MOD_PAGESPEED_DIR="$1"
-SECONDARY_PORT="$2"
+NGINX_EXECUTABLE="$2"
-MOD_PAGESPEED_DIR="$3"
+
-NGINX_EXECUTABLE="$4"
+: ${PRIMARY_PORT:=8050}
+: ${SECONDARY_PORT:=8051}
+: ${CONTROLLER_PORT:=8053}
+: ${RCPORT:=9991}
+: ${PAGESPEED_TEST_HOST:=selfsigned.modpagespeed.com}
+: ${PHP_PORT:=9000}
 
 this_dir="$( cd $(dirname "$0") && pwd)"
 
 function run_test_checking_failure() {
+  "$MOD_PAGESPEED_DIR/install/start_php.sh" "$PHP_PORT"
   USE_VALGRIND="$USE_VALGRIND" \
     PRIMARY_PORT="$PRIMARY_PORT" \
     SECONDARY_PORT="$SECONDARY_PORT" \
     MOD_PAGESPEED_DIR="$MOD_PAGESPEED_DIR" \
     NGINX_EXECUTABLE="$NGINX_EXECUTABLE" \
+    PAGESPEED_TEST_HOST="$PAGESPEED_TEST_HOST" \
     RUN_TESTS="$RUN_TESTS" \
+    CONTROLLER_PORT="$CONTROLLER_PORT" \
+    RCPORT="$RCPORT" \
     bash "$this_dir/nginx_system_test.sh"
   STATUS=$?
   echo "With $@ setup."
@@ -73,6 +83,9 @@ function run_test_checking_failure() {
     3)
       return  # Only expected failures.
       ;;
+    4)
+      return  # Return passing error code when running manually.
+      ;;
     *)
       exit 1  # Real failure.
   esac

test/valgrind.sup

@@ -0,0 +1,193 @@
+# Copyright 2013 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)
+
+
+# The first few suppressions can be found in other modules
+# and easily found when searched for, and seem false positives.
+{
+   <nginx false positive>
+   Memcheck:Param
+   socketcall.sendmsg(msg.msg_iov[i])
+   fun:__sendmsg_nocancel
+   fun:ngx_write_channel
+   fun:ngx_signal_worker_processes
+   fun:ngx_master_process_cycle
+   fun:main
+}
+{
+   <nginx false positive>
+   Memcheck:Param
+   socketcall.sendmsg(msg.msg_iov[i])
+   fun:__sendmsg_nocancel
+   fun:ngx_write_channel
+   fun:ngx_master_process_cycle
+   fun:main
+}
+{
+   <nginx false positive>
+   Memcheck:Param
+   socketcall.sendmsg(msg.msg_iov[i])
+   fun:__sendmsg_nocancel
+   fun:ngx_write_channel
+   fun:ngx_pass_open_channel
+   fun:ngx_start_cache_manager_processes
+   fun:ngx_master_process_cycle
+   fun:main
+}
+{
+   <nginx false positive>
+   Memcheck:Param
+   socketcall.sendmsg(msg.msg_iov[i])
+   fun:__sendmsg_nocancel
+   fun:ngx_write_channel
+   fun:ngx_pass_open_channel
+   fun:ngx_start_cache_manager_processes
+   fun:ngx_master_process_cycle
+   fun:main
+}
+{
+   <nginx false positive>
+   Memcheck:Leak
+   fun:malloc
+   fun:ngx_alloc
+   fun:ngx_event_process_init
+   fun:ngx_worker_process_init
+   fun:ngx_worker_process_cycle
+   fun:ngx_spawn_process
+   fun:ngx_start_worker_processes
+   fun:ngx_master_process_cycle
+   fun:main
+}
+{
+   <nginx false positive>
+   Memcheck:Param
+   socketcall.sendmsg(msg.msg_iov[i])
+   fun:__sendmsg_nocancel
+   fun:ngx_write_channel
+   fun:ngx_pass_open_channel
+   fun:ngx_start_worker_processes
+   fun:ngx_master_process_cycle
+   fun:main
+}
+
+# similar to http://trac.nginx.org/nginx/ticket/369
+{
+   <nginx false positive>
+   Memcheck:Param
+   pwrite64(buf)
+   obj:/lib/x86_64-linux-gnu/libpthread-2.15.so
+   fun:ngx_write_file
+   fun:ngx_write_chain_to_file
+   fun:ngx_write_chain_to_temp_file
+   fun:ngx_event_pipe_write_chain_to_temp_file
+   fun:ngx_event_pipe
+   fun:ngx_http_upstream_process_upstream
+   fun:ngx_http_upstream_process_header
+   fun:ngx_http_upstream_handler
+   fun:ngx_epoll_process_events
+   fun:ngx_process_events_and_timers
+   fun:ngx_worker_process_cycle
+}
+# Mentioned in https://github.com/pagespeed/ngx_pagespeed/issues/103
+# Assuming a false postives as the issue is closed.
+{
+   <nginx false positive>
+   Memcheck:Param
+   write(buf)
+   obj:/lib/x86_64-linux-gnu/libpthread-2.15.so
+   fun:ngx_log_error_core
+   fun:ngx_http_parse_complex_uri
+   fun:ngx_http_process_request_uri
+   fun:ngx_http_process_request_line
+   fun:ngx_http_wait_request_handler
+   fun:ngx_epoll_process_events
+   fun:ngx_process_events_and_timers
+   fun:ngx_worker_process_cycle
+   fun:ngx_spawn_process
+   fun:ngx_start_worker_processes
+   fun:ngx_master_process_cycle
+}
+
+# Extra suppresions for testing in release mode:
+
+{
+   <re2 uninitialised value in optimized code>
+   Memcheck:Cond
+   fun:_ZN3re24Prog8OptimizeEv
+   ...
+}
+{
+   <re2 uninitialised value in optimized code>
+   Memcheck:Value8
+   fun:_ZN3re24Prog8OptimizeEv
+   ...
+}
+
+{
+   <re2 uninitialised value in optimized code>
+   Memcheck:Cond
+   fun:_ZN3re2L4AddQEPNS_9SparseSetEi
+   ...
+}
+
+{
+   <re2 uninitialised value in optimized code>
+   Memcheck:Value8
+   fun:_ZN3re2L4AddQEPNS_9SparseSetEi
+   ...
+}
+
+{
+   <re2 uninitialized value in optimized code>
+   Memcheck:Value8
+   fun:_ZN3re23DFA10AddToQueueEPNS0_5WorkqEij
+   ...
+}
+
+{
+   <re2 uninitialized value in optimized code>
+   Memcheck:Cond
+   fun:_ZN3re23DFA10AddToQueueEPNS0_5WorkqEij
+   ...
+}
+
+{
+   <re2 uninitialized value in optimized code>
+   Memcheck:Value8
+   fun:_ZNK3re210SparseSetTIvE8containsEi
+   ...
+}
+
+{
+   <re2 uninitialized value in optimized code>
+   Memcheck:Cond
+   fun:_ZNK3re210SparseSetTIvE8containsEi
+   ...
+}
+
+{
+   <re2 uninitialized value in optimized code>
+   Memcheck:Value8
+   fun:_ZNK3re211SparseArrayIiE9has_indexEi
+   ...
+}
+
+{
+   <re2 uninitialized value in optimized code>
+   Memcheck:Cond
+   fun:_ZNK3re211SparseArrayIiE9has_indexEi
+   ...
+}