From: Marcin Siodelski Date: Thu, 6 Jun 2019 09:48:56 +0000 (+0200) Subject: [#491,!363] Updated unit tests for HTTP listener using transactions. X-Git-Tag: Kea-1.6.0-beta2~285 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d409b8c2e014ca42da531215acb6226f7eb7df53;p=thirdparty%2Fkea.git [#491,!363] Updated unit tests for HTTP listener using transactions. --- diff --git a/src/lib/http/tests/server_client_unittests.cc b/src/lib/http/tests/server_client_unittests.cc index ecd1928e2f..1cc1354500 100644 --- a/src/lib/http/tests/server_client_unittests.cc +++ b/src/lib/http/tests/server_client_unittests.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -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 +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 +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 + (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 + 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 + 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(); +} + +// This test verifies that changing the transaction before calling +// the socket write callback does not cause an exception. +TEST_F(HttpListenerTest, transactionChangeDuringWrite) { + testWriteBufferIssues(); } /// @brief Test fixture class for testing HTTP client.