[#491,!363] Updated unit tests for HTTP listener using transactions.

src/lib/http/tests/server_client_unittests.cc

@@ -11,6 +11,7 @@
 #include <http/client.h>
 #include <http/http_types.h>
 #include <http/listener.h>
+#include <http/listener_impl.h>
 #include <http/post_request_json.h>
 #include <http/response_creator.h>
 #include <http/response_creator_factory.h>
@@ -190,6 +191,206 @@ public:
     }
 };
 
+/// @brief Implementation of the HTTP listener used in tests.
+///
+/// This implementation replaces the @c HttpConnection type with a custom
+/// implementation.
+///
+/// @tparam HttpConnectionType Type of the connection object to be used by
+/// the listener implementation.
+template<typename HttpConnectionType>
+class HttpListenerImplCustom : public HttpListenerImpl {
+public:
+
+    HttpListenerImplCustom(IOService& io_service,
+                           const IOAddress& server_address,
+                           const unsigned short server_port,
+                           const HttpResponseCreatorFactoryPtr& creator_factory,
+                           const long request_timeout,
+                           const long idle_timeout)
+        : HttpListenerImpl(io_service, server_address, server_port,
+                           creator_factory, request_timeout,
+                           idle_timeout) {
+    }
+
+protected:
+
+    /// @brief Creates an instance of the @c HttpConnection.
+    ///
+    /// This method is virtual so as it can be overriden when customized
+    /// connections are to be used, e.g. in case of unit testing.
+    ///
+    /// @param io_service IO service to be used by the connection.
+    /// @param acceptor Reference to the TCP acceptor object used to listen for
+    /// new HTTP connections.
+    /// @param connection_pool Connection pool in which this connection is
+    /// stored.
+    /// @param response_creator Pointer to the response creator object used to
+    /// create HTTP response from the HTTP request received.
+    /// @param callback Callback invoked when new connection is accepted.
+    /// @param request_timeout Configured timeout for a HTTP request.
+    /// @param idle_timeout Timeout after which persistent HTTP connection is
+    /// closed by the server.
+    ///
+    /// @return Pointer to the created connection.
+    virtual HttpConnectionPtr createConnection(IOService& io_service,
+                                               HttpAcceptor& acceptor,
+                                               HttpConnectionPool& connection_pool,
+                                               const HttpResponseCreatorPtr& response_creator,
+                                               const HttpAcceptorCallback& callback,
+                                               const long request_timeout,
+                                               const long idle_timeout) {
+        HttpConnectionPtr
+            conn(new HttpConnectionType(io_service_, acceptor_, connections_,
+                                        response_creator, callback,
+                                        request_timeout_, idle_timeout_));
+        return (conn);
+    }
+
+
+};
+
+/// @brief Derivation of the @c HttpListener used in tests.
+///
+/// This class replaces the default implementation instance with the
+/// @c HttpListenerImplCustom using the customized connection type.
+///
+/// @tparam HttpConnectionType Type of the connection object to be used by
+/// the listener implementation.
+template<typename HttpConnectionType>
+class HttpListenerCustom : public HttpListener {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param io_service IO service to be used by the listener.
+    /// @param server_address Address on which the HTTP service should run.
+    /// @param server_port Port number on which the HTTP service should run.
+    /// @param creator_factory Pointer to the caller-defined
+    /// @ref HttpResponseCreatorFactory derivation which should be used to
+    /// create @ref HttpResponseCreator instances.
+    /// @param request_timeout Timeout after which the HTTP Request Timeout
+    /// is generated.
+    /// @param idle_timeout Timeout after which an idle persistent HTTP
+    /// connection is closed by the server.
+    ///
+    /// @throw HttpListenerError when any of the specified parameters is
+    /// invalid.
+    HttpListenerCustom(IOService& io_service,
+                       const IOAddress& server_address,
+                       const unsigned short server_port,
+                       const HttpResponseCreatorFactoryPtr& creator_factory,
+                       const HttpListener::RequestTimeout& request_timeout,
+                       const HttpListener::IdleTimeout& idle_timeout)
+        : HttpListener(io_service, server_address, server_port, creator_factory,
+                       request_timeout, idle_timeout) {
+        // Replace the default implementation with the customized version
+        // using the custom derivation of the HttpConnection.
+        impl_.reset(new HttpListenerImplCustom<HttpConnectionType>
+                    (io_service, server_address, server_port,
+                     creator_factory, request_timeout.value_,
+                     idle_timeout.value_));
+    }
+};
+
+/// @brief Implementation of the @c HttpConnection which injects greater
+/// length value than the buffer size into the write socket callback.
+class HttpConnectionLongWriteBuffer : public HttpConnection {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param io_service IO service to be used by the connection.
+    /// @param acceptor Reference to the TCP acceptor object used to listen for
+    /// new HTTP connections.
+    /// @param connection_pool Connection pool in which this connection is
+    /// stored.
+    /// @param response_creator Pointer to the response creator object used to
+    /// create HTTP response from the HTTP request received.
+    /// @param callback Callback invoked when new connection is accepted.
+    /// @param request_timeout Configured timeout for a HTTP request.
+    /// @param idle_timeout Timeout after which persistent HTTP connection is
+    /// closed by the server.
+    HttpConnectionLongWriteBuffer(IOService& io_service,
+                                  HttpAcceptor& acceptor,
+                                  HttpConnectionPool& connection_pool,
+                                  const HttpResponseCreatorPtr& response_creator,
+                                  const HttpAcceptorCallback& callback,
+                                  const long request_timeout,
+                                  const long idle_timeout)
+        : HttpConnection(io_service, acceptor, connection_pool,
+                         response_creator, callback, request_timeout,
+                         idle_timeout) {
+    }
+
+    /// @brief Callback invoked when data is sent over the socket.
+    ///
+    /// @param transaction Pointer to the transaction for which the callback
+    /// is invoked.
+    /// @param ec Error code.
+    /// @param length Length of the data sent.
+    virtual void socketWriteCallback(HttpConnection::TransactionPtr transaction,
+                                     boost::system::error_code ec,
+                                     size_t length) {
+        // Pass greater length of the data written. The callback should deal
+        // with this and adjust the data length.
+        HttpConnection::socketWriteCallback(transaction, ec, length + 1);
+    }
+
+};
+
+/// @brief Implementation of the @c HttpConnection which replaces
+/// transaction instance prior to calling write socket callback.
+class HttpConnectionTransactionChange : public HttpConnection {
+public:
+
+
+    /// @brief Constructor.
+    ///
+    /// @param io_service IO service to be used by the connection.
+    /// @param acceptor Reference to the TCP acceptor object used to listen for
+    /// new HTTP connections.
+    /// @param connection_pool Connection pool in which this connection is
+    /// stored.
+    /// @param response_creator Pointer to the response creator object used to
+    /// create HTTP response from the HTTP request received.
+    /// @param callback Callback invoked when new connection is accepted.
+    /// @param request_timeout Configured timeout for a HTTP request.
+    /// @param idle_timeout Timeout after which persistent HTTP connection is
+    /// closed by the server.
+    HttpConnectionTransactionChange(IOService& io_service,
+                                    HttpAcceptor& acceptor,
+                                    HttpConnectionPool& connection_pool,
+                                    const HttpResponseCreatorPtr& response_creator,
+                                    const HttpAcceptorCallback& callback,
+                                    const long request_timeout,
+                                    const long idle_timeout)
+        : HttpConnection(io_service, acceptor, connection_pool,
+                         response_creator, callback, request_timeout,
+                         idle_timeout) {
+    }
+
+    /// @brief Callback invoked when data is sent over the socket.
+    ///
+    /// @param transaction Pointer to the transaction for which the callback
+    /// is invoked.
+    /// @param ec Error code.
+    /// @param length Length of the data sent.
+    virtual void socketWriteCallback(HttpConnection::TransactionPtr transaction,
+                                     boost::system::error_code ec,
+                                     size_t length) {
+        // Replace the transaction. The socket callback should deal with this
+        // gracefully. It should detect that the output buffer is empty. Then
+        // try to see if the connection is persistent. This check should fail,
+        // because the request hasn't been created/finalized. The exception
+        // thrown upon checking the persistence should be caught and the
+        // connection closed.
+        transaction = HttpConnection::Transaction::create(response_creator_);
+        HttpConnection::socketWriteCallback(transaction, ec, length);
+    }
+};
+
+
 /// @brief Entity which can connect to the HTTP server endpoint.
 class TestHttpClient : public boost::noncopyable {
 public:
@@ -468,6 +669,86 @@ public:
         return (s.str());
     }
 
+    /// @brief Tests that HTTP request tiemout status is returned when the
+    /// server does not receive the entire request.
+    ///
+    /// @param request Partial request for which the parser will be waiting for
+    /// the next chunks of data.
+    /// @param expected_version HTTP version expected in the response.
+    void testRequestTimeout(const std::string& request,
+                            const HttpVersion& expected_version) {
+        // Open the listener with the Request Timeout of 1 sec and post the
+        // partial request.
+        HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT,
+                              factory_, HttpListener::RequestTimeout(1000),
+                              HttpListener::IdleTimeout(IDLE_TIMEOUT));
+        ASSERT_NO_THROW(listener.start());
+        ASSERT_NO_THROW(startRequest(request));
+        ASSERT_NO_THROW(runIOService());
+        ASSERT_EQ(1, clients_.size());
+        TestHttpClientPtr client = *clients_.begin();
+        ASSERT_TRUE(client);
+
+        // Build the reference response.
+        std::ostringstream expected_response;
+        expected_response
+            << "HTTP/" << expected_version.major_ << "." << expected_version.minor_
+            << " 408 Request Timeout\r\n"
+            "Content-Length: 44\r\n"
+            "Content-Type: application/json\r\n"
+            "Date: Tue, 19 Dec 2016 18:53:35 GMT\r\n"
+            "\r\n"
+            "{ \"result\": 408, \"text\": \"Request Timeout\" }";
+
+        // The server should wait for the missing part of the request for 1 second.
+        // The missing part never arrives so the server should respond with the
+        // HTTP Request Timeout status.
+        EXPECT_EQ(expected_response.str(), client->getResponse());
+    }
+
+    /// @brief Tests various cases when unexpected data is passed to the
+    /// socket write handler.
+    ///
+    /// This test uses the custom listener and the test specific derivations of
+    /// the @c HttpConnection class to enforce injection of the unexpected
+    /// data to the socket write callback. The two example applications of
+    /// this test are:
+    /// - injecting greater length value than the output buffer size,
+    /// - replacing the transaction with another transaction.
+    ///
+    /// It is expected that the socket write callback deals gracefully with
+    /// those situations.
+    ///
+    /// @tparam HttpConnectionType Test specific derivation of the
+    /// @c HttpConnection class.
+    template<typename HttpConnectionType>
+    void testWriteBufferIssues() {
+        // The HTTP/1.1 requests are by default persistent.
+        std::string request = "POST /foo/bar HTTP/1.1\r\n"
+            "Content-Type: application/json\r\n"
+            "Content-Length: 3\r\n\r\n"
+            "{ }";
+
+        // Use custom listener and the specialized connection object. 
+        HttpListenerCustom<HttpConnectionType>
+            listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT,
+                     factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT),
+                     HttpListener::IdleTimeout(IDLE_TIMEOUT));
+
+        ASSERT_NO_THROW(listener.start());
+
+        // Send the request.
+        ASSERT_NO_THROW(startRequest(request));
+
+        // Injecting unexpected data should not result in an exception.
+        ASSERT_NO_THROW(runIOService());
+
+        ASSERT_EQ(1, clients_.size());
+        TestHttpClientPtr client = *clients_.begin();
+        ASSERT_TRUE(client);
+        EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse());
+    }
+
     /// @brief IO service used in the tests.
     IOService io_service_;
 
@@ -861,36 +1142,42 @@ TEST_F(HttpListenerTest, addressInUse) {
 }
 
 // This test verifies that HTTP Request Timeout status is returned as
-// expected.
-TEST_F(HttpListenerTest, requestTimeout) {
+// expected when the read part of the request contains the HTTP
+// version number. The timeout response should contain the same
+// HTTP version number as the partial request.
+TEST_F(HttpListenerTest, requestTimeoutHttpVersionFound) {
     // The part of the request specified here is correct but it is not
     // a complete request.
     const std::string request = "POST /foo/bar HTTP/1.1\r\n"
         "Content-Type: application/json\r\n"
         "Content-Length:";
 
-    // Open the listener with the Request Timeout of 1 sec and post the
-    // partial request.
-    HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT,
-                          factory_, HttpListener::RequestTimeout(1000),
-                          HttpListener::IdleTimeout(IDLE_TIMEOUT));
-    ASSERT_NO_THROW(listener.start());
-    ASSERT_NO_THROW(startRequest(request));
-    ASSERT_NO_THROW(runIOService());
-    ASSERT_EQ(1, clients_.size());
-    TestHttpClientPtr client = *clients_.begin();
-    ASSERT_TRUE(client);
+    testRequestTimeout(request, HttpVersion::HTTP_11());
+}
 
-    // The server should wait for the missing part of the request for 1 second.
-    // The missing part never arrives so the server should respond with the
-    // HTTP Request Timeout status.
-    EXPECT_EQ("HTTP/1.1 408 Request Timeout\r\n"
-              "Content-Length: 44\r\n"
-              "Content-Type: application/json\r\n"
-              "Date: Tue, 19 Dec 2016 18:53:35 GMT\r\n"
-              "\r\n"
-              "{ \"result\": 408, \"text\": \"Request Timeout\" }",
-              client->getResponse());
+// This test verifies that HTTP Request Timeout status is returned as
+// expected when the read part of the request does not contain
+// the HTTP version number. The timeout response should by default
+// contain HTTP/1.0 version number.
+TEST_F(HttpListenerTest, requestTimeoutHttpVersionNotFound) {
+    // The part of the request specified here is correct but it is not
+    // a complete request.
+    const std::string request = "POST /foo/bar HTTP";
+
+    testRequestTimeout(request, HttpVersion::HTTP_10());
+}
+
+// This test verifies that injecting length value greater than the
+// output buffer length to the socket write callback does not cause
+// an exception.
+TEST_F(HttpListenerTest, tooLongWriteBuffer) {
+    testWriteBufferIssues<HttpConnectionLongWriteBuffer>();
+}
+
+// This test verifies that changing the transaction before calling
+// the socket write callback does not cause an exception.
+TEST_F(HttpListenerTest, transactionChangeDuringWrite) {
+    testWriteBufferIssues<HttpConnectionTransactionChange>();
 }
 
 /// @brief Test fixture class for testing HTTP client.