diff --git a/util/net/http_transport.h b/util/net/http_transport.h index 55482153..333986a9 100644 --- a/util/net/http_transport.h +++ b/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(); diff --git a/util/net/http_transport_mac.mm b/util/net/http_transport_mac.mm index 2170b1ff..135fd280 100644 --- a/util/net/http_transport_mac.mm +++ b/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,9 +152,9 @@ bool HTTPTransportMac::ExecuteSynchronously() { NSURLResponse* response = nil; NSError* error = nil; - [NSURLConnection sendSynchronousRequest:request - returningResponse:&response - error:&error]; + NSData* body = [NSURLConnection sendSynchronousRequest:request + returningResponse:&response + error:&error]; if (error) { LOG(ERROR) << [[error localizedDescription] UTF8String] << " (" @@ -178,6 +178,11 @@ bool HTTPTransportMac::ExecuteSynchronously() { return false; } + if (response_body) { + response_body->assign(static_cast([body bytes]), + [body length]); + } + return true; } } diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index 8fb8a57d..633ed341 100644 --- a/util/net/http_transport_test.cc +++ b/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(base::RandInt(' ', '~')); + } + + ASSERT_TRUE(LoggingWriteFile(WritePipeHandle(), + expect_response_body.c_str(), + expect_response_body.size())); + // Now execute the HTTP request. scoped_ptr 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; diff --git a/util/net/http_transport_test_server.py b/util/net/http_transport_test_server.py index 51d722e1..ecf71d70 100755 --- a/util/net/http_transport_test_server.py +++ b/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() diff --git a/util/net/http_transport_win.cc b/util/net/http_transport_win.cc index 8e1cef31..99f87ba2 100644 --- a/util/net/http_transport_win.cc +++ b/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 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; }