respect purge requests when serving ipro requests from ngx_pagespeed (#1193)

* respect purge requests when serving ipro requests from ngx_pagespeed

* Add TODO to share common base with ApacheFetch.

During system-tests, don't write into the source-controlled htdocs area. (#1197)

Instead, use a new (not-yet-committed) target in Makefile.tests to build
a mirror htdocs in test/tmp/root that sym-links the large readonly
section and deep copies the test areas for purge and flush tests.

Pass the install-path into Makefile (#1200)

* Pass the install-path into Makefile, rather than having the Makefile
depend on PWD.

* Move the double-quotes be before the value, to be consistent with other
Makefiles.

Author: jmarantz

src/ngx_base_fetch.cc

@@ -26,6 +26,8 @@
 
 #include "ngx_pagespeed.h"
 
+#include "net/instaweb/rewriter/public/rewrite_driver.h"
+#include "net/instaweb/rewriter/public/rewrite_options.h"
 #include "net/instaweb/rewriter/public/rewrite_stats.h"
 #include "pagespeed/kernel/base/google_message_handler.h"
 #include "pagespeed/kernel/base/message_handler.h"
@@ -41,14 +43,18 @@ const char kDone = 'D';
 NgxEventConnection* NgxBaseFetch::event_connection = NULL;
 int NgxBaseFetch::active_base_fetches = 0;
 
-NgxBaseFetch::NgxBaseFetch(ngx_http_request_t* r,
+NgxBaseFetch::NgxBaseFetch(StringPiece url,
+                           ngx_http_request_t* r,
                            NgxServerContext* server_context,
                            const RequestContextPtr& request_ctx,
                            PreserveCachingHeaders preserve_caching_headers,
-                           NgxBaseFetchType base_fetch_type)
+                           NgxBaseFetchType base_fetch_type,
+                           const RewriteOptions* options)
     : AsyncFetch(request_ctx),
+      url_(url.data(), url.size()),
       request_(r),
       server_context_(server_context),
+      options_(options),
       done_called_(false),
       last_buf_sent_(false),
       references_(2),
@@ -341,4 +347,9 @@ void NgxBaseFetch::HandleDone(bool success) {
   DecrefAndDeleteIfUnreferenced();
 }
 
+bool NgxBaseFetch::IsCachedResultValid(const ResponseHeaders& headers) {
+  return OptionsAwareHTTPCacheCallback::IsCacheValid(
+      url_, *options_, request_context(), headers);
+}
+
 }  // namespace net_instaweb

src/ngx_base_fetch.h

@@ -43,6 +43,9 @@
 //    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_
@@ -59,6 +62,7 @@ extern "C" {
 #include "ngx_server_context.h"
 
 #include "net/instaweb/http/public/async_fetch.h"
+#include "net/instaweb/rewriter/public/rewrite_options.h"
 #include "pagespeed/kernel/base/string.h"
 #include "pagespeed/kernel/http/headers.h"
 
@@ -74,10 +78,12 @@ enum NgxBaseFetchType {
 
 class NgxBaseFetch : public AsyncFetch {
  public:
-  NgxBaseFetch(ngx_http_request_t* r, NgxServerContext* server_context,
+  NgxBaseFetch(StringPiece url, ngx_http_request_t* r,
+               NgxServerContext* server_context,
                const RequestContextPtr& request_ctx,
                PreserveCachingHeaders preserve_caching_headers,
-               NgxBaseFetchType base_fetch_type);
+               NgxBaseFetchType base_fetch_type,
+               const RewriteOptions* options);
   virtual ~NgxBaseFetch();
 
   // Statically initializes event_connection, require for PSOL and nginx to
@@ -125,6 +131,8 @@ class NgxBaseFetch : public AsyncFetch {
   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);
   virtual bool HandleFlush(MessageHandler* handler);
@@ -152,13 +160,15 @@ class NgxBaseFetch : public AsyncFetch {
   int DecrefAndDeleteIfUnreferenced();
 
   static NgxEventConnection* event_connection;
-  
+
   // Live count of NgxBaseFetch instances that are currently in use.
   static int active_base_fetches;
 
+  GoogleString url_;
   ngx_http_request_t* request_;
   GoogleString buffer_;
   NgxServerContext* server_context_;
+  const RewriteOptions* options_;
   bool done_called_;
   bool last_buf_sent_;
   // How many active references there are to this fetch. Starts at two,

src/ngx_pagespeed.cc

@@ -1589,10 +1589,12 @@ void ps_release_base_fetch(ps_request_ctx_t* ctx) {
 }
 
 // TODO(chaizhenhua): merge into NgxBaseFetch ctor
-void ps_create_base_fetch(ps_request_ctx_t* ctx,
-                               RequestContextPtr request_context,
-                               RequestHeaders* request_headers,
-                               NgxBaseFetchType type) {
+void ps_create_base_fetch(StringPiece url,
+                          ps_request_ctx_t* ctx,
+                          RequestContextPtr request_context,
+                          RequestHeaders* request_headers,
+                          NgxBaseFetchType type,
+                          const RewriteOptions* options) {
   CHECK(ctx->base_fetch == NULL) << "Pre-existing base fetch!";
 
   ngx_http_request_t* r = ctx->r;
@@ -1602,9 +1604,9 @@ void ps_create_base_fetch(ps_request_ctx_t* ctx,
   // it, and call Done() on the associated parent (Proxy or Resource) fetch. If
   // we fail before creating the associated fetch then we need to call Done() on
   // the BaseFetch ourselves.
-  ctx->base_fetch = new NgxBaseFetch(r, cfg_s->server_context,
-                                     request_context,
-                                     ctx->preserve_caching_headers, type);
+  ctx->base_fetch = new NgxBaseFetch(url, r, cfg_s->server_context, request_context,
+                                     ctx->preserve_caching_headers, type,
+                                     options);
   ctx->base_fetch->SetRequestHeadersTakingOwnership(request_headers);
 }
 
@@ -1869,16 +1871,17 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
   if (pagespeed_resource) {
     // TODO(jefftk): Set using_spdy appropriately.  See
     // ProxyInterface::ProxyRequestCallback
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kPageSpeedResource);
+    ps_create_base_fetch(url.Spec(), ctx, request_context,
+                         request_headers.release(), kPageSpeedResource,
+                         options);
     ResourceFetch::Start(
         url,
         custom_options.release() /* null if there aren't custom options */,
         false /* using_spdy */, cfg_s->server_context, ctx->base_fetch);
     return ps_async_wait_response(r);
   } else if (is_an_admin_handler) {
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kAdminPage);
+    ps_create_base_fetch(url.Spec(), ctx, request_context,
+                         request_headers.release(), kAdminPage, options);
     QueryParams query_params;
     query_params.ParseFromUrl(url);
 
@@ -1926,8 +1929,8 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
 
     if (options->domain_lawyer()->MapOriginUrl(
             url, &mapped_url, &host_header, &is_proxy) && is_proxy) {
-      ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                          kPageSpeedProxy);
+      ps_create_base_fetch(url.Spec(), ctx, request_context,
+                           request_headers.release(), kPageSpeedProxy, options);
 
       RewriteDriver* driver;
       if (custom_options.get() == NULL) {
@@ -1951,8 +1954,8 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
   }
 
   if (html_rewrite) {
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kHtmlTransform);
+    ps_create_base_fetch(url.Spec(), ctx, request_context,
+                         request_headers.release(), kHtmlTransform, options);
     // Do not store driver in request_context, it's not safe.
     RewriteDriver* driver;
 
@@ -1996,8 +1999,8 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
   if (options->in_place_rewriting_enabled() &&
       options->enabled() &&
       options->IsAllowed(url.Spec())) {
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kIproLookup);
+    ps_create_base_fetch(url.Spec(), ctx, request_context, request_headers.release(),
+                         kIproLookup, options);
 
     // Do not store driver in request_context, it's not safe.
     RewriteDriver* driver;

test/nginx_system_test.sh

@@ -46,7 +46,23 @@ POSITION_AUX="${POSITION_AUX:-unset}"
 PRIMARY_HOSTNAME="localhost:$PRIMARY_PORT"
 SECONDARY_HOSTNAME="localhost:$SECONDARY_PORT"
 
-SERVER_ROOT="$MOD_PAGESPEED_DIR/src/install/"
+this_dir="$( cd $(dirname "$0") && pwd)"
+echo this_dir=$this_dir
+TEST_TMP="$this_dir/tmp"
+rm -rf "$TEST_TMP"
+mkdir -p "$TEST_TMP"
+echo TEST_TMP=$TEST_TMP
+
+APACHE_DOC_SRC="$MOD_PAGESPEED_DIR/src/install/"
+SERVER_ROOT="$TEST_TMP/root"
+echo SERVER_ROOT=$SERVER_ROOT
+rm -rf "$SERVER_ROOT"
+mkdir -p "$SERVER_ROOT"
+export APACHE_DOC_ROOT="$SERVER_ROOT"
+
+mkdir -p "$APACHE_DOC_ROOT"
+make -f "$APACHE_DOC_SRC/Makefile.tests" setup_doc_root \
+  INSTALL_DATA_DIR="$APACHE_DOC_SRC"
 
 # We need check and check_not before we source SYSTEM_TEST_FILE that provides
 # them.
@@ -136,17 +152,13 @@ function fire_ab_load() {
   sleep 2
  }
 
-this_dir="$( cd $(dirname "$0") && pwd)"
-
 # stop nginx/valgrind
 killall -s KILL nginx
 # TODO(oschaaf): Fix waiting for valgrind on 32 bits systems.
 killall -s KILL memcheck-amd64-
 while pgrep nginx > /dev/null; do sleep 1; done
 while pgrep memcheck > /dev/null; do sleep 1; done
 
-TEST_TMP="$this_dir/tmp"
-rm -r "$TEST_TMP"
 check_simple mkdir -p "$TEST_TMP"
 PROXY_CACHE="$TEST_TMP/proxycache"
 TMP_PROXY_CACHE="$TEST_TMP/tmpproxycache"
@@ -646,14 +658,14 @@ check touch "$SECONDARY_CACHE/cache.flush"
 check touch "$IPRO_CACHE/cache.flush"
 sleep 1
 
-CACHE_TESTING_DIR="$SERVER_ROOT/mod_pagespeed_test/cache_flush/"
+CACHE_TESTING_DIR="$SERVER_ROOT/cache_flush/"
 CACHE_TESTING_TMPDIR="$CACHE_TESTING_DIR/$$"
 mkdir "$CACHE_TESTING_TMPDIR"
 cp "$CACHE_TESTING_DIR/cache_flush_test.html" "$CACHE_TESTING_TMPDIR/"
 CSS_FILE="$CACHE_TESTING_TMPDIR/update.css"
 echo ".class myclass { color: $COLOR0; }" > "$CSS_FILE"
 
-URL_PATH="mod_pagespeed_test/cache_flush/$$/cache_flush_test.html"
+URL_PATH="cache_flush/$$/cache_flush_test.html"
 
 URL="$SECONDARY_HOSTNAME/$URL_PATH"
 CACHE_A="--header=Host:cache_a.example.com"

test/pagespeed_test.conf.template

@@ -812,7 +812,7 @@ http {
     server_name www.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
-    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@;
+    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@/;
     pagespeed MapRewriteDomain cdn.example.com origin.example.com;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters rewrite_css,rewrite_images;
@@ -826,7 +826,7 @@ http {
     server_name origin.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
-    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@;
+    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@/;
     pagespeed MapRewriteDomain cdn.example.com origin.example.com;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters rewrite_css,rewrite_images;
@@ -840,7 +840,7 @@ http {
     server_name cdn.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
 
-    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@;
+    pagespeed LoadFromFile http://cdn.example.com @@SERVER_ROOT@@/;
     pagespeed MapRewriteDomain cdn.example.com origin.example.com;
     pagespeed RewriteLevel PassThrough;
     pagespeed EnableFilters rewrite_css,rewrite_images;
@@ -1332,7 +1332,7 @@ http {
     listen [::]:@@SECONDARY_PORT@@;
     server_name proxy-post-origin.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/";
   }
 
   server {
@@ -1350,7 +1350,7 @@ http {
     listen [::]:@@SECONDARY_PORT@@;
     server_name script-filters.example.com;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/";
     set $filters "";
     set $domain_shards "cdn1.example.com,cdn2.example.com";
     if ($http_X_Script) {
@@ -1372,7 +1372,7 @@ http {
     pagespeed EnableCachePurge on;
 
     pagespeed PurgeMethod PURGE;
-    root "@@SERVER_ROOT@@/mod_pagespeed_test/purge";
+    root "@@SERVER_ROOT@@/purge";
     pagespeed FileCachePath "@@FILE_CACHE@@_purge";
     pagespeed DisableFilters add_instrumentation;
     pagespeed RewriteLevel PassThrough;
@@ -1388,7 +1388,7 @@ http {
     pagespeed EnableCachePurge on;
 
     pagespeed PurgeMethod PURGE;
-    root "@@SERVER_ROOT@@/mod_pagespeed_test/purge";
+    root "@@SERVER_ROOT@@/purge";
     pagespeed FileCachePath "@@FILE_CACHE@@_dir_on";
     pagespeed DisableFilters add_instrumentation;
     pagespeed RewriteLevel PassThrough;
@@ -1406,7 +1406,7 @@ http {
     pagespeed InPlaceResourceOptimization off;
     pagespeed off;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/";
     location /mod_pagespeed_test/nostore {
       add_header "Cache-Control" "max-age=12345";
       add_header "Cache-Control" "public, no-store";
@@ -1421,7 +1421,7 @@ http {
     pagespeed InPlaceResourceOptimization off;
     pagespeed off;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/";
     location / {
       add_header "PageSpeedFilters" "add_instrumentation";
     }
@@ -1434,7 +1434,7 @@ http {
     pagespeed InPlaceResourceOptimization off;
     pagespeed off;
     pagespeed FileCachePath "@@FILE_CACHE@@";
-    root "@@SERVER_ROOT@@";
+    root "@@SERVER_ROOT@@/";
     location / {
       add_header "PageSpeed" "off";
     }
@@ -1759,7 +1759,7 @@ http {
     # optimized.
     location /mod_pagespeed_test/auth/ {
        auth_basic "Restricted";
-       auth_basic_user_file "@@SERVER_ROOT@@mod_pagespeed_test/auth/passwd.conf";
+       auth_basic_user_file "@@SERVER_ROOT@@/mod_pagespeed_test/auth/passwd.conf";
     }
 
     location /mod_pagespeed_test/ipro/cookie/ {