headers: use headers generated by pagespeed

Pagespeed puts headers in ngx_base_fetch->response_headers() and nginx calls
NgxBaseFetch::CollectHeaders to get them out, which wasn't implemented before.

Other minor changes in this commit:
 - whitespace fixes
 - ordering of includes
 - ngx_http_pagespeed_str_to_string_piece now takes its argument by value

src/ngx_base_fetch.cc

@@ -18,9 +18,9 @@
 
 #include "ngx_base_fetch.h"
 #include "ngx_pagespeed.h"
+#include "net/instaweb/http/public/response_headers.h"
 #include "net/instaweb/util/public/google_message_handler.h"
 #include "net/instaweb/util/public/message_handler.h"
-#include "net/instaweb/http/public/response_headers.h"
 
 namespace net_instaweb {
 
@@ -46,7 +46,7 @@ void NgxBaseFetch::PopulateResponseHeaders() {
   response_headers()->Add(
       HttpAttributes::kContentType,
       ngx_http_pagespeed_str_to_string_piece(
-          &request_->headers_out.content_type));
+          request_->headers_out.content_type));
 }
 
 template<class HeadersT>
@@ -73,9 +73,8 @@ void NgxBaseFetch::CopyHeadersFromTable(ngx_list_t* headers_from,
       i = 0;
     }
 
-    StringPiece key = ngx_http_pagespeed_str_to_string_piece(&header[i].key);
-    StringPiece value = ngx_http_pagespeed_str_to_string_piece(
-        &header[i].value);
+    StringPiece key = ngx_http_pagespeed_str_to_string_piece(header[i].key);
+    StringPiece value = ngx_http_pagespeed_str_to_string_piece(header[i].value);
 
     headers_to->Add(key, value);
   }
@@ -177,7 +176,7 @@ ngx_int_t NgxBaseFetch::CopyBufferToNginx(ngx_chain_t** link_ptr) {
   if (done_called_) {
     tail_link->buf->last_buf = true;
     last_buf_sent_ = true;
-  }  
+  }
 
   return NGX_OK;
 }
@@ -204,12 +203,18 @@ ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
   const ResponseHeaders* pagespeed_headers = response_headers();
   Unlock();
 
-  headers_out->status = NGX_HTTP_OK;
+  headers_out->status = pagespeed_headers->status_code();
 
   ngx_int_t i;
   for (i = 0 ; i < pagespeed_headers->NumAttributes() ; i++) {
-    const GoogleString& name = pagespeed_headers->Name(i);
-    const GoogleString& value = pagespeed_headers->Value(i);
+    const GoogleString& name_gs = pagespeed_headers->Name(i);
+    const GoogleString& value_gs = pagespeed_headers->Value(i);
+
+    ngx_str_t name, value;
+    name.len = name_gs.length();
+    name.data = reinterpret_cast<u_char*>(const_cast<char*>(name_gs.data()));
+    value.len = value_gs.length();
+    value.data = reinterpret_cast<u_char*>(const_cast<char*>(value_gs.data()));
 
     // TODO(jefftk): If we're setting a cache control header we'd like to
     // prevent any downstream code from changing it.  Specifically, if we're
@@ -219,8 +224,51 @@ ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
     // shouldn't apply to our generated resources.  See Apache code in
     // net/instaweb/apache/header_util:AddResponseHeadersToRequest
 
-    // TODO(jefftk): actually copy headers over
-    fprintf(stderr, "Would set header '%s: %s'\n", name.c_str(), value.c_str());
+    // Make copies of name and value to put into headers_out.
+
+    u_char* value_s = ngx_pstrdup(request_->pool, &value);
+    if (value_s == NULL) {
+      return NGX_ERROR;
+    }
+
+    if (STR_EQ_LITERAL(name, "Content-Type")) {
+      // Unlike all the other headers, content_type is just a string.
+      headers_out->content_type.data = value_s;
+      headers_out->content_type.len = value.len;
+      continue;
+    }
+
+    u_char* name_s = ngx_pstrdup(request_->pool, &name);
+    if (name_s == NULL) {
+      return NGX_ERROR;
+    }
+
+    ngx_table_elt_t* header = static_cast<ngx_table_elt_t*>(
+        ngx_list_push(&headers_out->headers));
+    if (header == NULL) {
+      return NGX_ERROR;
+    }
+
+    header->hash = 1;  // Include this header in the output.
+    header->key.len = name.len;
+    header->key.data = name_s;
+    header->value.len = value.len;
+    header->value.data = value_s;
+
+    // Populate the shortcuts to commonly used headers.
+    if (STR_EQ_LITERAL(name, "Date")) {
+      headers_out->date = header;
+    } else if (STR_EQ_LITERAL(name, "Etag")) {
+      headers_out->etag = header;
+    } else if (STR_EQ_LITERAL(name, "Expires")) {
+      headers_out->expires = header;
+    } else if (STR_EQ_LITERAL(name, "Last-Modified")) {
+      headers_out->last_modified = header;
+    } else if (STR_EQ_LITERAL(name, "Location")) {
+      headers_out->location = header;
+    } else if (STR_EQ_LITERAL(name, "Server")) {
+      headers_out->server = header;
+    }
   }
 
   return NGX_OK;

src/ngx_pagespeed.cc

@@ -74,7 +74,6 @@ typedef struct {
   int pipe_fd;
   ngx_connection_t* pagespeed_connection;
   ngx_http_request_t* r;
-  const net_instaweb::ContentType* content_type;
   bool is_resource_fetch;
   bool sent_headers;
   bool write_pending;
@@ -152,11 +151,6 @@ static ngx_command_t ngx_http_pagespeed_commands[] = {
   ngx_null_command
 };
 
-StringPiece
-ngx_http_pagespeed_str_to_string_piece(ngx_str_t* s) {
-  return StringPiece(reinterpret_cast<char*>(s->data), s->len);
-}
-
 static void*
 ngx_http_pagespeed_create_srv_conf(ngx_conf_t* cf) {
   ngx_http_pagespeed_srv_conf_t* conf;
@@ -201,7 +195,7 @@ ngx_http_pagespeed_release_request_context(void* data) {
   // before then we need to tell it to delete itself.
   //
   // If this is a resource fetch then proxy_fetch was never initialized.
-  if (ctx->proxy_fetch != NULL) {    
+  if (ctx->proxy_fetch != NULL) {
     ctx->proxy_fetch->Done(false /* failure */);
   }
 
@@ -264,7 +258,7 @@ ngx_http_pagespeed_determine_url(ngx_http_request_t* r) {
   }
 
   StringPiece host =
-      ngx_http_pagespeed_str_to_string_piece(&r->headers_in.server);
+      ngx_http_pagespeed_str_to_string_piece(r->headers_in.server);
   if (host.size() == 0) {
     // If host is unspecified, perhaps because of a pure HTTP 1.0 "GET /path",
     // fall back to server IP address.  Based on ngx_http_variable_server_addr.
@@ -276,12 +270,12 @@ ngx_http_pagespeed_determine_url(ngx_http_request_t* r) {
     if (rc != NGX_OK) {
       s.len = 0;
     }
-    host = ngx_http_pagespeed_str_to_string_piece(&s);
+    host = ngx_http_pagespeed_str_to_string_piece(s);
   }
 
   return net_instaweb::StrCat(
       is_https ? "https://" : "http://", host, port_string,
-      ngx_http_pagespeed_str_to_string_piece(&r->unparsed_uri));
+      ngx_http_pagespeed_str_to_string_piece(r->unparsed_uri));
 }
 
 // Get the context for this request.  ngx_http_pagespeed_create_request_context
@@ -307,7 +301,7 @@ ngx_http_pagespeed_initialize_server_context(
 
   cfg->driver_factory = new net_instaweb::NgxRewriteDriverFactory();
   cfg->driver_factory->set_filename_prefix(
-      ngx_http_pagespeed_str_to_string_piece(&cfg->cache_dir));
+      ngx_http_pagespeed_str_to_string_piece(cfg->cache_dir));
   cfg->server_context = cfg->driver_factory->CreateServerContext();
   cfg->proxy_fetch_factory =
       new net_instaweb::ProxyFetchFactory(cfg->server_context);
@@ -388,7 +382,7 @@ ngx_http_pagespeed_update(ngx_http_pagespeed_request_ctx_t* ctx,
       return rc;
     }
   }
-  
+
   return done ? NGX_OK : NGX_AGAIN;
 }
 
@@ -465,7 +459,7 @@ ngx_http_pagespeed_connection_read_handler(ngx_event_t* ev) {
 
   if (rc == NGX_AGAIN) {
     // Request needs more work by pagespeed.
-    rc = ngx_handle_read_event(ev, 0);    
+    rc = ngx_handle_read_event(ev, 0);
     CHECK(rc == NGX_OK);
   } else if (rc == NGX_OK) {
     // Pagespeed is done.  Stop watching the pipe.  If we still have data to
@@ -534,7 +528,7 @@ ngx_http_pagespeed_create_request_context(ngx_http_request_t* r,
     ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "invalid url");
 
     // Let nginx deal with the error however it wants; we will see a NULL ctx in
-    // the body filter or content handler and do nothing.    
+    // the body filter or content handler and do nothing.
     return is_resource_fetch ? NGX_DECLINED : NGX_OK;
   }
 
@@ -660,7 +654,7 @@ ngx_http_pagespeed_create_request_context(ngx_http_request_t* r,
           custom_options);
     }
     driver->set_log_record(ctx->base_fetch->log_record());
-    
+
     // TODO(jefftk): FlushEarlyFlow would go here.
 
     // Will call StartParse etc.  The rewrite driver will take care of deleting
@@ -696,7 +690,7 @@ ngx_http_pagespeed_send_to_pagespeed(
   int last_buf = 0;
   for (cur = in; cur != NULL; cur = cur->next) {
     last_buf = cur->buf->last_buf;
-    
+
     // Buffers are not really the last buffer until they've been through
     // pagespeed.
     cur->buf->last_buf = 0;
@@ -737,19 +731,6 @@ ngx_http_pagespeed_body_filter(ngx_http_request_t* r, ngx_chain_t* in) {
   ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                  "http pagespeed filter \"%V\"", &r->uri);
 
-  if (ctx->content_type == NULL) {
-    // We don't know what type of resource this is, but we only want to send
-    // html through to pagespeed.  Check the content type header and find out.
-    ctx->content_type = net_instaweb::MimeTypeToContentType(
-        ngx_http_pagespeed_str_to_string_piece(
-            &r->headers_out.content_type));
-    if (ctx->content_type == NULL || !ctx->content_type->IsHtmlLike()) {
-      // Unknown or otherwise non-html content type: skip it.
-      ngx_http_set_ctx(r, NULL, ngx_pagespeed);
-      return ngx_http_next_body_filter(r, in);
-    }
-  }
-
   if (!ctx->data_received) {
     // This is the first set of buffers we've got for this request.
     ctx->data_received = true;
@@ -763,12 +744,22 @@ ngx_http_pagespeed_body_filter(ngx_http_request_t* r, ngx_chain_t* in) {
   if (in != NULL) {
     // Send all input data to the proxy fetch.
     ngx_http_pagespeed_send_to_pagespeed(r, ctx, in);
-  }  
+  }
 
   ngx_http_pagespeed_set_buffered(r, true);
   return NGX_AGAIN;
 }
 
+#ifndef ngx_http_clear_etag
+// The ngx_http_clear_etag(r) macro was added in 1.3.3.  Backport it if it's not
+// present.
+#define ngx_http_clear_etag(r)       \
+  if (r->headers_out.etag) {         \
+    r->headers_out.etag->hash = 0;   \
+    r->headers_out.etag = NULL;      \
+  }
+#endif
+
 static ngx_int_t
 ngx_http_pagespeed_header_filter(ngx_http_request_t* r) {
   ngx_http_pagespeed_request_ctx_t* ctx =
@@ -788,8 +779,7 @@ ngx_http_pagespeed_header_filter(ngx_http_request_t* r) {
   // to pagespeed.  Check the content type header and find out.
   const net_instaweb::ContentType* content_type = 
       net_instaweb::MimeTypeToContentType(
-          ngx_http_pagespeed_str_to_string_piece(
-              &r->headers_out.content_type));
+          ngx_http_pagespeed_str_to_string_piece(r->headers_out.content_type));
   if (content_type == NULL || !content_type->IsHtmlLike()) {
     // Unknown or otherwise non-html content type: skip it.
     return ngx_http_next_header_filter(r);
@@ -809,15 +799,37 @@ ngx_http_pagespeed_header_filter(ngx_http_request_t* r) {
   // and calculate on the fly.
   ngx_http_clear_content_length(r);
 
-  // Pagespeed doesn't need etags: html is always not to be cached while
-  // resources are modified to have an etag-like hash in the url.
+  // Pagespeed html doesn't need etags: it should never be cached.
   ngx_http_clear_etag(r);
 
   // An page may change without the underlying file changing, because of how
-  // resources are included.  Pagespeed adds cache control headers for resources
-  // instead of using the last modified header.
+  // resources are included.  Pagespeed adds cache control headers for
+  // resources instead of using the last modified header.
   ngx_http_clear_last_modified(r);
 
+  // Don't cache html.  See mod_instaweb:instaweb_fix_headers_filter.
+  // Based on ngx_http_add_cache_control.
+  if (r->headers_out.cache_control.elts == NULL) {
+    ngx_int_t rc = ngx_array_init(&r->headers_out.cache_control, r->pool,
+                                  1, sizeof(ngx_table_elt_t *));
+    if (rc != NGX_OK) {
+      return NGX_ERROR;
+    }
+  }
+  ngx_table_elt_t** cache_control_headers = static_cast<ngx_table_elt_t**>(
+      ngx_array_push(&r->headers_out.cache_control));
+  if (cache_control_headers == NULL) {
+    return NGX_ERROR;
+  }
+  cache_control_headers[0] = static_cast<ngx_table_elt_t*>(
+      ngx_list_push(&r->headers_out.headers));
+  if (cache_control_headers[0] == NULL) {
+    return NGX_ERROR;
+  }
+  cache_control_headers[0]->hash = 1;
+  ngx_str_set(&cache_control_headers[0]->key, "Cache-Control");
+  ngx_str_set(&cache_control_headers[0]->value, "max-age=0, no-cache");
+
   r->filter_need_in_memory = 1;
 
   // Set the "X-Page-Speed: VERSION" header.

src/ngx_pagespeed.h

@@ -25,6 +25,15 @@ extern "C" {
 
 #include "net/instaweb/util/public/string_util.h"
 
-StringPiece ngx_http_pagespeed_str_to_string_piece(ngx_str_t* s);
+static StringPiece
+ngx_http_pagespeed_str_to_string_piece(ngx_str_t s) {
+  return StringPiece(reinterpret_cast<char*>(s.data), s.len);
+}
+
+// s1: ngx_str_t, s2: string literal
+// true if they're equal, false otherwise
+#define STR_EQ_LITERAL(s1, s2)          \
+    ((s1).len == (sizeof(s2)-1) &&      \
+     ngx_strncmp((s1).data, (s2), (sizeof(s2)-1)) == 0)
 
 #endif  // NGX_PAGESPEED_H_