native fetcher ipv6 support - work in progress

This allows is to test again, but needs a lot of cleaning up.
More importantly, the google font inlining test fails, and I'm
not sure why that is, the fetch succeeds.

config

@@ -15,11 +15,9 @@
 # Environment Variables (Optional):
 #   MOD_PAGESPEED_DIR: absolute path to the mod_pagespeed/src directory
 #   PSOL_BINARY: absolute path to pagespeed_automatic.a
-#   PSOL_BUILDTYPE: Release or Debug
 
 mod_pagespeed_dir="${MOD_PAGESPEED_DIR:-unset}"
 position_aux="${POSITION_AUX:-unset}"
-psol_buildtype="${PSOL_BUILDTYPE:-unset}"
 
 if [ "$mod_pagespeed_dir" = "unset" ] ; then
   mod_pagespeed_dir="$ngx_addon_dir/psol/include"
@@ -77,7 +75,6 @@ else
   exit 1
 fi
 
-if [ "$psol_buildtype" = "unset" ] ; then
 if [ "$NGX_DEBUG" = "YES" ]; then
   buildtype=Debug
   # If we're using a psol tarball that doesn't contain Debug/ (which is the case
@@ -85,9 +82,6 @@ if [ "$psol_buildtype" = "unset" ] ; then
 else
   buildtype=Release
 fi
-else
-  buildtype=$psol_buildtype
-fi
 
 # If the compiler is gcc, we want to use g++ to link, if at all possible,
 # so that -static-libstdc++ works.

scripts/build_ngx_pagespeed.sh

@@ -634,13 +634,7 @@ Not deleting $directory; name is suspiciously short.  Something is wrong."
       run make apache_debug_psol
       PSOL_BINARY="$MOD_PAGESPEED_DIR/out/$BUILD_TYPE/pagespeed_automatic.a"
     else
-      if "$DO_DEPS_CHECK"; then
-        skip_deps_arg=""
-      else
-        skip_deps_arg="--skip_deps"
-      fi
-
-      run install/build_psol.sh --skip_tests --skip_packaging "$skip_deps_arg"
+      run install/build_psol.sh --skip_tests --skip_packaging
       PSOL_BINARY="$MOD_PAGESPEED_DIR/pagespeed/automatic/pagespeed_automatic.a"
     fi
     run popd

src/ngx_event_connection.cc

@@ -52,12 +52,6 @@ bool NgxEventConnection::Init(ngx_cycle_t* cycle) {
   } else {
     pipe_read_fd_ = file_descriptors[0];
     pipe_write_fd_ = file_descriptors[1];
-    // Attempt to bump the pipe capacity, because running out of buffer space
-    // can potentially lead up to writes spinning on EAGAIN.
-    // See https://github.com/pagespeed/ngx_pagespeed/issues/1380
-    // TODO(oschaaf): Consider implementing a queueing mechanism for retrying
-    // failed writes.
-    fcntl(pipe_write_fd_, F_SETPIPE_SZ, 200*1024 /* minimal amount of bytes */);
     return true;
   }
   close(file_descriptors[0]);

src/ngx_fetch.cc

@@ -140,6 +140,8 @@ NgxConnection* NgxConnection::Connect(ngx_peer_connection_t* pc,
 
   int rc = ngx_event_connect_peer(pc);
   if (rc == NGX_ERROR || rc == NGX_DECLINED || rc == NGX_BUSY) {
+    ngx_log_error(NGX_LOG_DEBUG, pc->log, 0,
+                    "NgxFetch: ngx_event_connect_peer failed");
     return NULL;
   }
 
@@ -363,14 +365,17 @@ bool NgxFetch::Init() {
 
   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(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_memzero(&sin_, sizeof(sin_));
+  if (s_ipaddress[0] == '[') {
+      s_ipaddress = s_ipaddress.substr(1);
+  }
+  if (s_ipaddress[s_ipaddress.size() - 1] == ']') {
+      s_ipaddress = s_ipaddress.substr(0, s_ipaddress.size()-2);      
+  }
+  ngx_int_t res = ngx_parse_addr(pool_, &sin_, (u_char*)s_ipaddress.c_str(), s_ipaddress.size());
+  if (res != NGX_OK) {
+    // Hostname isn't a valid IP address.  Check DNS.
     ngx_resolver_ctx_t temp;
     temp.name.data = tmp_url->host.data;
     temp.name.len = tmp_url->host.len;
@@ -405,6 +410,16 @@ bool NgxFetch::Init() {
       return false;
     }
   } else {
+      /*
+    u_char text[NGX_SOCKADDR_STRLEN];
+    ngx_str_t addrtext;
+    addrtext.len = ngx_sock_ntop(fetch->sin_.sockaddr, fetch->sin_.socklen, 
+        text, NGX_SOCKADDR_STRLEN, htons(fetcher->proxy_.url.len ? htons(fetcher->proxy_.port) : fetch->url_.port));
+    ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
+                    "NgxFetch %p: Resolved host [%V] to [%V]", fetch,
+                    &resolver_ctx->name, &addrtext);
+    */
+    ((struct sockaddr_in *)sin_.sockaddr)->sin_port = htons(fetcher_->proxy_.url.len ? htons(fetcher_->proxy_.port) : url_.port);
     if (InitRequest() != NGX_OK) {
       message_handler()->Message(kError, "NgxFetch: InitRequest failed");
       return false;
@@ -535,7 +550,7 @@ void NgxFetch::ResolveDoneHandler(ngx_resolver_ctx_t* resolver_ctx) {
   }
 
   ngx_uint_t i;
-  // Find the first ipv4 address. We don't support ipv6 yet.
+  // Find the first ipv4 address. We don't support ipv6 yet in dns results.
   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*).
@@ -566,37 +581,30 @@ void NgxFetch::ResolveDoneHandler(ngx_resolver_ctx_t* resolver_ctx) {
   ngx_memzero(&fetch->sin_, sizeof(fetch->sin_));
 
 #if (nginx_version < 1005008)
+  // TODO(oschaaf): need to test this again, might be broken.
   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;
+  fetch->sin_ = resolver_ctx->addrs[i];
 #endif
 
-  fetch->sin_.sin_family = AF_INET;
-  fetch->sin_.sin_port = htons(fetch->url_.port);
-
   // 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);
+  ((struct sockaddr_in *)fetch->sin_.sockaddr)->sin_port = 
+    htons(fetcher->proxy_.url.len ? htons(fetcher->proxy_.port) : fetch->url_.port);
 
+  u_char text[NGX_SOCKADDR_STRLEN];
+  ngx_str_t addrtext;
+  addrtext.len = ngx_sock_ntop(fetch->sin_.sockaddr, fetch->sin_.socklen, 
+    text, NGX_SOCKADDR_STRLEN, htons(fetcher->proxy_.url.len ? htons(fetcher->proxy_.port) : fetch->url_.port));
+  addrtext.data = text;
   ngx_log_error(NGX_LOG_DEBUG, fetch->log_, 0,
-                "NgxFetch %p: Resolved host [%V] to [%s]", fetch,
-                &resolver_ctx->name, ip_address);
-
-  fetch->release_resolver();
+                "NgxFetch %p: Resolved host [%V] to [%V]", fetch,
+                &resolver_ctx->name, &addrtext);
 
   if (fetch->InitRequest() != NGX_OK) {
     fetch->message_handler()->Message(kError, "NgxFetch: InitRequest failed");
     fetch->CallbackDone(false);
   }
+  fetch->release_resolver();
 }
 
 // Prepare the request data for this fetch, and hook the write event.
@@ -692,8 +700,8 @@ int NgxFetch::InitRequest() {
 int NgxFetch::Connect() {
   ngx_peer_connection_t pc;
   ngx_memzero(&pc, sizeof(pc));
-  pc.sockaddr = (struct sockaddr*)&sin_;
-  pc.socklen = sizeof(struct sockaddr_in);
+  pc.sockaddr =  sin_.sockaddr;
+  pc.socklen = sin_.socklen;
   pc.name = &url_.host;
 
   // get callback is dummy function, it just returns NGX_OK

src/ngx_fetch.h

@@ -191,7 +191,8 @@ class NgxFetch : public PoolElement<NgxFetch> {
   int64 content_length_;
   bool content_length_known_;
 
-  struct sockaddr_in sin_;
+
+  ngx_addr_t sin_;
   ngx_log_t* log_;
   ngx_buf_t* out_;
   ngx_buf_t* in_;

src/ngx_pagespeed.cc

@@ -84,7 +84,6 @@ extern ngx_module_t ngx_pagespeed;
 
 // Needed for SystemRewriteDriverFactory to use shared memory.
 #define PAGESPEED_SUPPORT_POSIX_SHARED_MEM
-#define NGINX_1_13_4 1013004
 
 net_instaweb::NgxRewriteDriverFactory* active_driver_factory = NULL;
 
@@ -2704,7 +2703,8 @@ ngx_int_t ps_simple_handler(ngx_http_request_t* r,
     }
   }
 
-  return send_out_headers_and_body(r, response_headers, output);
+  send_out_headers_and_body(r, response_headers, output);
+  return NGX_OK;
 }
 
 void ps_beacon_handler_helper(ngx_http_request_t* r,
@@ -3019,17 +3019,13 @@ ngx_int_t ps_preaccess_handler(ngx_http_request_t* r) {
   ph = cmcf->phase_engine.handlers;
 
   i = r->phase_handler;
-
   // move handlers before try_files && content phase
-  // As of nginx 1.13.4 we will be right before the try_files module
-  #if (nginx_version < NGINX_1_13_4)
   while (ph[i + 1].checker != ngx_http_core_try_files_phase &&
          ph[i + 1].checker != ngx_http_core_content_phase) {
     ph[i] = ph[i + 1];
     ph[i].next--;
     i++;
   }
-  #endif
 
   // insert ps phase handler
   ph[i].checker = ps_phase_handler;
@@ -3085,17 +3081,8 @@ ngx_int_t ps_init(ngx_conf_t* cf) {
     ngx_http_core_main_conf_t* cmcf = static_cast<ngx_http_core_main_conf_t*>(
         ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module));
 
-    int phase = NGX_HTTP_PREACCESS_PHASE;
-
-    // As of nginx 1.13.4, try_files has changed.
-    // https://github.com/nginx/nginx/commit/129b06dc5dfab7b4513a4f274b3778cd9b8a6a22
-#if (nginx_version >= NGINX_1_13_4)
-    phase = NGX_HTTP_PRECONTENT_PHASE;
-#endif
-
     ngx_http_handler_pt* h = static_cast<ngx_http_handler_pt*>(
-        ngx_array_push(&cmcf->phases[phase].handlers));
-
+        ngx_array_push(&cmcf->phases[NGX_HTTP_PREACCESS_PHASE].handlers));
     if (h == NULL) {
       return NGX_ERROR;
     }

test/pagespeed_test.conf.template

@@ -88,16 +88,6 @@ http {
   # Expanded to CentralControllerPort directive when RUN_CONTROLLER_TEST=on
   @@CONTROLLER@@
 
-  server {
-    # This is the default vhost for @@SECONDARY_PORT@@
-    # Your request will land here when you pass in an unknown host.
-    listen @@SECONDARY_PORT@@;
-    listen [::]:@@SECONDARY_PORT@@;
-    server_name default.example.com;
-    pagespeed FileCachePath "@@FILE_CACHE@@";
-    pagespeed RewriteLevel CoreFilters;
-  }
-
   server {
     # Sets up a logical home-page server on
     # max-cacheable-content-length.example.com.  This server is only used to