From: Marcin Siodelski Date: Wed, 1 Feb 2017 15:12:16 +0000 (+0100) Subject: [5107] Implemented CtrlAgentResponseCreator and other classes. X-Git-Tag: trac5137_base~8^2~9 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b1c20ec763e3e18ede5d643d022fcc14adba2c45;p=thirdparty%2Fkea.git [5107] Implemented CtrlAgentResponseCreator and other classes. --- diff --git a/src/bin/agent/Makefile.am b/src/bin/agent/Makefile.am index f5f11e194e..de778fe1d2 100644 --- a/src/bin/agent/Makefile.am +++ b/src/bin/agent/Makefile.am @@ -44,10 +44,13 @@ noinst_LTLIBRARIES = libagent.la libagent_la_SOURCES = ctrl_agent_cfg_mgr.cc ctrl_agent_cfg_mgr.h libagent_la_SOURCES += ctrl_agent_controller.cc ctrl_agent_controller.h +libagent_la_SOURCES += ctrl_agent_command_mgr.cc ctrl_agent_command_mgr.h libagent_la_SOURCES += ctrl_agent_log.cc ctrl_agent_log.h libagent_la_SOURCES += ctrl_agent_process.cc ctrl_agent_process.h libagent_la_SOURCES += agent_parser.cc agent_parser.h libagent_la_SOURCES += parser_context.cc parser_context.h parser_context_decl.h +libagent_la_SOURCES += ctrl_agent_response_creator.cc ctrl_agent_response_creator.h +libagent_la_SOURCES += ctrl_agent_response_creator_factory.h libagent_la_SOURCES += agent_lexer.ll nodist_libagent_la_SOURCES = ctrl_agent_messages.h ctrl_agent_messages.cc @@ -66,6 +69,7 @@ kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la +kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/http/libkea-http.la kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la kea_ctrl_agent_LDADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/bin/agent/ctrl_agent_command_mgr.cc b/src/bin/agent/ctrl_agent_command_mgr.cc new file mode 100644 index 0000000000..8c89fccf02 --- /dev/null +++ b/src/bin/agent/ctrl_agent_command_mgr.cc @@ -0,0 +1,37 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include + +using namespace isc::data; + +namespace isc { +namespace agent { + +CtrlAgentCommandMgr& +CtrlAgentCommandMgr::instance() { + static CtrlAgentCommandMgr command_mgr; + return (command_mgr); +} + +CtrlAgentCommandMgr::CtrlAgentCommandMgr() + : HookedCommandMgr() { +} + +ConstElementPtr +CtrlAgentCommandMgr::handleCommand(const std::string& cmd_name, + const isc::data::ConstElementPtr& params) { + ElementPtr answer_list = Element::createList(); + answer_list->add(boost::const_pointer_cast< + Element>(HookedCommandMgr::handleCommand(cmd_name, params))); + return (answer_list); +} + + +} // end of namespace isc::agent +} // end of namespace isc diff --git a/src/bin/agent/ctrl_agent_command_mgr.h b/src/bin/agent/ctrl_agent_command_mgr.h new file mode 100644 index 0000000000..573c575ddc --- /dev/null +++ b/src/bin/agent/ctrl_agent_command_mgr.h @@ -0,0 +1,69 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef CTRL_AGENT_COMMAND_MGR_H +#define CTRL_AGENT_COMMAND_MGR_H + +#include +#include +#include + +namespace isc { +namespace agent { + +/// @brief Command Manager for Control Agent. +/// +/// This is an implementation of the Command Manager within Control Agent. +/// In addition to the standard capabilities of the @ref HookedCommandMgr +/// it is also intended to forward commands to the respective Kea servers +/// when the command is not supported directly by the Control Agent. +/// +/// @todo This Command Manager doesn't yet support forwarding commands. +/// +/// The @ref CtrlAgentCommandMgr is implemented as a singleton. The commands +/// are registered using @c CtrlAgentCommandMgr::instance().registerCommand(). +/// The @ref CtrlAgentResponseCreator uses the sole instance of the Command +/// Manager to handle incoming commands. +class CtrlAgentCommandMgr : public config::HookedCommandMgr, + public boost::noncopyable { +public: + + /// @brief Returns sole instance of the Command Manager. + static CtrlAgentCommandMgr& instance(); + +private: + + /// @brief Handles the command having a given name and arguments. + /// + /// This method extends the base implementation with the ability to forward + /// commands to Kea servers if the Control Agent failed to handle it itself. + /// + /// @todo Currently this method only wraps an answer within a list Element. + /// This will be later used to include multiple answers within this list. + /// For now it is just a single answer from the Control Agent. + /// + /// @param cmd_name Command name. + /// @param params Command arguments. + /// + /// @return Pointer to the const data element representing response + /// to a command. + virtual isc::data::ConstElementPtr + handleCommand(const std::string& cmd_name, + const isc::data::ConstElementPtr& params); + + + /// @brief Private constructor. + /// + /// The instance should be created using @ref CtrlAgentCommandMgr::instance, + /// thus the constructor is private. + CtrlAgentCommandMgr(); + +}; + +} // end of namespace isc::agent +} // end of namespace isc + +#endif diff --git a/src/bin/agent/ctrl_agent_process.cc b/src/bin/agent/ctrl_agent_process.cc index 51e2377f2b..4f9b0c1764 100644 --- a/src/bin/agent/ctrl_agent_process.cc +++ b/src/bin/agent/ctrl_agent_process.cc @@ -6,12 +6,26 @@ #include #include +#include #include +#include #include +#include #include +using namespace isc::asiolink; +using namespace isc::http; using namespace isc::process; +// Temporarily hardcoded configuration. +namespace { + +const IOAddress SERVER_ADDRESS("127.0.0.1"); +const unsigned short SERVER_PORT = 8081; +const long REQUEST_TIMEOUT = 10000; + +} + namespace isc { namespace agent { @@ -32,6 +46,12 @@ CtrlAgentProcess::run() { LOG_INFO(agent_logger, CTRL_AGENT_STARTED).arg(VERSION); try { + + HttpResponseCreatorFactoryPtr rcf(new CtrlAgentResponseCreatorFactory()); + HttpListener http_listener(*getIoService(), SERVER_ADDRESS, + SERVER_PORT, rcf, REQUEST_TIMEOUT); + http_listener.start(); + while (!shouldShutdown()) { getIoService()->run_one(); } diff --git a/src/bin/agent/ctrl_agent_response_creator.cc b/src/bin/agent/ctrl_agent_response_creator.cc new file mode 100644 index 0000000000..5b11e069b5 --- /dev/null +++ b/src/bin/agent/ctrl_agent_response_creator.cc @@ -0,0 +1,85 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include +#include + +using namespace isc::data; +using namespace isc::http; + +namespace isc { +namespace agent { + +HttpRequestPtr +CtrlAgentResponseCreator::createNewHttpRequest() const { + return (HttpRequestPtr(new PostHttpRequestJson())); +} + +HttpResponsePtr +CtrlAgentResponseCreator:: +createStockHttpResponse(const ConstHttpRequestPtr& request, + const HttpStatusCode& status_code) const { + // The request hasn't been finalized so the request object + // doesn't contain any information about the HTTP version number + // used. But, the context should have this data (assuming the + // HTTP version is parsed ok). + HttpVersion http_version(request->context()->http_version_major_, + request->context()->http_version_minor_); + // We only accept HTTP version 1.0 or 1.1. If other version number is found + // we fall back to HTTP/1.0. + if ((http_version < HttpVersion(1, 0)) || (HttpVersion(1, 1) < http_version)) { + http_version.major_ = 1; + http_version.minor_ = 0; + } + // This will generate the response holding JSON content. + HttpResponsePtr response(new HttpResponseJson(http_version, status_code)); + return (response); +} + +HttpResponsePtr +CtrlAgentResponseCreator:: +createDynamicHttpResponse(const ConstHttpRequestPtr& request) { + // The request is always non-null, because this is verified by the + // createHttpResponse method. Let's try to convert it to the + // ConstPostHttpRequestJson type as this is the type generated by the + // createNewHttpRequest. If the conversion result is null it means that + // the caller did not use createNewHttpRequest method to create this + // instance. This is considered an error in the server logic. + ConstPostHttpRequestJsonPtr request_json = boost::dynamic_pointer_cast< + const PostHttpRequestJson>(request); + if (!request_json) { + // Notify the client that we have a problem with our server. + return (createStockHttpResponse(request, HttpStatusCode::INTERNAL_SERVER_ERROR)); + } + + // We have already checked that the request is finalized so the call + // to getBodyAsJson must not trigger an exception. + ConstElementPtr command = request_json->getBodyAsJson(); + + // Process command doesn't generate exceptions but can possibly return + // null response, if the handler is not implemented properly. This is + // again an internal server issue. + ConstElementPtr response = CtrlAgentCommandMgr::instance().processCommand(command); + if (!response) { + // Notify the client that we have a problem with our server. + return (createStockHttpResponse(request, HttpStatusCode::INTERNAL_SERVER_ERROR)); + } + // The response is ok, so let's create new HTTP response with the status OK. + HttpResponseJsonPtr http_response = boost::dynamic_pointer_cast< + HttpResponseJson>(createStockHttpResponse(request, HttpStatusCode::OK)); + http_response->setBodyAsJson(response); + + return (http_response); +} + + + +} // end of namespace isc::agent +} // end of namespace isc diff --git a/src/bin/agent/ctrl_agent_response_creator.h b/src/bin/agent/ctrl_agent_response_creator.h new file mode 100644 index 0000000000..610b3fc385 --- /dev/null +++ b/src/bin/agent/ctrl_agent_response_creator.h @@ -0,0 +1,69 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef CTRL_AGENT_RESPONSE_CREATOR_H +#define CTRL_AGENT_RESPONSE_CREATOR_H + +#include +#include +#include + +namespace isc { +namespace agent { + +class CtrlAgentResponseCreator; + +/// @brief Pointer to the @ref CtrlAgentResponseCreator. +typedef boost::shared_ptr CtrlAgentResponseCreatorPtr; + +/// @brief Concrete implementation of the HTTP response crator used +/// by the Control Agent. +/// +/// See the documentation of the @ref HttpResponseCreator for the basic +/// information how HTTP response creators are utilized by the libkea-http +/// library to generate HTTP responses. +/// +/// This creator expects that received requests are encapsulated in the +/// @ref PostHttpRequestJson objects. The generated responses are +/// encapsulated in the HttpResponseJson objects. +/// +/// This class uses @ref CtrlAgentCommandMgr singleton to process commands +/// conveyed in the HTTP body. The JSON responses returned by the manager +/// are placed in the body of the generated HTTP responses. +class CtrlAgentResponseCreator : public http::HttpResponseCreator { +public: + + /// @brief Create a new request. + /// + /// This method creates a bare instance of the @ref PostHttpRequestJson. + /// + /// @return Pointer to the new instance of the @ref PostHttpRequestJson. + virtual http::HttpRequestPtr createNewHttpRequest() const; + + /// @brief Creates stock HTTP response. + /// + /// @param request Pointer to an object representing HTTP request. + /// @param status_code Status code of the response. + /// @return Pointer to an @ref HttpResponseJson object representing stock + /// HTTP response. + virtual http::HttpResponsePtr + createStockHttpResponse(const http::ConstHttpRequestPtr& request, + const http::HttpStatusCode& status_code) const; + +private: + + /// @brief Creates implementation specific HTTP response. + /// + /// @param request Pointer to an object representing HTTP request. + /// @return Pointer to an object representing HTTP response. + virtual http::HttpResponsePtr + createDynamicHttpResponse(const http::ConstHttpRequestPtr& request); +}; + +} // end of namespace isc::agent +} // end of namespace isc + +#endif diff --git a/src/bin/agent/ctrl_agent_response_creator_factory.h b/src/bin/agent/ctrl_agent_response_creator_factory.h new file mode 100644 index 0000000000..d6f6ba0568 --- /dev/null +++ b/src/bin/agent/ctrl_agent_response_creator_factory.h @@ -0,0 +1,55 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef CTRL_AGENT_RESPONSE_CREATOR_FACTORY_H +#define CTRL_AGENT_RESPONSE_CREATOR_FACTORY_H + +#include +#include + +namespace isc { +namespace agent { + +/// @brief HTTP response creator factory for Control Agent. +/// +/// See the documentation of the @ref HttpResponseCreatorFactory for +/// the details how the response factory object is used by the +/// @ref HttpListener. +/// +/// This class always returns the same instance of the +/// @ref CtrlAgentResponseCreator which @ref HttpListener and +/// @ref HttpConnection classes use to generate HTTP response messages +/// which comply with the formats required by the Control Agent. +class CtrlAgentResponseCreatorFactory : public http::HttpResponseCreatorFactory { +public: + + /// @brief Constructor. + /// + /// Creates sole instance of the @ref CtrlAgentResponseCreator object + /// returned by the @ref CtrlAgentResponseCreatorFactory::create. + CtrlAgentResponseCreatorFactory() + : sole_creator_(new CtrlAgentResponseCreator()) { + } + + /// @brief Returns an instance of the @ref CtrlAgentResponseCreator which + /// is used by HTTP server to generate responses to commands. + /// + /// @return Pointer to the @ref CtrlAgentResponseCreator object. + virtual http::HttpResponseCreatorPtr create() const { + return (sole_creator_); + } + +private: + + /// @brief Instance of the @ref CtrlAgentResponseCreator returned. + http::HttpResponseCreatorPtr sole_creator_; + +}; + +} // end of namespace isc::agent +} // end of namespace isc + +#endif diff --git a/src/bin/agent/tests/Makefile.am b/src/bin/agent/tests/Makefile.am index 64695e4d0f..626aaa4148 100644 --- a/src/bin/agent/tests/Makefile.am +++ b/src/bin/agent/tests/Makefile.am @@ -44,9 +44,12 @@ TESTS += ctrl_agent_unittest ctrl_agent_unittest_SOURCES = ctrl_agent_cfg_mgr_unittest.cc ctrl_agent_unittest_SOURCES += ctrl_agent_controller_unittest.cc +ctrl_agent_unittests_SOURCES += ctrl_agent_command_mgr_unittests.cc ctrl_agent_unittest_SOURCES += parser_unittest.cc ctrl_agent_unittest_SOURCES += ctrl_agent_process_unittest.cc +ctrl_agent_unittests_SOURCES += ctrl_agent_response_creator_factory_unittests.cc ctrl_agent_unittest_SOURCES += ctrl_agent_unittest.cc +ctrl_agent_unittests_SOURCES += ctrl_agent_response_creator_unittests.cc ctrl_agent_unittest_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) ctrl_agent_unittest_LDFLAGS = $(AM_LDFLAGS) $(CRYPTO_LDFLAGS) $(GTEST_LDFLAGS) @@ -61,6 +64,7 @@ ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns. ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la +ctrl_agent_unittests_LDADD += $(top_builddir)/src/lib/http/libkea-http.la ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la ctrl_agent_unittest_LDADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/bin/agent/tests/ctrl_agent_command_mgr_unittests.cc b/src/bin/agent/tests/ctrl_agent_command_mgr_unittests.cc new file mode 100644 index 0000000000..df0db24eba --- /dev/null +++ b/src/bin/agent/tests/ctrl_agent_command_mgr_unittests.cc @@ -0,0 +1,38 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include + +using namespace isc::agent; + +namespace { + +/// @brief Test fixture class for @ref CtrlAgentCommandMgr. +/// +/// @todo Add tests for various commands, including the cases when the +/// commands are forwarded to other servers via unix sockets. +/// Meanwhile, this is just a placeholder for the tests. +class CtrlAgentCommandMgrTest : public ::testing::Test { +public: + + /// @brief Constructor. + /// + /// Deregisters all commands except 'list-commands'. + CtrlAgentCommandMgrTest() { + CtrlAgentCommandMgr::instance().deregisterAll(); + } + + /// @brief Destructor. + /// + /// Deregisters all commands except 'list-commands'. + virtual ~CtrlAgentCommandMgrTest() { + CtrlAgentCommandMgr::instance().deregisterAll(); + } +}; + +} diff --git a/src/bin/agent/tests/ctrl_agent_response_creator_factory_unittests.cc b/src/bin/agent/tests/ctrl_agent_response_creator_factory_unittests.cc new file mode 100644 index 0000000000..db975fe223 --- /dev/null +++ b/src/bin/agent/tests/ctrl_agent_response_creator_factory_unittests.cc @@ -0,0 +1,39 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include + +using namespace isc::agent; + +namespace { + +// This test verifies that create() method always returns the same +// instance of the CtrlAgentResponseCreator object. +TEST(CtrlAgentResponseCreatorFactory, create) { + CtrlAgentResponseCreatorFactory factory; + + // Invoke twice. + CtrlAgentResponseCreatorPtr response1; + CtrlAgentResponseCreatorPtr response2; + ASSERT_NO_THROW(response1 = boost::dynamic_pointer_cast< + CtrlAgentResponseCreator>(factory.create())); + ASSERT_NO_THROW(response2 = boost::dynamic_pointer_cast< + CtrlAgentResponseCreator>(factory.create())); + + // It must always return non-null object. + ASSERT_TRUE(response1); + ASSERT_TRUE(response2); + + // And it must always return the same object. + EXPECT_TRUE(response1 == response2); + +} + +} diff --git a/src/bin/agent/tests/ctrl_agent_response_creator_unittests.cc b/src/bin/agent/tests/ctrl_agent_response_creator_unittests.cc new file mode 100644 index 0000000000..8fa6c04b88 --- /dev/null +++ b/src/bin/agent/tests/ctrl_agent_response_creator_unittests.cc @@ -0,0 +1,209 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace isc::agent; +using namespace isc::config; +using namespace isc::data; +using namespace isc::http; + +namespace { + +/// @brief Test fixture class for @ref CtrlAgentResponseCreator. +class CtrlAgentResponseCreatorTest : public ::testing::Test { +public: + + /// @brief Constructor. + /// + /// Creates instance of the response creator and uses this instance to + /// create "empty" request. It also removes registered commands from the + /// command manager. + CtrlAgentResponseCreatorTest() + : response_creator_(), + request_(response_creator_.createNewHttpRequest()) { + // Deregisters commands. + CtrlAgentCommandMgr::instance().deregisterAll(); + CtrlAgentCommandMgr::instance(). + registerCommand("foo", boost::bind(&CtrlAgentResponseCreatorTest:: + fooCommandHandler, + this, _1, _2)); + + // Make sure that the request has been initialized properly. + if (!request_) { + ADD_FAILURE() << "CtrlAgentResponseCreator::createNewHttpRequest" + " returns NULL!"; + } + } + + /// @brief Destructor. + /// + /// Removes registered commands from the command manager. + virtual ~CtrlAgentResponseCreatorTest() { + CtrlAgentCommandMgr::instance().deregisterAll(); + } + + /// @brief Fills request context with required data to create new request. + /// + /// @param request Request which context should be configured. + void setBasicContext(const HttpRequestPtr& request) { + request->context()->method_ = "POST"; + request->context()->http_version_major_ = 1; + request->context()->http_version_minor_ = 1; + request->context()->uri_ = "/foo"; + + // Content-Type + HttpHeaderContext content_type; + content_type.name_ = "Content-Type"; + content_type.value_ = "application/json"; + request->context()->headers_.push_back(content_type); + + // Content-Length + HttpHeaderContext content_length; + content_length.name_ = "Content-Length"; + content_length.value_ = "0"; + request->context()->headers_.push_back(content_length); + } + + /// @brief Test creation of stock response. + /// + /// @param status_code Status code to be included in the response. + /// @param must_contain Text that must be present in the textual + /// representation of the generated response. + void testStockResponse(const HttpStatusCode& status_code, + const std::string& must_contain) { + HttpResponsePtr response; + ASSERT_NO_THROW( + response = response_creator_.createStockHttpResponse(request_, + status_code) + ); + ASSERT_TRUE(response); + HttpResponseJsonPtr response_json = boost::dynamic_pointer_cast< + HttpResponseJson>(response); + ASSERT_TRUE(response_json); + // Make sure the response contains the string specified as argument. + EXPECT_TRUE(response_json->toString().find(must_contain) != std::string::npos); + + } + + /// @brief Handler for the 'foo' test command. + /// + /// @param command_name Command name, i.e. 'foo'. + /// @param command_arguments Command arguments (empty). + /// + /// @return Returns response with a single string "bar". + ConstElementPtr fooCommandHandler(const std::string& command_name, + const ConstElementPtr& command_arguments) { + ElementPtr arguments = Element::createList(); + arguments->add(Element::create("bar")); + return (createAnswer(CONTROL_RESULT_SUCCESS, arguments)); + } + + /// @brief Instance of the response crator. + CtrlAgentResponseCreator response_creator_; + + /// @brief Instance of the "empty" request. + /// + /// The context belonging to this request may be modified by the unit + /// tests to verify various scenarios of response creation. + HttpRequestPtr request_; + +}; + +// This test verifies that the created "empty" reuqest has valid type. +TEST_F(CtrlAgentResponseCreatorTest, createNewHttpRequest) { + // The request must be of PostHttpRequestJson type. + PostHttpRequestJsonPtr request_json = boost::dynamic_pointer_cast< + PostHttpRequestJson>(request_); + ASSERT_TRUE(request_json); +} + +// Test that HTTP version of stock response is set to 1.0 if the request +// context doesn't specify any version. +TEST_F(CtrlAgentResponseCreatorTest, createStockHttpResponseNoVersion) { + testStockResponse(HttpStatusCode::BAD_REQUEST, "HTTP/1.0 400 Bad Request"); +} + +// Test that HTTP version of stock response is set to 1.0 if the request +// version is higher than 1.1. +TEST_F(CtrlAgentResponseCreatorTest, createStockHttpResponseHighVersion) { + request_->context()->http_version_major_ = 2; + testStockResponse(HttpStatusCode::REQUEST_TIMEOUT, + "HTTP/1.0 408 Request Timeout"); +} + +// Test that the server responsds with version 1.1 if request version is 1.1. +TEST_F(CtrlAgentResponseCreatorTest, createStockHttpResponseCorrectVersion) { + request_->context()->http_version_major_ = 1; + request_->context()->http_version_minor_ = 1; + testStockResponse(HttpStatusCode::NO_CONTENT, "HTTP/1.1 204 No Content"); +} + +// Test successful server response when the client specifies valid command. +TEST_F(CtrlAgentResponseCreatorTest, createDynamicHttpResponse) { + setBasicContext(request_); + + // Body: "foo" command has been registered in the test fixture constructor. + request_->context()->body_ = "{ \"command\": \"foo\" }"; + + // All requests must be finalized before they can be processed. + ASSERT_NO_THROW(request_->finalize()); + + // Create response from the request. + HttpResponsePtr response; + ASSERT_NO_THROW(response = response_creator_.createHttpResponse(request_)); + ASSERT_TRUE(response); + + // Response must be convertible to HttpResponseJsonPtr. + HttpResponseJsonPtr response_json = boost::dynamic_pointer_cast< + HttpResponseJson>(response); + ASSERT_TRUE(response_json); + + // Response must be successful. + EXPECT_TRUE(response_json->toString().find("HTTP/1.1 200 OK") != + std::string::npos); + // Response must contain JSON body with "result" of 0. + EXPECT_TRUE(response_json->toString().find("\"result\": 0") != + std::string::npos); +} + +// This test verifies that Internal Server Error is returned when invalid C++ +// request type is used. This is considered an error in the server logic. +TEST_F(CtrlAgentResponseCreatorTest, createDynamicHttpResponseInvalidType) { + PostHttpRequestPtr request(new PostHttpRequest()); + setBasicContext(request); + + // Body: "list-commands" is natively supported by the command manager. + request->context()->body_ = "{ \"command\": \"list-commands\" }"; + + // All requests must be finalized before they can be processed. + ASSERT_NO_THROW(request->finalize()); + + HttpResponsePtr response; + ASSERT_NO_THROW(response = response_creator_.createHttpResponse(request)); + ASSERT_TRUE(response); + + // Response must be convertible to HttpResponseJsonPtr. + HttpResponseJsonPtr response_json = boost::dynamic_pointer_cast< + HttpResponseJson>(response); + ASSERT_TRUE(response_json); + + // Response must contain Internal Server Error status code. + EXPECT_TRUE(response_json->toString().find("HTTP/1.1 500 Internal Server Error") != + std::string::npos); + +} + +} diff --git a/src/lib/config/hooked_command_mgr.h b/src/lib/config/hooked_command_mgr.h index 044d3ebec0..ab7cdebd04 100644 --- a/src/lib/config/hooked_command_mgr.h +++ b/src/lib/config/hooked_command_mgr.h @@ -36,21 +36,16 @@ protected: return (callout_handle_); } -private: - /// @brief Handles the command having a given name and arguments. /// /// This method checks if the hook library is installed which implements /// callouts for the 'control_command_receive' hook point, and calls them /// if they exist. If the hook library supports the given command it creates /// a response and returns it in the 'response' argument of the - /// @ref ConstElementPtr type. If the callout also sets the 'skip' status, - /// the response created by the callout is returned. Otherwise, the + /// @ref isc::data::ConstElementPtr type. If the callout also sets the 'skip' + /// status, the response created by the callout is returned. Otherwise, the /// @ref BaseCommandMgr::handleCommand is called. /// - /// This method is private because it is its final implementation which - /// should not be overridden in the derived classes. - /// /// @param cmd_name Command name. /// @param params Command arguments. /// diff --git a/src/lib/http/connection.cc b/src/lib/http/connection.cc index 0febee328d..6d7d28024a 100644 --- a/src/lib/http/connection.cc +++ b/src/lib/http/connection.cc @@ -8,7 +8,6 @@ #include #include #include -#include using namespace isc::asiolink; @@ -96,6 +95,8 @@ HttpConnection::doWrite() { socket_.asyncSend(output_buf_.data(), output_buf_.length(), socket_write_callback_); + } else { + stopThisConnection(); } } catch (const std::exception& ex) { stopThisConnection(); @@ -156,6 +157,7 @@ HttpConnection::socketWriteCallback(boost::system::error_code ec, } else { output_buf_.clear(); + stopThisConnection(); } } diff --git a/src/lib/http/post_request.h b/src/lib/http/post_request.h index 04f5564a65..b1b0134ca4 100644 --- a/src/lib/http/post_request.h +++ b/src/lib/http/post_request.h @@ -8,10 +8,18 @@ #define HTTP_POST_REQUEST_H #include +#include namespace isc { namespace http { +class PostHttpRequest; + +/// @brief Pointer to @ref PostHttpRequest. +typedef boost::shared_ptr PostHttpRequestPtr; +/// @brief Pointer to const @ref PostHttpRequest. +typedef boost::shared_ptr ConstPostHttpRequestPtr; + /// @brief Represents HTTP POST request. /// /// Instructs the parent class to require: @@ -25,6 +33,7 @@ public: PostHttpRequest(); }; + } // namespace http } // namespace isc diff --git a/src/lib/http/post_request_json.cc b/src/lib/http/post_request_json.cc index c12d8be809..c871dbbca0 100644 --- a/src/lib/http/post_request_json.cc +++ b/src/lib/http/post_request_json.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -34,13 +34,13 @@ PostHttpRequestJson::reset() { } ConstElementPtr -PostHttpRequestJson::getBodyAsJson() { +PostHttpRequestJson::getBodyAsJson() const { checkFinalized(); return (json_); } ConstElementPtr -PostHttpRequestJson::getJsonElement(const std::string& element_name) { +PostHttpRequestJson::getJsonElement(const std::string& element_name) const { try { ConstElementPtr body = getBodyAsJson(); if (body) { diff --git a/src/lib/http/post_request_json.h b/src/lib/http/post_request_json.h index a3f94ff879..a51b484cf1 100644 --- a/src/lib/http/post_request_json.h +++ b/src/lib/http/post_request_json.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace isc { @@ -22,6 +23,13 @@ public: HttpRequestError(file, line, what) { }; }; +class PostHttpRequestJson; + +/// @brief Pointer to @ref PostHttpRequestJson. +typedef boost::shared_ptr PostHttpRequestJsonPtr; +/// @brief Pointer to const @ref PostHttpRequestJson. +typedef boost::shared_ptr ConstPostHttpRequestJsonPtr; + /// @brief Represents HTTP POST request with JSON body. /// /// In addition to the requirements specified by the @ref PostHttpRequest @@ -47,7 +55,7 @@ public: /// /// @return Pointer to the root element of the JSON structure. /// @throw HttpRequestJsonError if an error occurred. - data::ConstElementPtr getBodyAsJson(); + data::ConstElementPtr getBodyAsJson() const; /// @brief Retrieves a single JSON element. /// @@ -58,7 +66,7 @@ public: /// @return Pointer to the specified element or NULL if such element /// doesn't exist. /// @throw HttpRequestJsonError if an error occurred. - data::ConstElementPtr getJsonElement(const std::string& element_name); + data::ConstElementPtr getJsonElement(const std::string& element_name) const; protected: diff --git a/src/lib/http/response.cc b/src/lib/http/response.cc index 0896ede8d2..7062795c22 100644 --- a/src/lib/http/response.cc +++ b/src/lib/http/response.cc @@ -116,10 +116,8 @@ HttpResponse::toString() const { // Update or add "Date" header. addHeaderInternal("Date", getDateHeaderValue(), headers); - // Add "Content-Length" if body present. - if (!body_.empty()) { - addHeaderInternal("Content-Length", body_.length(), headers); - } + // Always add "Content-Length", perhaps equal to 0. + addHeaderInternal("Content-Length", body_.length(), headers); // Include all headers. for (auto header = headers.cbegin(); header != headers.cend(); diff --git a/src/lib/http/tests/listener_unittests.cc b/src/lib/http/tests/listener_unittests.cc index f224f6ead1..b14df77b92 100644 --- a/src/lib/http/tests/listener_unittests.cc +++ b/src/lib/http/tests/listener_unittests.cc @@ -315,6 +315,7 @@ TEST_F(HttpListenerTest, listen) { HttpClientPtr client = *clients_.begin(); ASSERT_TRUE(client); EXPECT_EQ("HTTP/1.1 200 OK\r\n" + "Content-Length: 0\r\n" "Content-Type: application/json\r\n" "Date: Tue, 19 Dec 2016 18:53:35 GMT\r\n" "\r\n", diff --git a/src/lib/http/tests/response_creator_unittests.cc b/src/lib/http/tests/response_creator_unittests.cc index 6862f58908..d6a462c12d 100644 --- a/src/lib/http/tests/response_creator_unittests.cc +++ b/src/lib/http/tests/response_creator_unittests.cc @@ -39,7 +39,7 @@ public: private: - /// @brief Creates HTTP response.. + /// @brief Creates HTTP response. /// /// @param request Pointer to the HTTP request. /// @return Pointer to the generated HTTP response. @@ -115,6 +115,7 @@ TEST(HttpResponseCreatorTest, goodRequest) { ASSERT_TRUE(response); EXPECT_EQ("HTTP/1.0 200 OK\r\n" + "Content-Length: 0\r\n" "Content-Type: application/json\r\n" "Date: Tue, 19 Dec 2016 18:53:35 GMT\r\n\r\n", response->toString()); diff --git a/src/lib/http/tests/response_json_unittests.cc b/src/lib/http/tests/response_json_unittests.cc index 8df063eb21..9b005e2c79 100644 --- a/src/lib/http/tests/response_json_unittests.cc +++ b/src/lib/http/tests/response_json_unittests.cc @@ -77,6 +77,8 @@ public: HttpResponse::isServerError(status_code)) { response_string << "Content-Length: " << status_message_json.str().size() << "\r\n"; + } else { + response_string << "Content-Length: 0\r\n"; } // Content-Type and Date are automatically included. diff --git a/src/lib/http/tests/response_unittests.cc b/src/lib/http/tests/response_unittests.cc index 25986564d2..f6e510bf60 100644 --- a/src/lib/http/tests/response_unittests.cc +++ b/src/lib/http/tests/response_unittests.cc @@ -40,6 +40,7 @@ public: std::ostringstream response_string; response_string << "HTTP/1.0 " << static_cast(status_code) << " " << status_message << "\r\n" + << "Content-Length: 0\r\n" << "Content-Type: text/html\r\n" << "Date: " << response.getDateHeaderValue() << "\r\n\r\n";