Unit test ApacheFetch and ApacheWriter.

* pulls ApacheFetch out into it's own file
* creates a set of mock apache functions that log to a global varible
* extends NullCondvarCapableMutex to support condvars
* comments on, but does not fix, bug in cookie stripping

Author: jeffkaufman

net/instaweb/apache/apache_writer.cc

@@ -23,7 +23,6 @@
 #include "net/instaweb/util/public/string_util.h"
 
 #include "apr_strings.h"  // for apr_pstrdup    // NOLINT
-#include "httpd.h"                              // NOLINT
 #include "http_protocol.h"                      // NOLINT
 
 namespace net_instaweb {
@@ -87,6 +86,12 @@ void ApacheWriter::OutputHeaders(ResponseHeaders* response_headers) {
   if (disable_downstream_header_filters_) {
     DisableDownstreamHeaderFilters(request_);
   }
+
+  // TODO(jefftk): Sanitize strips cookies and a lot of other headers.  It's
+  // being run after ResponseHeadersToApacheRequest(), however, which means it's
+  // not actually doing anything.  This code is not doing what it says it does,
+  // and should be fixed, but it's possible some users like the mobilizing proxy
+  // are depending on the current behavior.
   if (strip_cookies_ && response_headers->Sanitize()) {
     response_headers->ComputeCaching();
   }

net/instaweb/apache/apache_writer.h

@@ -20,7 +20,6 @@
 #include "net/instaweb/util/public/basictypes.h"
 #include "net/instaweb/util/public/string_util.h"
 #include "net/instaweb/util/public/writer.h"
-#include "httpd.h"  // NOLINT
 
 struct request_rec;
 
@@ -57,6 +56,7 @@ class ApacheWriter : public Writer {
 
   // Removes 'Set-Cookie' and 'Set-Cookie2' from the response headers
   // once they are complete.  Default is false.
+  // TODO(jefftk): Doesn't actually do anything, because of an old bug.
   void set_strip_cookies(bool x) {
     strip_cookies_ = x;
   }

net/instaweb/apache/header_util.cc

@@ -153,6 +153,27 @@ void PrintHeaders(request_rec* request) {
   fflush(stdout);
 }
 
+int StringAttributeCallback(void* rec, const char* key, const char* value) {
+  GoogleString* out = static_cast<GoogleString*>(rec);
+  out->append(key);
+  out->append(": ");
+  out->append(value);
+  out->append("\n");
+  return 1;
+}
+
+GoogleString HeadersOutToString(request_rec* request) {
+  GoogleString out;
+  apr_table_do(StringAttributeCallback, &out, request->headers_out, NULL);
+  return out;
+}
+
+GoogleString SubprocessEnvToString(request_rec* request) {
+  GoogleString out;
+  apr_table_do(StringAttributeCallback, &out, request->subprocess_env, NULL);
+  return out;
+}
+
 class ApacheCachingHeaders : public CachingHeaders {
  public:
   explicit ApacheCachingHeaders(request_rec* request)

net/instaweb/apache/header_util.h

@@ -80,6 +80,12 @@ void DisableDownstreamHeaderFilters(request_rec* request);
 // Debug utility for printing Apache headers to stdout
 void PrintHeaders(request_rec* request);
 
+// Get request->headers_out as a string, intended for tests.
+GoogleString HeadersOutToString(request_rec* request);
+
+// Get request->subprocess_env as a string, intended for tests.
+GoogleString SubprocessEnvToString(request_rec* request);
+
 // Updates headers related to caching (but not Cache-Control).
 void DisableCachingRelatedHeaders(request_rec* request);
 

net/instaweb/apache/header_util_test.cc

@@ -15,29 +15,15 @@
 // Author: jmarantz@google.com (Joshua Marantz)
 
 #include "net/instaweb/apache/header_util.h"
+
+#include "pagespeed/apache/mock_apache.h"
 #include "pagespeed/kernel/base/callback.h"
 #include "pagespeed/kernel/base/gtest.h"
 #include "pagespeed/kernel/http/http_names.h"
 #include "pagespeed/kernel/http/request_headers.h"
 
-#include "apr_pools.h"                                               // NOLINT
 #include "http_request.h"                                            // NOLINT
 
-// Provide stubs for functions normally resolved when loaded into
-// httpd, which we are not linking into the test.
-extern "C" {
-  void ap_set_content_type(request_rec* request, char* type) {
-    // Note that this is not the exact correct functionallity, but is
-    // sufficient for the test.
-    apr_table_set(request->headers_out,
-                  net_instaweb::HttpAttributes::kContentType,
-                  type);
-  }
-
-  void ap_remove_output_filter(ap_filter_t* filter) {
-  }
-}
-
 namespace net_instaweb {
 
 class HeaderUtilTest : public testing::Test {
@@ -48,15 +34,13 @@ class HeaderUtilTest : public testing::Test {
 
  protected:
   virtual void SetUp() {
-    apr_initialize();
-    atexit(apr_terminate);
-    apr_pool_create(&pool_, NULL);
-    request_.headers_in = apr_table_make(pool_, 10);
-    request_.headers_out = apr_table_make(pool_, 10);
+    MockApache::Initialize();
+    MockApache::PrepareRequest(&request_);
   }
 
   virtual void TearDown() {
-    apr_pool_destroy(pool_);
+    MockApache::CleanupRequest(&request_);
+    MockApache::Terminate();
   }
 
   void SetLastModified(const char* last_modified) {
@@ -77,7 +61,6 @@ class HeaderUtilTest : public testing::Test {
     return apr_table_get(request_.headers_out, HttpAttributes::kCacheControl);
   }
 
-  apr_pool_t* pool_;
   request_rec request_;
 };