HTTPTransport: callers should be able to obtain the HTTP response body.

This adds a new optional out-parameter to
HTTPTransport::ExecuteSynchronously() and provides Mac and Windows
implementations.

BUG=crashpad:5
R=rsesek@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/885183004

util/net/http_transport.h

@@ -74,9 +74,13 @@ class HTTPTransport {
   //! \brief Performs the HTTP request with the configured parameters and waits
   //!     for the execution to complete.
   //!
+  //! \param[out] response On success, this will be set to the HTTP response
+  //!     body. This parameter is optional and may be set to `nullptr` if the
+  //!     response body is not required.
+  //!
   //! \return Whether or not the request was successful, defined as returning
   //!     a HTTP status 200 (OK) code.
-  virtual bool ExecuteSynchronously() = 0;
+  virtual bool ExecuteSynchronously(std::string* response_body) = 0;
 
  protected:
   HTTPTransport();

util/net/http_transport_mac.mm

@@ -116,7 +116,7 @@ class HTTPTransportMac final : public HTTPTransport {
   HTTPTransportMac();
   ~HTTPTransportMac() override;
 
-  bool ExecuteSynchronously() override;
+  bool ExecuteSynchronously(std::string* response_body) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(HTTPTransportMac);
@@ -128,7 +128,7 @@ HTTPTransportMac::HTTPTransportMac() : HTTPTransport() {
 HTTPTransportMac::~HTTPTransportMac() {
 }
 
-bool HTTPTransportMac::ExecuteSynchronously() {
+bool HTTPTransportMac::ExecuteSynchronously(std::string* response_body) {
   DCHECK(body_stream());
 
   @autoreleasepool {
@@ -152,7 +152,7 @@ bool HTTPTransportMac::ExecuteSynchronously() {
 
     NSURLResponse* response = nil;
     NSError* error = nil;
-    [NSURLConnection sendSynchronousRequest:request
+    NSData* body = [NSURLConnection sendSynchronousRequest:request
                                          returningResponse:&response
                                                      error:&error];
 
@@ -178,6 +178,11 @@ bool HTTPTransportMac::ExecuteSynchronously() {
       return false;
     }
 
+    if (response_body) {
+      response_body->assign(static_cast<const char*>([body bytes]),
+                            [body length]);
+    }
+
     return true;
   }
 }

util/net/http_transport_test.cc

@@ -23,6 +23,7 @@
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/rand_util.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
 #include "gtest/gtest.h"
@@ -82,6 +83,18 @@ class HTTPTransportTestFixture : public MultiprocessExec {
     ASSERT_TRUE(LoggingWriteFile(
         WritePipeHandle(), &response_code_, sizeof(response_code_)));
 
+    // The parent will also tell the web server what response body to send back.
+    // The web server will only send the response body if the response code is
+    // 200.
+    std::string expect_response_body;
+    for (size_t index = 0; index < 8; ++index) {
+      expect_response_body += static_cast<char>(base::RandInt(' ', '~'));
+    }
+
+    ASSERT_TRUE(LoggingWriteFile(WritePipeHandle(),
+                                 expect_response_body.c_str(),
+                                 expect_response_body.size()));
+
     // Now execute the HTTP request.
     scoped_ptr<HTTPTransport> transport(HTTPTransport::Create());
     transport->SetMethod("POST");
@@ -91,7 +104,16 @@ class HTTPTransportTestFixture : public MultiprocessExec {
     }
     transport->SetBodyStream(body_stream_.Pass());
 
-    EXPECT_EQ(transport->ExecuteSynchronously(), (response_code_ == 200));
+    std::string response_body;
+    bool success = transport->ExecuteSynchronously(&response_body);
+    if (response_code_ == 200) {
+      EXPECT_TRUE(success);
+      expect_response_body += "\r\n";
+      EXPECT_EQ(expect_response_body, response_body);
+    } else {
+      EXPECT_FALSE(success);
+      EXPECT_TRUE(response_body.empty());
+    }
 
     // Read until the child's stdout closes.
     std::string request;

util/net/http_transport_test_server.py

@@ -17,10 +17,13 @@
 
 """A one-shot testing webserver.
 
-When invoked, this server will write an integer to stdout, indiciating on which
-port the server is listening. It will then read one integer from stdin,
-indiciating the response code to set for a request. The server will process
-one HTTP request, writing it out entirely to stdout, and will then terminate.
+When invoked, this server will write a short integer to stdout, indiciating on
+which port the server is listening. It will then read one integer from stdin,
+indiciating the response code to be sent in response to a request. It also reads
+8 characters from stdin, which, after having "\r\n" appended, will form the
+response body in a successful response (one with code 200). The server will
+process one HTTP request, deliver the prearranged response to the client, and
+write the entire request to stdout. It will then terminate.
 
 This server is written in Python since it provides a simple HTTP stack, and
 because parsing Chunked encoding is safer and easier in a memory-safe language.
@@ -57,6 +60,7 @@ class BufferedReadFile(object):
 
 class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
   response_code = 500
+  response_body = ''
 
   def handle_one_request(self):
     # Wrap the rfile in the buffering file object so that the raw header block
@@ -78,6 +82,9 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     self.send_response(self.response_code)
     self.end_headers()
+    if self.response_code == 200:
+      self.wfile.write(self.response_body)
+      self.wfile.write('\r\n')
 
     writer.write(body)
     writer.flush()
@@ -131,10 +138,10 @@ def Main():
   sys.stdout.write(struct.pack('=H', server.server_address[1]))
   sys.stdout.flush()
 
-  # Read the desired test response code as an unsigned short from the parent
-  # process.
-  RequestHandler.response_code = \
-      struct.unpack('=H', sys.stdin.read(struct.calcsize('=H')))[0]
+  # Read the desired test response code as an unsigned short and the desired
+  # response body as an 8-byte string from the parent process.
+  RequestHandler.response_code, RequestHandler.response_body = \
+      struct.unpack('=H8s', sys.stdin.read(struct.calcsize('=H8s')))
 
   # Handle the request.
   server.handle_request()

util/net/http_transport_win.cc

@@ -73,7 +73,7 @@ class HTTPTransportWin final : public HTTPTransport {
   HTTPTransportWin();
   ~HTTPTransportWin() override;
 
-  bool ExecuteSynchronously() override;
+  bool ExecuteSynchronously(std::string* response_body) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(HTTPTransportWin);
@@ -85,7 +85,7 @@ HTTPTransportWin::HTTPTransportWin() : HTTPTransport() {
 HTTPTransportWin::~HTTPTransportWin() {
 }
 
-bool HTTPTransportWin::ExecuteSynchronously() {
+bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) {
   ScopedHINTERNET session(
       WinHttpOpen(base::UTF8ToUTF16(PACKAGE_NAME "/" PACKAGE_VERSION).c_str(),
                   WINHTTP_ACCESS_TYPE_DEFAULT_PROXY,
@@ -154,8 +154,9 @@ bool HTTPTransportWin::ExecuteSynchronously() {
   std::vector<uint8_t> post_data;
 
   // Write the body of a POST if any.
+  const size_t kBufferSize = 4096;
   for (;;) {
-    uint8_t buffer[4096];
+    uint8_t buffer[kBufferSize];
     ssize_t bytes_to_write =
         body_stream()->GetBytesBuffer(buffer, sizeof(buffer));
     if (bytes_to_write == 0)
@@ -198,8 +199,25 @@ bool HTTPTransportWin::ExecuteSynchronously() {
     return false;
   }
 
-  // TODO(scottmg): Retrieve body of response if necessary with
-  // WinHttpQueryDataAvailable and WinHttpReadData.
+  if (response_body) {
+    response_body->clear();
+
+    // There isn’t any reason to call WinHttpQueryDataAvailable(), because it
+    // returns the number of bytes available to be read without blocking at the
+    // time of the call, not the number of bytes until end-of-file. This method,
+    // which executes synchronously, is only concerned with reading until EOF.
+    DWORD bytes_read = 0;
+    do {
+      char read_buffer[kBufferSize];
+      if (!WinHttpReadData(
+              request.get(), read_buffer, sizeof(read_buffer), &bytes_read)) {
+        LogErrorWinHttpMessage("WinHttpReadData");
+        return false;
+      }
+
+      response_body->append(read_buffer, bytes_read);
+    } while (bytes_read > 0);
+  }
 
   return true;
 }