diff --git a/ChangeLog b/ChangeLog index 981960b3a2..bbec06a9f0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,35 @@ +1340. [func] marcin + Added support for "dhcp-enable" and "dhcp-disable" commands in + the DHCPv4 and DHCPv6 server. + (Trac #5442, git 36dc68ff7aa8b3cfd265c4f982d10248590039bd) + +1339. [doc] marcin + Updated User's Guide describing how to selectively disable + legal logging for a subnet. + (Trac #5407, git 469080abd711f8e88a5133f76f4ab31a5549a858) + +1338. [func] marcin + Persistent HTTP/1.1 connections and HTTP/1.0 keep-alive + are supported by RESTful API. + (Trac #5448, git 05018f7cc0662d6956b9b7648646e0c17da948ba) + +1337. [doc] marcin + Added placeholder section for the libdhcp_ha hooks library. + (Trac #5447, git d939b5b8bc4befb24daf863f2408d97493e4bfbf) + +1336. [bug] marcin + DHCPv6 server always sends prefixes with the lifetime of 0 for + the prefix leases that should no longer be used, even if those + prefixes are not included in the Renew/Rebind. + (Trac #5403, git 91bb0855ff7ef86ff72b5a946ae716798d7bebc1) + +1335. [bug] marcin + Fixed a bug which prevented inserting multiple host reservations + where IPv4 address was unspecified or when selected subnet + identifier was not specified. This change affects both Postgres + and MySQL backend. + (Trac #5416, git 03fab8f7d5c2e8a5ea735b11ff75652aa31d791d) + Kea 1.3.0 released on October 27, 2017 1334. [bug] marcin diff --git a/doc/guide/ctrl-channel.xml b/doc/guide/ctrl-channel.xml index 04907a0573..2b37e9660c 100644 --- a/doc/guide/ctrl-channel.xml +++ b/doc/guide/ctrl-channel.xml @@ -557,6 +557,42 @@ $ curl -X POST -H "Content-Type: application/json" -d '{ "command": "config-get" +
+ dhcp-disable + + The dhcp-disable command globally disables the DHCP + service. The server continues to operate, but it drops all received DHCP + messages. This command is useful when the server's maintenance requires that + the server temporarily stops allocating new leases and renew existing leases. + It is also useful in failover like configurations during a synchronization of + the lease databases at startup or recovery after a failure. The optional parameter + max-period specifies the time in seconds after which the + DHCP service should be automatically re-enabled if the + dhcp-enable command is not sent before this time elapses. + + +{ + "command": "dhcp-disable", + "arguments": { + "max-period": 20 + } +} + +
+ +
+ dhcp-enable + + The dhcp-enable command globally enables the DHCP + service. + + +{ + "command": "dhcp-enable" +} + +
+
version-get diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index c285396a5c..b17e2fd185 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -4556,6 +4556,8 @@ autogenerated IDs are not stable across configuration changes. config-set config-test config-write + dhcp-disable + dhcp-enable leases-reclaim list-commands shutdown @@ -4617,7 +4619,7 @@ autogenerated IDs are not stable across configuration changes.
-
+
User contexts in IPv4 Kea allows loading hook libraries that sometimes could benefit from diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index c323b76496..1384e2ac32 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -4558,6 +4558,8 @@ autogenerated IDs are not stable across configuration changes. config-set config-test config-write + dhcp-disable + dhcp-enable leases-reclaim list-commands shutdown @@ -4578,7 +4580,7 @@ autogenerated IDs are not stable across configuration changes.
-
+
User contexts in IPv6 Kea allows loading hook libraries that sometimes could benefit from diff --git a/doc/guide/hooks.xml b/doc/guide/hooks.xml index 87474d317f..be499490f7 100644 --- a/doc/guide/hooks.xml +++ b/doc/guide/hooks.xml @@ -258,6 +258,25 @@ or configuration parameters currently used by the server. + + High Availability + Support customers + Kea 1.4.0 + Minimizing a risk of DHCP service unavailability is achieved + by setting up multiple instances of the DHCP servers in a network. + Each server can serve selected group of clients in this network + (load balancing) or all clients, if it detects that its partner has + crashed or cannot be providing DHCP service for any other reason. + It is also possible to designate one server to serve all DHCP clients, + and leave another server as "standby". This server will activate its + DHCP function when it detects that its partner is not available. + Such cooperation between the DHCP servers requires that these + servers constantly communicate with each other to send updates about + allocated leases and to periodically test whether their partners are still + operational. The "libdhcp_ha" library provides such functionality for + Kea DHCP. + + @@ -687,6 +706,59 @@ Administrator deleted a lease for a device identified by: duid of 1a:1b:1c:1d:1e + + + If it is desired to restrict forensic logging to certain subnets, the + "legal-logging" boolean parameter can be specified within a user context of + these subnets. For example: + +"Dhcpv4" { + "subnet4": [ + { + "subnet": "192.0.2.0/24", + "pools": [ + { + "pool": "192.0.2.1 - 192.0.2.200" + } + ], + "user-context": { + "legal-logging": false + } + } + ] +} + + disables legal logging for the subnet "192.0.2.0/24". If this parameter + is not specified, it defaults to 'true', which enables legal logging for + the subnet. + + + + The following example demonstrates how to selectively disable legal logging + for an IPv6 subnet. + +"Dhcpv6": { + "subnet6": [ + { + "subnet": "2001:db8:1::/64", + "pools": [ + { + "pool": "2001:db8:1::1-2001:db8:1::ffff" + } + ], + "user-context": { + "legal-logging": false + } + } + ] +} + + + + + See and + to learn more about user contexts in Kea configuration. +
@@ -2529,8 +2601,19 @@ both the command and the response.
- - + + +
+ libdhcp_ha: High Availability + + This section will describe the libdhcp_ha hook library + being developed for the Kea 1.4.0 release. + +
+ + + +
User contexts diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index e1fd8dac61..2f2e157bba 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -26,6 +26,8 @@ namespace { const long REQUEST_TIMEOUT = 10000; +const long IDLE_TIMEOUT = 30000; + } namespace isc { @@ -151,10 +153,11 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set, // Create http listener. It will open up a TCP socket and be // prepared to accept incoming connection. - HttpListenerPtr http_listener(new HttpListener(*getIoService(), - server_address, - server_port, rcf, - REQUEST_TIMEOUT)); + HttpListenerPtr + http_listener(new HttpListener(*getIoService(), server_address, + server_port, rcf, + HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT))); // Instruct the http listener to actually open socket, install // callback and start listening. diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index afb0e2dbdb..6a82e07be5 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -19,6 +19,7 @@ #include #include #include +#include using namespace isc::data; using namespace isc::dhcp; @@ -374,6 +375,64 @@ ControlledDhcpv4Srv::commandConfigTestHandler(const string&, return (checkConfig(dhcp4)); } +ConstElementPtr +ControlledDhcpv4Srv::commandDhcpDisableHandler(const std::string&, + ConstElementPtr args) { + std::ostringstream message; + int64_t max_period = 0; + + // Parse arguments to see if the 'max-period' parameter has been specified. + if (args) { + // Arguments must be a map. + if (args->getType() != Element::map) { + message << "arguments for the 'dhcp-disable' command must be a map"; + + } else { + ConstElementPtr max_period_element = args->get("max-period"); + // max-period is optional. + if (max_period_element) { + // It must be an integer, if specified. + if (max_period_element->getType() != Element::integer) { + message << "'max-period' argument must be a number"; + + } else { + // It must be positive integer. + max_period = max_period_element->intValue(); + if (max_period <= 0) { + message << "'max-period' must be positive integer"; + } + + // The user specified that the DHCP service should resume not + // later than in max-period seconds. If the 'dhcp-enable' command + // is not sent, the DHCP service will resume automatically. + network_state_.delayedEnableAll(static_cast(max_period)); + } + } + } + } + + // No error occurred, so let's disable the service. + if (message.tellp() == 0) { + network_state_.disableService(); + + message << "DHCPv4 service disabled"; + if (max_period > 0) { + message << " for " << max_period << " seconds"; + } + // Success. + return (config::createAnswer(CONTROL_RESULT_SUCCESS, message.str())); + } + + // Failure. + return (config::createAnswer(CONTROL_RESULT_ERROR, message.str())); +} + +ConstElementPtr +ControlledDhcpv4Srv::commandDhcpEnableHandler(const std::string&, ConstElementPtr) { + network_state_.enableService(); + return (config::createAnswer(CONTROL_RESULT_SUCCESS, "DHCP service successfully enabled")); +} + ConstElementPtr ControlledDhcpv4Srv::commandVersionGetHandler(const string&, ConstElementPtr) { ElementPtr extended = Element::create(Dhcpv4Srv::getVersion(true)); @@ -455,6 +514,12 @@ ControlledDhcpv4Srv::processCommand(const string& command, } else if (command == "config-test") { return (srv->commandConfigTestHandler(command, args)); + } else if (command == "dhcp-disable") { + return (srv->commandDhcpDisableHandler(command, args)); + + } else if (command == "dhcp-enable") { + return (srv->commandDhcpEnableHandler(command, args)); + } else if (command == "version-get") { return (srv->commandVersionGetHandler(command, args)); @@ -619,6 +684,12 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) CommandMgr::instance().registerCommand("config-write", boost::bind(&ControlledDhcpv4Srv::commandConfigWriteHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("dhcp-enable", + boost::bind(&ControlledDhcpv4Srv::commandDhcpEnableHandler, this, _1, _2)); + + CommandMgr::instance().registerCommand("dhcp-disable", + boost::bind(&ControlledDhcpv4Srv::commandDhcpDisableHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("libreload", boost::bind(&ControlledDhcpv4Srv::commandLibReloadHandler, this, _1, _2)); @@ -675,6 +746,8 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() { CommandMgr::instance().deregisterCommand("leases-reclaim"); CommandMgr::instance().deregisterCommand("libreload"); CommandMgr::instance().deregisterCommand("config-set"); + CommandMgr::instance().deregisterCommand("dhcp-disable"); + CommandMgr::instance().deregisterCommand("dhcp-enable"); CommandMgr::instance().deregisterCommand("shutdown"); CommandMgr::instance().deregisterCommand("statistic-get"); CommandMgr::instance().deregisterCommand("statistic-get-all"); diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index d2fcf193f6..f9598041d1 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -223,12 +223,34 @@ private: commandConfigTestHandler(const std::string& command, isc::data::ConstElementPtr args); + /// @brief A handler for processing 'dhcp-disable' command. + /// + /// @param command command name (ignored). + /// @param args aguments for the command. It must be a map and + /// it may include optional 'max-period' parameter. + /// + /// @return result of the command. + isc::data::ConstElementPtr + commandDhcpDisableHandler(const std::string& command, + isc::data::ConstElementPtr args); + + /// @brief A handler for processing 'dhcp-enable' command. + /// + /// @param command command name (ignored) + /// @param args arguments for the command (ignored). + /// + /// @return result of the command. + isc::data::ConstElementPtr + commandDhcpEnableHandler(const std::string& command, + isc::data::ConstElementPtr args); + + /// @Brief handler for processing 'version-get' command /// /// This handler processes version-get command, which returns /// over the control channel the -v and -V command line arguments. /// @param command (parameter ignored) - /// @param args (parameter ignored) + /// @param args (parameter ignored) /// /// @return status of the command with the version in text and /// the extended version in arguments. @@ -241,7 +263,7 @@ private: /// This handler processes build-report command, which returns /// over the control channel the -W command line argument. /// @param command (parameter ignored) - /// @param args (parameter ignored) + /// @param args (parameter ignored) /// /// @return status of the command with the config report isc::data::ConstElementPtr diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 15b9e82723..05426d29e7 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -404,6 +404,12 @@ will not send a response but will instead ignore the packet. The first argument contains the client and transaction identification information. The second argument includes the details of the error. +% DHCP4_PACKET_DROP_0008 %1: DHCP service is globally disabled +This debug message is issued when a packet is dropped because the DHCP service +has been temporarily disabled. This affects all received DHCP packets. The +service may be enabled by the "dhcp-enable" control command or automatically +after a specified amount of time since receiving "dhcp-disable" command. + % DHCP4_PACKET_NAK_0001 %1: failed to select a subnet for incoming packet, src %2, type %3 This error message is output when a packet was received from a subnet for which the DHCPv4 server has not been configured. The most probable diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 8912441fa4..97ecfa5cf3 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -416,7 +416,7 @@ const std::string Dhcpv4Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_"); Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast, const bool direct_response_desired) : io_service_(new IOService()), shutdown_(true), alloc_engine_(), port_(port), - use_bcast_(use_bcast) { + use_bcast_(use_bcast), network_state_(NetworkState::DHCPv4) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port); try { @@ -804,7 +804,14 @@ Dhcpv4Srv::run_one() { return; } - processPacket(query, rsp); + // If the DHCP service has been globally disabled, drop the packet. + if (!network_state_.isServiceEnabled()) { + LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_BASIC, + DHCP4_PACKET_DROP_0008) + .arg(query->getLabel()); + } else { + processPacket(query, rsp); + } if (!rsp) { return; diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 2fa90f127e..e72af87c84 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -15,10 +15,11 @@ #include #include #include -#include -#include #include #include +#include +#include +#include #include #include @@ -840,6 +841,12 @@ private: uint16_t port_; ///< UDP port number on which server listens. bool use_bcast_; ///< Should broadcast be enabled on sockets (if true). +protected: + + /// @brief Holds information about disabled DHCP service and/or + /// disabled subnet/network scopes. + NetworkState network_state_; + public: /// Class methods for DHCPv4-over-DHCPv6 handler diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 19500b540f..1ac0981a51 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -86,6 +86,7 @@ public: /// Expose internal methods for the sake of testing using Dhcpv4Srv::receivePacket; + using Dhcpv4Srv::network_state_; }; /// @brief Default control connection timeout. @@ -1154,6 +1155,74 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configReloadValid) { ::remove("test8.json"); } +// This test verifies if it is possible to disable DHCP service via command. +TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisable) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"dhcp-disable\" }", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + EXPECT_FALSE(server_->network_state_.isServiceEnabled()); +} + +// This test verifies that it is possible to disable DHCP service for a short +// period of time, after which the service is automatically enabled. +TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisableTemporarily) { + createUnixChannelServer(); + std::string response; + + // Send a command to disable DHCP service for 3 seconds. + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"max-period\": 3" + " }" + "}", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // The service should be disabled. + EXPECT_FALSE(server_->network_state_.isServiceEnabled()); + // And the timer should be scheduled which counts the time to automatic + // enabling of the service. + EXPECT_TRUE(server_->network_state_.isDelayedEnableAll()); +} + +// This test verifies if it is possible to enable DHCP service via command. +TEST_F(CtrlChannelDhcpv4SrvTest, dhcpEnable) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"dhcp-enable\" }", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + EXPECT_TRUE(server_->network_state_.isServiceEnabled()); +} + /// Verify that concurrent connections over the control channel can be /// established. /// @todo Future Kea 1.3 tickets will modify the behavior of the CommandMgr @@ -1402,4 +1471,5 @@ TEST_F(CtrlChannelDhcpv4SrvTest, connectionTimeout) { + } // End of anonymous namespace diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 87bf60bfde..dceb6367fb 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -20,6 +20,7 @@ #include #include #include +#include using namespace isc::config; using namespace isc::dhcp; @@ -377,6 +378,64 @@ ControlledDhcpv6Srv::commandConfigTestHandler(const string&, return (checkConfig(dhcp6)); } +ConstElementPtr +ControlledDhcpv6Srv::commandDhcpDisableHandler(const std::string&, + ConstElementPtr args) { + std::ostringstream message; + int64_t max_period = 0; + + // Parse arguments to see if the 'max-period' parameter has been specified. + if (args) { + // Arguments must be a map. + if (args->getType() != Element::map) { + message << "arguments for the 'dhcp-disable' command must be a map"; + + } else { + ConstElementPtr max_period_element = args->get("max-period"); + // max-period is optional. + if (max_period_element) { + // It must be an integer, if specified. + if (max_period_element->getType() != Element::integer) { + message << "'max-period' argument must be a number"; + + } else { + // It must be positive integer. + max_period = max_period_element->intValue(); + if (max_period <= 0) { + message << "'max-period' must be positive integer"; + } + + // The user specified that the DHCP service should resume not + // later than in max-period seconds. If the 'dhcp-enable' command + // is not sent, the DHCP service will resume automatically. + network_state_.delayedEnableAll(static_cast(max_period)); + } + } + } + } + + // No error occurred, so let's disable the service. + if (message.tellp() == 0) { + network_state_.disableService(); + + message << "DHCPv6 service disabled"; + if (max_period > 0) { + message << " for " << max_period << " seconds"; + } + // Success. + return (config::createAnswer(CONTROL_RESULT_SUCCESS, message.str())); + } + + // Failure. + return (config::createAnswer(CONTROL_RESULT_ERROR, message.str())); +} + +ConstElementPtr +ControlledDhcpv6Srv::commandDhcpEnableHandler(const std::string&, ConstElementPtr) { + network_state_.enableService(); + return (config::createAnswer(CONTROL_RESULT_SUCCESS, "DHCP service successfully enabled")); +} + ConstElementPtr ControlledDhcpv6Srv::commandVersionGetHandler(const string&, ConstElementPtr) { ElementPtr extended = Element::create(Dhcpv6Srv::getVersion(true)); @@ -457,6 +516,12 @@ ControlledDhcpv6Srv::processCommand(const std::string& command, } else if (command == "config-test") { return (srv->commandConfigTestHandler(command, args)); + } else if (command == "dhcp-disable") { + return (srv->commandDhcpDisableHandler(command, args)); + + } else if (command == "dhcp-enable") { + return (srv->commandDhcpEnableHandler(command, args)); + } else if (command == "version-get") { return (srv->commandVersionGetHandler(command, args)); @@ -638,6 +703,12 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port) CommandMgr::instance().registerCommand("config-write", boost::bind(&ControlledDhcpv6Srv::commandConfigWriteHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("dhcp-disable", + boost::bind(&ControlledDhcpv6Srv::commandDhcpDisableHandler, this, _1, _2)); + + CommandMgr::instance().registerCommand("dhcp-enable", + boost::bind(&ControlledDhcpv6Srv::commandDhcpEnableHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("leases-reclaim", boost::bind(&ControlledDhcpv6Srv::commandLeasesReclaimHandler, this, _1, _2)); @@ -694,6 +765,8 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() { CommandMgr::instance().deregisterCommand("config-reload"); CommandMgr::instance().deregisterCommand("config-test"); CommandMgr::instance().deregisterCommand("config-write"); + CommandMgr::instance().deregisterCommand("dhcp-disable"); + CommandMgr::instance().deregisterCommand("dhcp-enable"); CommandMgr::instance().deregisterCommand("leases-reclaim"); CommandMgr::instance().deregisterCommand("libreload"); CommandMgr::instance().deregisterCommand("shutdown"); diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 4648aefb5c..21d7e7493d 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -223,6 +223,27 @@ private: commandConfigTestHandler(const std::string& command, isc::data::ConstElementPtr args); + /// @brief A handler for processing 'dhcp-disable' command. + /// + /// @param command command name (ignored). + /// @param args aguments for the command. It must be a map and + /// it may include optional 'max-period' parameter. + /// + /// @return result of the command. + isc::data::ConstElementPtr + commandDhcpDisableHandler(const std::string& command, + isc::data::ConstElementPtr args); + + /// @brief A handler for processing 'dhcp-enable' command. + /// + /// @param command command name (ignored) + /// @param args arguments for the command (ignored). + /// + /// @return result of the command. + isc::data::ConstElementPtr + commandDhcpEnableHandler(const std::string& command, + isc::data::ConstElementPtr args); + /// @Brief handler for processing 'version-get' command /// /// This handler processes version-get command, which returns diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 4fde8c77ed..0d6c9caebe 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -409,8 +409,14 @@ server is about to open sockets on the specified port. A warning message issued when IfaceMgr fails to open and bind a socket. The reason for the failure is appended as an argument of the log message. +% DHCP6_PACKET_DROP_DHCP_DISABLED %1: DHCP service is globally disabled +This debug message is issued when a packet is dropped because the DHCP service +has been temporarily disabled. This affects all received DHCP packets. The +service may be enabled by the "dhcp-enable" control command or automatically +after a specified amount of time since receiving "dhcp-disable" command. + % DHCP6_PACKET_DROP_PARSE_FAIL failed to parse packet from %1 to %2, received over interface %3, reason: %4 -The DHCPv4 server has received a packet that it is unable to +The DHCPv6 server has received a packet that it is unable to interpret. The reason why the packet is invalid is included in the message. % DHCP6_PACKET_DROP_SERVERID_MISMATCH %1: dropping packet with server identifier: %2, server is using: %3 diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index a0aa2cb111..94fd68d137 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -179,7 +179,7 @@ const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_"); Dhcpv6Srv::Dhcpv6Srv(uint16_t port) : io_service_(new IOService()), port_(port), serverid_(), shutdown_(true), - alloc_engine_() + alloc_engine_(), name_change_reqs_(), network_state_(NetworkState::DHCPv6) { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_OPEN_SOCKET).arg(port); @@ -468,7 +468,14 @@ void Dhcpv6Srv::run_one() { return; } - processPacket(query, rsp); + // If the DHCP service has been globally disabled, drop the packet. + if (!network_state_.isServiceEnabled()) { + LOG_DEBUG(bad_packet6_logger, DBG_DHCP6_DETAIL_DATA, + DHCP6_PACKET_DROP_DHCP_DISABLED) + .arg(query->getLabel()); + } else { + processPacket(query, rsp); + } if (!rsp) { return; @@ -2004,18 +2011,31 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query, .arg(static_cast((*l)->prefixlen_)) .arg(ia->getIAID()); - // Now remove this address from the hints list. + // Now remove this prefix from the hints list. AllocEngine::ResourceType hint_type((*l)->addr_, (*l)->prefixlen_); hints.erase(std::remove(hints.begin(), hints.end(), hint_type), hints.end()); } - /// @todo: Maybe we should iterate over ctx.old_leases_, i.e. the leases - /// that used to be valid, but they are not anymore. + /// For the leases that we just retired, send the prefixes with 0 lifetimes. + for (Lease6Collection::const_iterator l = ctx.currentIA().old_leases_.begin(); + l != ctx.currentIA().old_leases_.end(); ++l) { - // For all the leases the client had requested, but we didn't assign, put them with - // zero lifetimes - // Finally, if there are any addresses requested that we haven't dealt with + // Send a prefix with zero lifetimes only when this lease belonged to + // this client. Do not send it when we're reusing an old lease that belonged + // to someone else. + if (equalValues(query->getClientId(), (*l)->duid_)) { + Option6IAPrefixPtr prefix(new Option6IAPrefix(D6O_IAPREFIX, (*l)->addr_, + (*l)->prefixlen_, 0, 0)); + ia_rsp->addOption(prefix); + } + + // Now remove this prefix from the hints list. + AllocEngine::ResourceType hint_type((*l)->addr_, (*l)->prefixlen_); + hints.erase(std::remove(hints.begin(), hints.end(), hint_type), hints.end()); + } + + // Finally, if there are any prefixes requested that we haven't dealt with // already, inform the client that he can't have them. for (AllocEngine::HintContainer::const_iterator prefix = hints.begin(); prefix != hints.end(); ++prefix) { diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index ed7a103005..53d0d25a3c 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -871,6 +872,11 @@ protected: /// Holds a list of @c isc::dhcp_ddns::NameChangeRequest objects, which /// are waiting for sending to kea-dhcp-ddns module. std::queue name_change_reqs_; + + /// @brief Holds information about disabled DHCP service and/or + /// disabled subnet/network scopes. + NetworkState network_state_; + }; }; // namespace isc::dhcp diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 9e24fdb249..b105e5557d 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -83,6 +83,7 @@ public: /// Expose internal methods for the sake of testing using Dhcpv6Srv::receivePacket; + using Dhcpv6Srv::network_state_; }; /// @brief Default control connection timeout. @@ -1175,6 +1176,74 @@ TEST_F(CtrlChannelDhcpv6SrvTest, configReloadValid) { ::remove("test8.json"); } +// This test verifies if it is possible to disable DHCP service via command. +TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisable) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"dhcp-disable\" }", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + EXPECT_FALSE(server_->network_state_.isServiceEnabled()); +} + +// This test verifies that it is possible to disable DHCP service for a short +// period of time, after which the service is automatically enabled. +TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisableTemporarily) { + createUnixChannelServer(); + std::string response; + + // Send a command to disable DHCP service for 3 seconds. + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"max-period\": 3" + " }" + "}", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // The service should be disabled. + EXPECT_FALSE(server_->network_state_.isServiceEnabled()); + // And the timer should be scheduled which counts the time to automatic + // enabling of the service. + EXPECT_TRUE(server_->network_state_.isDelayedEnableAll()); +} + +// This test verifies if it is possible to enable DHCP service via command. +TEST_F(CtrlChannelDhcpv6SrvTest, dhcpEnable) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"dhcp-enable\" }", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + EXPECT_TRUE(server_->network_state_.isServiceEnabled()); +} + /// Verify that concurrent connections over the control channel can be /// established. /// @todo Future Kea 1.3 tickets will modify the behavior of the CommandMgr diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index cc3e148634..de492e611b 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -140,6 +140,7 @@ endif libkea_dhcpsrv_la_SOURCES += ncr_generator.cc ncr_generator.h libkea_dhcpsrv_la_SOURCES += network.cc network.h +libkea_dhcpsrv_la_SOURCES += network_state.cc network_state.h if HAVE_PGSQL libkea_dhcpsrv_la_SOURCES += pgsql_connection.cc pgsql_connection.h diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index c7c08c371a..28e35668f7 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -1397,7 +1397,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx, // It is for advertise only. We should not insert the lease into LeaseMgr, // but rather check that we could have inserted it. Lease6Ptr existing = LeaseMgrFactory::instance().getLease6( - Lease::TYPE_NA, addr); + ctx.currentIA().type_, addr); if (!existing) { return (lease); } else { diff --git a/src/lib/dhcpsrv/host.cc b/src/lib/dhcpsrv/host.cc index 2f37017064..562bfb4702 100644 --- a/src/lib/dhcpsrv/host.cc +++ b/src/lib/dhcpsrv/host.cc @@ -434,7 +434,7 @@ Host::toElement4() const { } else { isc_throw(ToElementError, "invalid identifier type: " << id_type); } - // Set the reservation + // Set the reservation (if not 0.0.0.0 which may not be re-read) const IOAddress& address = getIPv4Reservation(); if (!address.isV4Zero()) { map->set("ip-address", Element::create(address.toText())); diff --git a/src/lib/dhcpsrv/network_state.cc b/src/lib/dhcpsrv/network_state.cc new file mode 100644 index 0000000000..552f7c76d8 --- /dev/null +++ b/src/lib/dhcpsrv/network_state.cc @@ -0,0 +1,155 @@ +// 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 + +namespace { + +/// @brief Name of the timer used by the @c NetworkState class. +const std::string NETWORK_STATE_TIMER_NAME = "network-state-timer"; + +} // end of anonymous namespace + +namespace isc { +namespace dhcp { + +/// @brief Implementation of the @c NetworkState class. +class NetworkStateImpl : public boost::enable_shared_from_this { +public: + + /// @brief Constructor. + NetworkStateImpl(const NetworkState::ServerType& server_type) + : server_type_(server_type), globally_disabled_(false), disabled_subnets_(), + disabled_networks_(), timer_mgr_(TimerMgr::instance()) { + } + + /// @brief Destructor. + ~NetworkStateImpl() { + destroyTimer(); + } + + /// @brief Globally disables or enables DHCP service. + void setDisableService(const bool disable) { + globally_disabled_ = disable; + } + + /// @brief Enables DHCP service globally and per scopes. + /// + /// If delayed enabling DHCP service has been scheduled, it cancels it. + void enableAll() { + setDisableService(false); + + /// @todo Enable service for all subnets and networks here. + + destroyTimer(); + } + + /// @brief Creates a timer counting the time when @c enableAll should be + /// automatically called. + /// + /// If the timer has been already scheduled, it is destroyed and replaced + /// with a new timer. + /// + /// @param seconds Number of seconds to elapse before the @c enableAll is + /// called. + void createTimer(const unsigned int seconds) { + destroyTimer(); + timer_mgr_->registerTimer(NETWORK_STATE_TIMER_NAME, + boost::bind(&NetworkStateImpl::enableAll, + shared_from_this()), + seconds * 1000, + asiolink::IntervalTimer::ONE_SHOT); + timer_mgr_->setup(NETWORK_STATE_TIMER_NAME); + } + + /// @brief Destroys a timer if present. + void destroyTimer() { + if (timer_mgr_->isTimerRegistered(NETWORK_STATE_TIMER_NAME)) { + timer_mgr_->unregisterTimer(NETWORK_STATE_TIMER_NAME); + } + } + + /// @brief Server type. + NetworkState::ServerType server_type_; + + /// @brief A flag indicating if DHCP service is globally disabled. + bool globally_disabled_; + + /// @brief A list of subnets for which the DHCP service has been disabled. + NetworkState::Subnets disabled_subnets_; + + /// @brief A list of networks for which the DHCP service has been disabled. + NetworkState::Networks disabled_networks_; + + /// @brief A pointer to the common timer manager. + /// + /// This pointer is held here to make sure that the timer manager is not + /// destroyed before an instance of this class is destroyed. + TimerMgrPtr timer_mgr_; +}; + +NetworkState::NetworkState(const NetworkState::ServerType& server_type) + : impl_(new NetworkStateImpl(server_type)) { +} + +void +NetworkState::disableService() { + impl_->setDisableService(true); +} + +void +NetworkState::enableService() { + impl_->setDisableService(false); +} + +void +NetworkState::enableAll() { + impl_->enableAll(); +} + +void +NetworkState::delayedEnableAll(const unsigned int seconds) { + impl_->createTimer(seconds); +} + +bool +NetworkState::isServiceEnabled() const { + return (!impl_->globally_disabled_); +} + +bool +NetworkState::isDelayedEnableAll() const { + return (TimerMgr::instance()->isTimerRegistered(NETWORK_STATE_TIMER_NAME)); +} + +void +NetworkState::selectiveDisable(const NetworkState::Subnets& subnets) { + isc_throw(NotImplemented, "selectiveDisableService is not implemented"); +} + +void +NetworkState::selectiveDisable(const NetworkState::Networks& networks) { + isc_throw(NotImplemented, "selectiveDisableService is not implemented"); +} + +void +NetworkState::selectiveEnable(const NetworkState::Subnets& subnets) { + isc_throw(NotImplemented, "selectiveEnableService is not implemented"); +} + +void +NetworkState::selectiveEnable(const NetworkState::Networks& networks) { + isc_throw(NotImplemented, "selectiveEnableService is not implemented"); +} + + +} // end of namespace isc::dhcp +} // end of namespace isc diff --git a/src/lib/dhcpsrv/network_state.h b/src/lib/dhcpsrv/network_state.h new file mode 100644 index 0000000000..137c29bbed --- /dev/null +++ b/src/lib/dhcpsrv/network_state.h @@ -0,0 +1,161 @@ +// 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 NETWORK_STATE_H +#define NETWORK_STATE_H + +#include +#include +#include +#include +#include + +namespace isc { +namespace dhcp { + +class NetworkStateImpl; + +/// @brief Holds information about DHCP service enabling status. +/// +/// When the DHCP server receives a command to disable DHCP service entirely +/// or for specific networks, this has to be recorded to allow for re-enabling +/// DHCP service for these networks as a result of receiving a command from +/// the administrator or when the timeout for re-enabling the service occurs. +/// +/// In the future, it will be possible to specify "disabled" parameter for +/// a subnet (or network) in the configuration file to indicate that this subnet +/// should be excluded from the service. When a command is subsequently sent to +/// temporarily disable a service for some other subnets for a specified amount +/// ot time, only these subnets should be re-enabled when the time elapses. This +/// class fulfils this requirement by recording the subnets disabled with a command +/// and re-enabling them when required. The subnets specified as "disabled" in +/// the configuration file should remain disabled until explcitly enabled with a +/// control command. +/// +/// This class also allows for disabling the DHCP service globally. In this case +/// the server drops all received packets. +/// +/// The "dhcp-disable" and "dhcp-enable" commands are used for globally disabling +/// and enabling the DHCP service. The "dhcp-disable-scopes" and "dhcp-enable-scopes" +/// commands are used to disable and enable DHCP service for subnets and networks. +/// In case of the "dhcp-disable" and "dhcp-disable-scopes" commands, it is possible +/// to specify "max-period" parameter which provides a timeout, after which the +/// settings are reverted (service is re-enabled globally and/or for specific +/// scopes). +/// +/// Disabling DHCP service with a timeout is useful to guard against issues when +/// the controlling client dies after disabling the DHCP service on the server, +/// e.g. failover peers may instruct each other to disable the DHCP service while +/// database synchronization takes place. If the peer subsequently dies, the +/// surviving server must re-enable DHCP on its own. +/// +/// @todo This class currently supports only the case of globally disabling +/// the DHCP service. Disabling per network/subnet will be added later. +class NetworkState { +public: + + /// @brief DHCP server type. + enum ServerType { + DHCPv4, + DHCPv6 + }; + + /// @brief Type of the container holding collection of subnet identifiers. + typedef std::set Subnets; + + /// @brief Type of the container holding collection of shared network names. + typedef std::set Networks; + + /// @brief Constructor. + NetworkState(const ServerType& server_type); + + /// @brief Globally disables DHCP service. + /// + /// The DHCP service becomes disabled for all subnets and networks, + /// regardless of the per scope settings. + void disableService(); + + /// @brief Globally enables DHCP service. + /// + /// The DHCP service becomes enabled but per scope settings are in effect. + /// In order to enable the service for all scopes previously disabled with + /// a control command, use @c enableAll. + void enableService(); + + /// @brief Enables DHCP service globally and for scopes which have been + /// disabled as a result of control command. + void enableAll(); + + /// @brief Schedules enabling DHCP service in the future. + /// + /// @param seconds Number of seconds after which the service should be enabled + /// unless @c enableAll is enabled before that time. + void delayedEnableAll(const unsigned int seconds); + + /// @brief Checks if the DHCP service is globally enabled. + /// + /// @return true if the service is globally enabled, false otherwise. + bool isServiceEnabled() const; + + /// @brief Checks if delayed enabling of DHCP services is scheduled. + /// + /// It indicates that the timer is present which counts the time until + /// @c enableAll function will be called automatically. + /// + /// @return true if delayed enabling of the DHCP service is scheduled, + /// false otherwise. + bool isDelayedEnableAll() const; + + /// @name Selective disabling/enabling DHCP service per scopes + //@{ + + /// @brief Disable DHCP service for selected subnets. + /// + /// @param subnets Collection of subnet identifiers for which the service + /// should be disabled. + /// + /// @throw isc::NotImplemented + void selectiveDisable(const NetworkState::Subnets& subnets); + + /// @brief Disable DHCP service for selected networks. + /// + /// @param networks Collection of shared network names for which the service + /// should be disabled. + /// + /// @throw isc::NotImplemented + void selectiveDisable(const NetworkState::Networks& networks); + + /// @brief Enable DHCP service for selected subnets. + /// + /// @param subnets Collection of subnet identifiers for which the service + /// should be disabled. + /// + /// @throw isc::NotImplemented + void selectiveEnable(const NetworkState::Subnets& subnets); + + /// @brief Enable DHCP service for selected networks. + /// + /// @param networks Collection of shared network names for which the service + /// should be enabled. + /// + /// @throw isc::NotImplemented + void selectiveEnable(const NetworkState::Networks& networks); + + //@} + +private: + + /// @brief Pointer to the @c NetworkState implementation. + boost::shared_ptr impl_; +}; + +/// @brief Pointer to the @c NetworkState object. +typedef boost::shared_ptr NetworkStatePtr; + +} // end of namespace isc::dhcp +} // end of namespace isc + +#endif // NETWORK_STATE_H diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index b332cbb8d4..0f17fcf5ed 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -236,8 +236,20 @@ SrvConfig::toElement() const { dhcp->set("option-data", cfg_option_->toElement()); // Set subnets and shared networks. + + // We have two problems to solve: + // - a subnet is unparsed once: + // * if it is a plain subnet in the global subnet list + // * if it is a member of a shared network in the shared network + // subnet list + // - unparsed subnets must be kept to add host reservations in them. + // Of course this can be done only when subnets are unparsed. + + // The list of all unparsed subnets std::vector sn_list; + if (family == AF_INET) { + // Get plain subnets ElementPtr plain_subnets = Element::createList(); const Subnet4Collection* subnets = cfg_subnets4_->getAll(); for (Subnet4Collection::const_iterator subnet = subnets->cbegin(); @@ -254,7 +266,11 @@ SrvConfig::toElement() const { } dhcp->set("subnet4", plain_subnets); + // Get shared networks ElementPtr shared_networks = cfg_shared_networks4_->toElement(); + dhcp->set("shared-networks", shared_networks); + + // Get subnets in shared network subnet lists const std::vector networks = shared_networks->listValue(); for (auto network = networks.cbegin(); network != networks.cend(); ++network) { @@ -265,9 +281,9 @@ SrvConfig::toElement() const { sn_list.push_back(*subnet); } } - dhcp->set("shared-networks", shared_networks); } else { + // Get plain subnets ElementPtr plain_subnets = Element::createList(); const Subnet6Collection* subnets = cfg_subnets6_->getAll(); for (Subnet6Collection::const_iterator subnet = subnets->cbegin(); @@ -284,7 +300,11 @@ SrvConfig::toElement() const { } dhcp->set("subnet6", plain_subnets); + // Get shared networks ElementPtr shared_networks = cfg_shared_networks6_->toElement(); + dhcp->set("shared-networks", shared_networks); + + // Get subnets in shared network subnet lists const std::vector networks = shared_networks->listValue(); for (auto network = networks.cbegin(); network != networks.cend(); ++network) { @@ -295,7 +315,6 @@ SrvConfig::toElement() const { sn_list.push_back(*subnet); } } - dhcp->set("shared-networks", shared_networks); } // Insert reservations CfgHostsList resv_list; diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 342ed921d8..8f55790f3c 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -133,6 +133,7 @@ libdhcpsrv_unittests_SOURCES += test_get_callout_handle.cc test_get_callout_hand libdhcpsrv_unittests_SOURCES += triplet_unittest.cc libdhcpsrv_unittests_SOURCES += test_utils.cc test_utils.h libdhcpsrv_unittests_SOURCES += timer_mgr_unittest.cc +libdhcpsrv_unittests_SOURCES += network_state_unittest.cc libdhcpsrv_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) if HAVE_MYSQL diff --git a/src/lib/dhcpsrv/tests/host_unittest.cc b/src/lib/dhcpsrv/tests/host_unittest.cc index bc17bb13f4..d3603fbae7 100644 --- a/src/lib/dhcpsrv/tests/host_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_unittest.cc @@ -191,7 +191,7 @@ TEST_F(HostTest, getIdentifier) { // This test verifies that it is possible to create a Host object // using hardware address in the textual format. TEST_F(HostTest, createFromHWAddrString) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2), IOAddress("192.0.2.3"), @@ -231,7 +231,7 @@ TEST_F(HostTest, createFromHWAddrString) { // This test verifies that it is possible to create Host object using // a DUID in the textual format. TEST_F(HostTest, createFromDUIDString) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("a1:b2:c3:d4:e5:06", "duid", SubnetID(10), SubnetID(20), IOAddress("192.0.2.5"), @@ -266,7 +266,7 @@ TEST_F(HostTest, createFromDUIDString) { // This test verifies that it is possible to create Host object using // hardware address in the binary format. TEST_F(HostTest, createFromHWAddrBinary) { - boost::scoped_ptr host; + HostPtr host; // Prepare the hardware address in binary format. const uint8_t hwaddr_data[] = { 0xaa, 0xab, 0xca, 0xda, 0xbb, 0xee @@ -303,7 +303,7 @@ TEST_F(HostTest, createFromHWAddrBinary) { // This test verifies that it is possible to create a Host object using // DUID in the binary format. TEST_F(HostTest, createFromDuidBinary) { - boost::scoped_ptr host; + HostPtr host; // Prepare DUID binary. const uint8_t duid_data[] = { 1, 2, 3, 4, 5, 6 @@ -332,7 +332,7 @@ TEST_F(HostTest, createFromDuidBinary) { // This test verifies that it is possible create Host instance using all // supported identifiers in a binary format. TEST_F(HostTest, createFromIdentifierBinary) { - boost::scoped_ptr host; + HostPtr host; // Iterate over all supported identifier types. for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) { const Host::IdentifierType type = static_cast(i); @@ -362,7 +362,7 @@ TEST_F(HostTest, createFromIdentifierBinary) { // This test verifies that it is possible to create Host instance using // all supported identifiers in hexadecimal format. TEST_F(HostTest, createFromIdentifierHex) { - boost::scoped_ptr host; + HostPtr host; // Iterate over all supported identifiers. for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) { const Host::IdentifierType type = static_cast(i); @@ -408,7 +408,7 @@ TEST_F(HostTest, createFromIdentifierHex) { // This test verifies that it is possible to create Host instance using // identifiers specified as text in quotes. TEST_F(HostTest, createFromIdentifierString) { - boost::scoped_ptr host; + HostPtr host; // It is not allowed to specify HW address or DUID as a string in quotes. for (unsigned int i = 2; i < identifierTypeUpperBound(); ++i) { const Host::IdentifierType type = static_cast(i); @@ -454,7 +454,7 @@ TEST_F(HostTest, createFromIdentifierString) { // using setIdentifier method with an identifier specified in // hexadecimal format. TEST_F(HostTest, setIdentifierHex) { - boost::scoped_ptr host; + HostPtr host; // Iterate over all supported identifiers. for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) { @@ -542,7 +542,7 @@ TEST_F(HostTest, setIdentifierHex) { // using setIdentifier method with an identifier specified in binary // format. TEST_F(HostTest, setIdentifierBinary) { - boost::scoped_ptr host; + HostPtr host; // Iterate over all supported identifier types. for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) { @@ -600,7 +600,7 @@ TEST_F(HostTest, setIdentifierBinary) { // This test verifies that the IPv6 reservations of a different type can // be added for the host. TEST_F(HostTest, addReservations) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2), IOAddress("192.0.2.3")))); @@ -658,7 +658,7 @@ TEST_F(HostTest, addReservations) { // This test checks that various modifiers may be used to replace the current // values of the Host class. TEST_F(HostTest, setValues) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2), IOAddress("192.0.2.3"), @@ -714,7 +714,7 @@ TEST_F(HostTest, setValues) { // Test that Host constructors initialize client classes from string. TEST_F(HostTest, clientClassesFromConstructor) { - boost::scoped_ptr host; + HostPtr host; // Prepare the hardware address in binary format. const uint8_t hwaddr_data[] = { 0xaa, 0xab, 0xca, 0xda, 0xbb, 0xee @@ -755,7 +755,7 @@ TEST_F(HostTest, clientClassesFromConstructor) { // Test that new client classes can be added for the Host. TEST_F(HostTest, addClientClasses) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2), IOAddress("192.0.2.3")))); @@ -942,7 +942,7 @@ TEST_F(HostTest, getIdentifierName) { // This test checks that Host object is correctly described in the // textual format using the toText method. TEST_F(HostTest, toText) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2), IOAddress("192.0.2.3"), @@ -1188,7 +1188,7 @@ TEST_F(HostTest, unparse) { // Test verifies if the host can store HostId properly. TEST_F(HostTest, hostId) { - boost::scoped_ptr host; + HostPtr host; ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2), IOAddress("192.0.2.3"), diff --git a/src/lib/dhcpsrv/tests/network_state_unittest.cc b/src/lib/dhcpsrv/tests/network_state_unittest.cc new file mode 100644 index 0000000000..30933adc7e --- /dev/null +++ b/src/lib/dhcpsrv/tests/network_state_unittest.cc @@ -0,0 +1,154 @@ +// 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 + +using namespace isc; +using namespace isc::asiolink; +using namespace isc::dhcp; + +namespace { + +/// @brief Test fixture class for @c NetworkState class. +class NetworkStateTest : public ::testing::Test { +public: + + /// @brief Constructor. + NetworkStateTest() + : io_service_(new IOService()), + test_timer_(*io_service_) { + TimerMgr::instance()->unregisterTimers(); + TimerMgr::instance()->setIOService(io_service_); + } + + /// @brief Destructor. + virtual ~NetworkStateTest() { + // Cancel timers. + TimerMgr::instance()->unregisterTimers(); + test_timer_.cancel(); + // Make sure IO service will stop when no timers are scheduled. + io_service_->stopWork(); + // Run outstanding tasks. + io_service_->run(); + } + + /// @brief Test timer callback stopping IO service. + void testTimerCallback() { + TimerMgr::instance()->unregisterTimers(); + io_service_->stop(); + } + + /// @brief Runs IO service with a timeout. + /// + /// @param timeout_ms Timeout for running IO service in milliseconds. + void runIOService(const long timeout_ms) { + test_timer_.setup(boost::bind(&NetworkStateTest::testTimerCallback, this), timeout_ms, + IntervalTimer::ONE_SHOT); + io_service_->run(); + } + + /// @brief IO service used during the tests. + IOServicePtr io_service_; + + /// @brief Timeout detecting timer. + IntervalTimer test_timer_; +}; + +// This test verifies the default is enable state. +TEST_F(NetworkStateTest, default) { + NetworkState state4(NetworkState::DHCPv4); + EXPECT_TRUE(state4.isServiceEnabled()); + NetworkState state6(NetworkState::DHCPv6); + EXPECT_TRUE(state6.isServiceEnabled()); +} + +// This test verifies that it is possible to disable and then enable DHCPv4 service. +TEST_F(NetworkStateTest, disableEnableService4) { + NetworkState state(NetworkState::DHCPv4); + state.disableService(); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(); + EXPECT_TRUE(state.isServiceEnabled()); +} + +// This test verifies that it is possible to disable and then enable DHCPv6 service. +TEST_F(NetworkStateTest, disableEnableService6) { + NetworkState state(NetworkState::DHCPv6); + state.disableService(); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(); + EXPECT_TRUE(state.isServiceEnabled()); +} + +// This test verifies that enableAll() enables the service. This test will be extended +// in the future to verify that it also enables disabled scopes. +TEST_F(NetworkStateTest, enableAll) { + NetworkState state(NetworkState::DHCPv4); + state.disableService(); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableAll(); + EXPECT_TRUE(state.isServiceEnabled()); +} + +// This test verifies that it is possible to setup delayed execution of enableAll +// function. +TEST_F(NetworkStateTest, delayedEnableAll) { + NetworkState state(NetworkState::DHCPv4); + // Disable the service and then schedule enabling it in 1 second. + state.disableService(); + state.delayedEnableAll(1); + // Initially the service should be still disabled. + EXPECT_FALSE(state.isServiceEnabled()); + // After running IO service for 2 seconds, the service should be enabled. + runIOService(2000); + EXPECT_TRUE(state.isServiceEnabled()); +} + +// This test verifies that explicitly enabling the service cancels the timer +// scheduled for automatically enabling it. +TEST_F(NetworkStateTest, earlyEnableAll) { + NetworkState state(NetworkState::DHCPv4); + // Disable the service. + state.disableService(); + EXPECT_FALSE(state.isServiceEnabled()); + // Schedule enabling the service in 2 seconds. + state.delayedEnableAll(2); + // Explicitly enable the service. + state.enableAll(); + // The timer should be now canceled and the service should be enabled. + EXPECT_FALSE(state.isDelayedEnableAll()); + EXPECT_TRUE(state.isServiceEnabled()); +} + +// This test verifies that it is possible to call delayedEnableAll multiple times +// and that it results in only one timer being scheduled. +TEST_F(NetworkStateTest, multipleDelayedEnableAll) { + NetworkState state(NetworkState::DHCPv4); + // Disable the service and then schedule enabling it in 1 second. + state.disableService(); + // Schedule the first timer for 5 seconds. + state.delayedEnableAll(5); + // When calling it the second time the old timer should be destroyed and + // the timeout should be set to 2 seconds. + state.delayedEnableAll(2); + // Initially the service should be still disabled. + EXPECT_FALSE(state.isServiceEnabled()); + // After running IO service for 3 seconds, the service should be enabled. + runIOService(3000); + EXPECT_TRUE(state.isServiceEnabled()); + // The timer should not be present, even though the first timer was created + // with 5 seconds interval. + EXPECT_FALSE(state.isDelayedEnableAll()); +} + + +} // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/srv_config_unittest.cc b/src/lib/dhcpsrv/tests/srv_config_unittest.cc index acf7806e59..1258164015 100644 --- a/src/lib/dhcpsrv/tests/srv_config_unittest.cc +++ b/src/lib/dhcpsrv/tests/srv_config_unittest.cc @@ -15,6 +15,7 @@ using namespace isc::asiolink; using namespace isc::dhcp; +using namespace isc::data; // Those are the tests for SrvConfig storage. Right now they are minimal, // but the number is expected to grow significantly once we migrate more @@ -409,10 +410,10 @@ TEST_F(SrvConfigTest, hooksLibraries) { EXPECT_EQ(0, libraries.get().size()); // Verify we can update it. - isc::data::ConstElementPtr elem0; + ConstElementPtr elem0; libraries.add("foo", elem0); std::string config = "{ \"library\": \"bar\" }"; - isc::data::ConstElementPtr elem1 = isc::data::Element::fromJSON(config); + ConstElementPtr elem1 = Element::fromJSON(config); libraries.add("bar", elem1); EXPECT_EQ(2, libraries.get().size()); EXPECT_EQ(2, conf.getHooksConfig().get().size()); @@ -487,4 +488,261 @@ TEST_F(SrvConfigTest, unparse) { (header4 + defaults + defaults4 + params + trailer, conf); } +// Verifies that the toElement method does not miss host reservations +TEST_F(SrvConfigTest, unparseHR) { + // DHCPv4 version + SrvConfig conf4(32); + + // Add a plain subnet + Triplet def_triplet; + SubnetID p_id(1); + Subnet4Ptr psubnet4(new Subnet4(IOAddress("192.0.1.0"), 24, + def_triplet, def_triplet, 4000, p_id)); + conf4.getCfgSubnets4()->add(psubnet4); + + // Add a shared network + SharedNetwork4Ptr network4(new SharedNetwork4("frog")); + conf4.getCfgSharedNetworks4()->add(network4); + + // Add a shared subnet + SubnetID s_id(100); + Subnet4Ptr ssubnet4(new Subnet4(IOAddress("192.0.2.0"), 24, + def_triplet, def_triplet, 4000, s_id)); + network4->add(ssubnet4); + + // Add a host reservation to the plain subnet + HostPtr phost4(new Host("00:01:02:03:04:05", "hw-address", + p_id, SubnetID(0), IOAddress("192.0.1.1"))); + conf4.getCfgHosts()->add(phost4); + + // Add a host reservation to the shared subnet + HostPtr shost4(new Host("00:05:04:03:02:01", "hw-address", + s_id, SubnetID(0), IOAddress("192.0.2.1"))); + conf4.getCfgHosts()->add(shost4); + + // Unparse the config + ConstElementPtr unparsed4 = conf4.toElement(); + ASSERT_TRUE(unparsed4); + ASSERT_EQ(Element::map, unparsed4->getType()); + + // Get Dhcp4 entry + ConstElementPtr dhcp4; + ASSERT_NO_THROW(dhcp4 = unparsed4->get("Dhcp4")); + ASSERT_TRUE(dhcp4); + ASSERT_EQ(Element::map, dhcp4->getType()); + + // Get plain subnets + ConstElementPtr check; + ASSERT_NO_THROW(check = dhcp4->get("subnet4")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the plain subnet + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Its ID is 1 + ConstElementPtr sub; + ASSERT_NO_THROW(sub = check->get("id")); + ASSERT_TRUE(sub); + ASSERT_EQ(Element::integer, sub->getType()); + EXPECT_EQ(p_id, sub->intValue()); + + // Get its host reservations + ASSERT_NO_THROW(check = check->get("reservations")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the plain host reservation + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Check the reserved address + ASSERT_NO_THROW(check = check->get("ip-address")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::string, check->getType()); + EXPECT_EQ("192.0.1.1", check->stringValue()); + + // Get shared networks + ASSERT_NO_THROW(check = dhcp4->get("shared-networks")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the shared network + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Its name is "frog" + ASSERT_NO_THROW(sub = check->get("name")); + ASSERT_TRUE(sub); + ASSERT_EQ(Element::string, sub->getType()); + EXPECT_EQ("frog", sub->stringValue()); + + // Get shared subnets + ASSERT_NO_THROW(check = check->get("subnet4")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the shared subnet + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Its ID is 100 + ASSERT_NO_THROW(sub = check->get("id")); + ASSERT_TRUE(sub); + ASSERT_EQ(Element::integer, sub->getType()); + EXPECT_EQ(s_id, sub->intValue()); + + // Get its host reservations + ASSERT_NO_THROW(check = check->get("reservations")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the shared host reservation + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Check the reserved address + ASSERT_NO_THROW(check = check->get("ip-address")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::string, check->getType()); + EXPECT_EQ("192.0.2.1", check->stringValue()); + + // DHCPv6 version + CfgMgr::instance().setFamily(AF_INET6); + SrvConfig conf6(32); + + // Add a plain subnet + Subnet6Ptr psubnet6(new Subnet6(IOAddress("2001:db8:1::"), 64, + 1000, 2000, 3000, 4000, p_id)); + conf6.getCfgSubnets6()->add(psubnet6); + + // Add a shared network + SharedNetwork6Ptr network6(new SharedNetwork6("frog")); + conf6.getCfgSharedNetworks6()->add(network6); + + // Add a shared subnet + Subnet6Ptr ssubnet6(new Subnet6(IOAddress("2001:db8:2::"), 64, + 1000, 2000, 3000, 4000, s_id)); + network6->add(ssubnet6); + + // Add a host reservation to the plain subnet + HostPtr phost6(new Host("a1:b2:c3:d4:e5:f6", "duid", SubnetID(0), + p_id, IOAddress::IPV4_ZERO_ADDRESS(), + "foo.example.org")); + conf6.getCfgHosts()->add(phost6); + + // Add a host reservation to the shared subnet + HostPtr shost6(new Host("f6:e5:d4:c3:b2:a1", "duid", SubnetID(0), + s_id, IOAddress::IPV4_ZERO_ADDRESS(), + "bar.example.org")); + conf6.getCfgHosts()->add(shost6); + + // Unparse the config + ConstElementPtr unparsed6 = conf6.toElement(); + ASSERT_TRUE(unparsed6); + ASSERT_EQ(Element::map, unparsed6->getType()); + + // Get Dhcp6 entry + ConstElementPtr dhcp6; + ASSERT_NO_THROW(dhcp6 = unparsed6->get("Dhcp6")); + ASSERT_TRUE(dhcp6); + ASSERT_EQ(Element::map, dhcp6->getType()); + + // Get plain subnets + ASSERT_NO_THROW(check = dhcp6->get("subnet6")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the plain subnet + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Its ID is 1 + ASSERT_NO_THROW(sub = check->get("id")); + ASSERT_TRUE(sub); + ASSERT_EQ(Element::integer, sub->getType()); + EXPECT_EQ(p_id, sub->intValue()); + + // Get its host reservations + ASSERT_NO_THROW(check = check->get("reservations")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the plain host reservation + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Check the host name + ASSERT_NO_THROW(check = check->get("hostname")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::string, check->getType()); + EXPECT_EQ("foo.example.org", check->stringValue()); + + // Get shared networks + ASSERT_NO_THROW(check = dhcp6->get("shared-networks")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the shared network + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Its name is "frog" + ASSERT_NO_THROW(sub = check->get("name")); + ASSERT_TRUE(sub); + ASSERT_EQ(Element::string, sub->getType()); + EXPECT_EQ("frog", sub->stringValue()); + + // Get shared subnets + ASSERT_NO_THROW(check = check->get("subnet6")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the shared subnet + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Its ID is 100 + ASSERT_NO_THROW(sub = check->get("id")); + ASSERT_TRUE(sub); + ASSERT_EQ(Element::integer, sub->getType()); + EXPECT_EQ(s_id, sub->intValue()); + + // Get its host reservations + ASSERT_NO_THROW(check = check->get("reservations")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::list, check->getType()); + EXPECT_EQ(1, check->size()); + + // Get the shared host reservation + ASSERT_NO_THROW(check = check->get(0)); + ASSERT_TRUE(check); + ASSERT_EQ(Element::map, check->getType()); + + // Check the host name + ASSERT_NO_THROW(check = check->get("hostname")); + ASSERT_TRUE(check); + ASSERT_EQ(Element::string, check->getType()); + EXPECT_EQ("bar.example.org", check->stringValue()); +} + } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc index 43ef1ac935..b6e6e94017 100644 --- a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc @@ -176,6 +176,8 @@ TEST_F(TimerMgrTest, registerTimer) { // Add a timer with a correct name. ASSERT_NO_THROW(timer_mgr_->registerTimer("timer2", makeCallback("timer2"), 1, IntervalTimer::ONE_SHOT)); + EXPECT_TRUE(timer_mgr_->isTimerRegistered("timer2")); + // Adding the timer with the same name as the existing timer is not // allowed. ASSERT_THROW(timer_mgr_->registerTimer("timer2", makeCallback("timer2"), 1, @@ -207,6 +209,7 @@ TEST_F(TimerMgrTest, unregisterTimer) { // Now unregister the correct one. ASSERT_NO_THROW(timer_mgr_->unregisterTimer("timer1")); ASSERT_EQ(0, timer_mgr_->timersCount()); + EXPECT_FALSE(timer_mgr_->isTimerRegistered("timer1")); doWait(100); diff --git a/src/lib/dhcpsrv/timer_mgr.cc b/src/lib/dhcpsrv/timer_mgr.cc index f2fe5269bb..bf3f6be139 100644 --- a/src/lib/dhcpsrv/timer_mgr.cc +++ b/src/lib/dhcpsrv/timer_mgr.cc @@ -114,6 +114,13 @@ public: /// process. void unregisterTimers(); + /// @brief Checks if the timer with a specified name has been registered. + /// + /// @param timer_name Name of the timer. + /// @return true if the timer with the specified name has been registered, + /// false otherwise. + bool isTimerRegistered(const std::string& timer_name); + /// @brief Returns the number of registered timers. size_t timersCount() const; @@ -233,6 +240,11 @@ TimerMgrImpl::unregisterTimers() { } } +bool +TimerMgrImpl::isTimerRegistered(const std::string& timer_name) { + return (registered_timers_.find(timer_name) != registered_timers_.end()); +} + size_t TimerMgrImpl::timersCount() const { return (registered_timers_.size()); @@ -351,6 +363,11 @@ TimerMgr::unregisterTimers() { impl_->unregisterTimers(); } +bool +TimerMgr::isTimerRegistered(const std::string& timer_name) { + return (impl_->isTimerRegistered(timer_name)); +} + size_t TimerMgr::timersCount() const { return (impl_->timersCount()); diff --git a/src/lib/dhcpsrv/timer_mgr.h b/src/lib/dhcpsrv/timer_mgr.h index ec5b9e3933..eaef7cafcb 100644 --- a/src/lib/dhcpsrv/timer_mgr.h +++ b/src/lib/dhcpsrv/timer_mgr.h @@ -100,6 +100,13 @@ public: /// @brief Unregisters all timers. void unregisterTimers(); + /// @brief Checks if the timer with a specified name has been registered. + /// + /// @param timer_name Name of the timer. + /// @return true if the timer with the specified name has been registered, + /// false otherwise. + bool isTimerRegistered(const std::string& timer_name); + /// @brief Returns the number of registered timers. size_t timersCount() const; diff --git a/src/lib/http/Makefile.am b/src/lib/http/Makefile.am index 2b0eed5752..599711500c 100644 --- a/src/lib/http/Makefile.am +++ b/src/lib/http/Makefile.am @@ -28,6 +28,7 @@ libkea_http_la_SOURCES += date_time.cc date_time.h libkea_http_la_SOURCES += http_log.cc http_log.h libkea_http_la_SOURCES += header_context.h libkea_http_la_SOURCES += http_acceptor.h +libkea_http_la_SOURCES += http_header.cc http_header.h libkea_http_la_SOURCES += http_types.h libkea_http_la_SOURCES += listener.cc listener.h libkea_http_la_SOURCES += post_request.cc post_request.h diff --git a/src/lib/http/connection.cc b/src/lib/http/connection.cc index 1e5f5cb2ec..418715ea8a 100644 --- a/src/lib/http/connection.cc +++ b/src/lib/http/connection.cc @@ -30,9 +30,12 @@ HttpConnection:: HttpConnection(asiolink::IOService& io_service, HttpConnectionPool& connection_pool, const HttpResponseCreatorPtr& response_creator, const HttpAcceptorCallback& callback, - const long request_timeout) + const long request_timeout, + const long idle_timeout) : request_timer_(io_service), + request_timer_setup_(false), request_timeout_(request_timeout), + idle_timeout_(idle_timeout), socket_(io_service), acceptor_(acceptor), connection_pool_(connection_pool), @@ -50,6 +53,7 @@ HttpConnection::~HttpConnection() { void HttpConnection::close() { + request_timer_setup_ = false; request_timer_.cancel(); socket_.close(); } @@ -117,7 +121,13 @@ HttpConnection::doWrite() { output_buf_.length(), cb); } else { - stopThisConnection(); + if (!request_->isPersistent()) { + stopThisConnection(); + + } else { + reinitProcessingState(); + doRead(); + } } } catch (const std::exception& ex) { stopThisConnection(); @@ -148,13 +158,8 @@ HttpConnection::acceptorCallback(const boost::system::error_code& ec) { HTTP_REQUEST_RECEIVE_START) .arg(getRemoteEndpointAddressAsText()) .arg(static_cast(request_timeout_/1000)); - // Pass raw pointer rather than shared_ptr to this object, - // because IntervalTimer already passes shared pointer to the - // IntervalTimerImpl to make sure that the callback remains - // valid. - request_timer_.setup(boost::bind(&HttpConnection::requestTimeoutCallback, - this), - request_timeout_, IntervalTimer::ONE_SHOT); + + setupRequestTimer(); doRead(); } } @@ -241,10 +246,47 @@ HttpConnection::socketWriteCallback(boost::system::error_code ec, size_t length) } else { output_buf_.clear(); - stopThisConnection(); + + if (!request_->isPersistent()) { + stopThisConnection(); + + } else { + reinitProcessingState(); + doRead(); + } } } +void +HttpConnection::reinitProcessingState() { + request_ = response_creator_->createNewHttpRequest(); + parser_.reset(new HttpRequestParser(*request_)); + parser_->initModel(); + setupIdleTimer(); +} + +void +HttpConnection::setupRequestTimer() { + // Pass raw pointer rather than shared_ptr to this object, + // because IntervalTimer already passes shared pointer to the + // IntervalTimerImpl to make sure that the callback remains + // valid. + if (!request_timer_setup_) { + request_timer_setup_ = true; + request_timer_.setup(boost::bind(&HttpConnection::requestTimeoutCallback, + this), + request_timeout_, IntervalTimer::ONE_SHOT); + } +} + +void +HttpConnection::setupIdleTimer() { + request_timer_setup_ = false; + request_timer_.setup(boost::bind(&HttpConnection::idleTimeoutCallback, + this), + idle_timeout_, IntervalTimer::ONE_SHOT); +} + void HttpConnection::requestTimeoutCallback() { LOG_DEBUG(http_logger, isc::log::DBGLVL_TRACE_DETAIL, @@ -256,6 +298,14 @@ HttpConnection::requestTimeoutCallback() { asyncSendResponse(response); } +void +HttpConnection::idleTimeoutCallback() { + LOG_DEBUG(http_logger, isc::log::DBGLVL_TRACE_DETAIL, + HTTP_IDLE_CONNECTION_TIMEOUT_OCCURRED) + .arg(getRemoteEndpointAddressAsText()); + stopThisConnection(); +} + std::string HttpConnection::getRemoteEndpointAddressAsText() const { try { diff --git a/src/lib/http/connection.h b/src/lib/http/connection.h index 6305d55159..fd387244e9 100644 --- a/src/lib/http/connection.h +++ b/src/lib/http/connection.h @@ -92,12 +92,15 @@ public: /// 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. HttpConnection(asiolink::IOService& io_service, HttpAcceptor& acceptor, HttpConnectionPool& connection_pool, const HttpResponseCreatorPtr& response_creator, const HttpAcceptorCallback& callback, - const long request_timeout); + const long request_timeout, + const long idle_timeout); /// @brief Destructor. /// @@ -166,12 +169,28 @@ private: void socketWriteCallback(boost::system::error_code ec, size_t length); + /// @brief Reinitializes request processing state after sending a response. + /// + /// This method is only called for persistent connections, when the response + /// to a previous command has been sent. It initializes the state machine to + /// be able to process the next request. It also sets the persistent connection + /// idle timer to monitor the connection timeout. + void reinitProcessingState(); + + /// @brief Reset timer for detecting request timeouts. + void setupRequestTimer(); + + /// @brief Reset timer for detecing idle timeout in persistent connections. + void setupIdleTimer(); + /// @brief Callback invoked when the HTTP Request Timeout occurs. /// /// This callback creates HTTP response with Request Timeout error code /// and sends it to the client. void requestTimeoutCallback(); + void idleTimeoutCallback(); + /// @brief Stops current connection. void stopThisConnection(); @@ -181,9 +200,15 @@ private: /// @brief Timer used to detect Request Timeout. asiolink::IntervalTimer request_timer_; + bool request_timer_setup_; + /// @brief Configured Request Timeout in milliseconds. long request_timeout_; + /// @brief Timeout after which the persistent HTTP connection is closed + /// by the server. + long idle_timeout_; + /// @brief Socket used by this connection. asiolink::TCPSocket socket_; diff --git a/src/lib/http/http_header.cc b/src/lib/http/http_header.cc new file mode 100644 index 0000000000..3e62cc14a8 --- /dev/null +++ b/src/lib/http/http_header.cc @@ -0,0 +1,54 @@ +// 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 + +namespace isc { +namespace http { + +HttpHeader::HttpHeader(const std::string& header_name, + const std::string& header_value) + : header_name_(header_name), header_value_(header_value) { +} + +uint64_t +HttpHeader::getUint64Value() const { + try { + return (boost::lexical_cast(header_value_)); + + } catch (const boost::bad_lexical_cast& ex) { + isc_throw(BadValue, header_name_ << " HTTP header value " + << header_value_ << " is not a valid number"); + } +} + +std::string +HttpHeader::getLowerCaseName() const { + std::string ln = header_name_; + util::str::lowercase(ln); + return (ln); +} + +std::string +HttpHeader::getLowerCaseValue() const { + std::string lc = header_value_; + util::str::lowercase(lc); + return (lc); +} + +bool +HttpHeader::isValueEqual(const std::string& v) const { + std::string lcv = v; + util::str::lowercase(lcv); + return (lcv == getLowerCaseValue()); +} + + +} // end of namespace isc::http +} // end of namespace isc diff --git a/src/lib/http/http_header.h b/src/lib/http/http_header.h new file mode 100644 index 0000000000..975ad35efe --- /dev/null +++ b/src/lib/http/http_header.h @@ -0,0 +1,70 @@ +// 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 HTTP_HEADER_H +#define HTTP_HEADER_H + +#include +#include + +namespace isc { +namespace http { + +/// @brief Represents HTTP header including a header name and value. +/// +/// It includes methods for retrieving header name and value in lower case +/// and for case insensitive comparison of header values. +class HttpHeader { +public: + + /// @brief Constructor. + /// + /// @param header_name Header name. + /// @param header_value Header value. + explicit HttpHeader(const std::string& header_name, + const std::string& header_value = ""); + + /// @brief Returns header name. + std::string getName() const { + return (header_name_); + } + + /// @brief Returns header value. + std::string getValue() const { + return (header_value_); + } + + /// @brief Returns header value as unsigned integer. + /// + /// @throw BadValue if the header value is not a valid number. + uint64_t getUint64Value() const; + + /// @brief Returns lower case header name. + std::string getLowerCaseName() const; + + /// @brief Returns lower case header value. + std::string getLowerCaseValue() const; + + /// @brief Case insensitive comparison of header value. + /// + /// @param v Value to be compared. + /// + /// @return true if header value is equal, false otherwise. + bool isValueEqual(const std::string& v) const; + +private: + + std::string header_name_; ///< Header name. + std::string header_value_; ///< Header value. +}; + +/// @brief Pointer to the @c HttpHeader class. +typedef boost::shared_ptr HttpHeaderPtr; + +} // end of namespace isc::http +} // end of namespace isc + +#endif // HTTP_HEADER_H diff --git a/src/lib/http/http_messages.mes b/src/lib/http/http_messages.mes index ad317fbf0b..414eab53df 100644 --- a/src/lib/http/http_messages.mes +++ b/src/lib/http/http_messages.mes @@ -22,6 +22,10 @@ of the request. The first argument specifies the amount of received data. The second argument specifies an address of the remote endpoint which produced the data. +% HTTP_IDLE_CONNECTION_TIMEOUT_OCCURRED closing persistent connection with %1 as a result of a timeout +This debug message is issued when the persistent HTTP connection is being +closed as a result of being idle. + % HTTP_REQUEST_RECEIVED received HTTP request from %1 This debug message is issued when the server finished receiving a HTTP request from the remote endpoint. The address of the remote endpoint is diff --git a/src/lib/http/http_types.h b/src/lib/http/http_types.h index 285096643c..68d1c0952b 100644 --- a/src/lib/http/http_types.h +++ b/src/lib/http/http_types.h @@ -44,6 +44,30 @@ struct HttpVersion { bool operator!=(const HttpVersion& rhs) const { return (!operator==(rhs)); } + + /// @name Methods returning @c HttpVersion object encapsulating typical + /// HTTP version numbers. + //@{ + + /// @brief HTTP version 1.0. + static const HttpVersion& HTTP_10() { + static HttpVersion ver(1, 0); + return (ver); + }; + + /// @brief HTTP version 1.1. + static const HttpVersion& HTTP_11() { + static HttpVersion ver(1, 1); + return (ver); + } + + /// @brief HTTP version 2.0. + static const HttpVersion& HTTP_20() { + static HttpVersion ver(2, 0); + return (ver); + } + + //@} }; } // end of namespace isc::http diff --git a/src/lib/http/listener.cc b/src/lib/http/listener.cc index b0e9d2c061..643376942f 100644 --- a/src/lib/http/listener.cc +++ b/src/lib/http/listener.cc @@ -37,6 +37,8 @@ public: /// 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. @@ -44,7 +46,8 @@ public: const asiolink::IOAddress& server_address, const unsigned short server_port, const HttpResponseCreatorFactoryPtr& creator_factory, - const long request_timeout); + const long request_timeout, + const long idle_timeout); /// @brief Returns reference to the current listener endpoint. const TCPEndpoint& getEndpoint() const; @@ -97,16 +100,21 @@ private: /// @brief Timeout for HTTP Request Timeout desired. long request_timeout_; + + /// @brief Timeout after which idle persistent connection is closed by + /// the server. + long idle_timeout_; }; HttpListenerImpl::HttpListenerImpl(IOService& io_service, const asiolink::IOAddress& server_address, const unsigned short server_port, const HttpResponseCreatorFactoryPtr& creator_factory, - const long request_timeout) + const long request_timeout, + const long idle_timeout) : io_service_(io_service), acceptor_(io_service), endpoint_(), creator_factory_(creator_factory), - request_timeout_(request_timeout) { + request_timeout_(request_timeout), idle_timeout_(idle_timeout) { // Try creating an endpoint. This may cause exceptions. try { endpoint_.reset(new TCPEndpoint(server_address, server_port)); @@ -127,6 +135,12 @@ HttpListenerImpl::HttpListenerImpl(IOService& io_service, isc_throw(HttpListenerError, "Invalid desired HTTP request timeout " << request_timeout_); } + + // Idle persistent connection timeout is signed and must be greater than 0. + if (idle_timeout_ <= 0) { + isc_throw(HttpListenerError, "Invalid desired HTTP idle persistent connection" + " timeout " << idle_timeout_); + } } const TCPEndpoint& @@ -169,7 +183,8 @@ HttpListenerImpl::accept() { connections_, response_creator, acceptor_callback, - request_timeout_)); + request_timeout_, + idle_timeout_)); // Add this new connection to the pool. connections_.start(conn); } @@ -185,9 +200,11 @@ HttpListener::HttpListener(IOService& io_service, const asiolink::IOAddress& server_address, const unsigned short server_port, const HttpResponseCreatorFactoryPtr& creator_factory, - const long request_timeout) + const HttpListener::RequestTimeout& request_timeout, + const HttpListener::IdleTimeout& idle_timeout) : impl_(new HttpListenerImpl(io_service, server_address, server_port, - creator_factory, request_timeout)) { + creator_factory, request_timeout.value_, + idle_timeout.value_)) { } HttpListener::~HttpListener() { diff --git a/src/lib/http/listener.h b/src/lib/http/listener.h index 1659d8868b..7c41bc2007 100644 --- a/src/lib/http/listener.h +++ b/src/lib/http/listener.h @@ -51,6 +51,28 @@ class HttpListenerImpl; class HttpListener { public: + /// @brief HTTP request timeout value. + struct RequestTimeout { + /// @brief Constructor. + /// + /// @param value Request timeout value in milliseconds. + explicit RequestTimeout(long value) + : value_(value) { + } + long value_; ///< Request timeout value specified. + }; + + /// @brief Idle connection timeout. + struct IdleTimeout { + /// @brief Constructor. + /// + /// @param value Connection idle timeout value in milliseconds. + explicit IdleTimeout(long value) + : value_(value) { + } + long value_; ///< Connection idle timeout value specified. + }; + /// @brief Constructor. /// /// This constructor creates new server endpoint using the specified IP @@ -67,6 +89,8 @@ public: /// 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. @@ -74,7 +98,8 @@ public: const asiolink::IOAddress& server_address, const unsigned short server_port, const HttpResponseCreatorFactoryPtr& creator_factory, - const long request_timeout); + const RequestTimeout& request_timeout, + const IdleTimeout& idle_timeout); /// @brief Destructor. /// diff --git a/src/lib/http/request.cc b/src/lib/http/request.cc index 206ab332fe..74b5d18759 100644 --- a/src/lib/http/request.cc +++ b/src/lib/http/request.cc @@ -34,13 +34,15 @@ void HttpRequest::requireHeader(const std::string& header_name) { // Empty value denotes that the header is required but no specific // value is expected. - required_headers_[header_name] = ""; + HttpHeaderPtr hdr(new HttpHeader(header_name)); + required_headers_[hdr->getLowerCaseName()] = hdr; } void HttpRequest::requireHeaderValue(const std::string& header_name, const std::string& header_value) { - required_headers_[header_name] = header_value; + HttpHeaderPtr hdr(new HttpHeader(header_name, header_value)); + required_headers_[hdr->getLowerCaseName()] = hdr; } bool @@ -48,7 +50,9 @@ HttpRequest::requiresBody() const { // If Content-Length is required the body must exist too. There may // be probably some cases when Content-Length is not provided but // the body is provided. But, probably not in our use cases. - return (required_headers_.find("Content-Length") != required_headers_.end()); + // Use lower case header name because this is how it is indexed in + // the storage. + return (required_headers_.find("content-length") != required_headers_.end()); } void @@ -79,7 +83,8 @@ HttpRequest::create() { for (auto header = context_->headers_.begin(); header != context_->headers_.end(); ++header) { - headers_[header->name_] = header->value_; + HttpHeaderPtr hdr(new HttpHeader(header->name_, header->value_)); + headers_[hdr->getLowerCaseName()] = hdr; } // Iterate over required headers and check that they exist @@ -91,13 +96,13 @@ HttpRequest::create() { if (header == headers_.end()) { isc_throw(BadValue, "required header " << req_header->first << " not found in the HTTP request"); - } else if (!req_header->second.empty() && - header->second != req_header->second) { + } else if (!req_header->second->getValue().empty() && + !header->second->isValueEqual(req_header->second->getValue())) { // If specific value is required for the header, check // that the value in the HTTP request matches it. isc_throw(BadValue, "required header's " << header->first - << " value is " << req_header->second - << ", but " << header->second << " was found"); + << " value is " << req_header->second->getValue() + << ", but " << header->second->getValue() << " was found"); } } @@ -151,31 +156,47 @@ HttpRequest::getHttpVersion() const { context_->http_version_minor_)); } -std::string -HttpRequest::getHeaderValue(const std::string& header) const { +HttpHeaderPtr +HttpRequest::getHeader(const std::string& header_name) const { + HttpHeaderPtr http_header = getHeaderSafe(header_name); + + // No such header. + if (!http_header) { + isc_throw(HttpRequestNonExistingHeader, header_name << " HTTP header" + " not found in the request"); + } + + // Header found. + return (http_header); +} + +HttpHeaderPtr +HttpRequest::getHeaderSafe(const std::string& header_name) const { checkCreated(); - auto header_it = headers_.find(header); + HttpHeader hdr(header_name); + auto header_it = headers_.find(hdr.getLowerCaseName()); if (header_it != headers_.end()) { return (header_it->second); } - // No such header. - isc_throw(HttpRequestNonExistingHeader, header << " HTTP header" - " not found in the request"); + + // Header not found. Return null pointer. + return (HttpHeaderPtr()); +} + +std::string +HttpRequest::getHeaderValue(const std::string& header_name) const { + return (getHeader(header_name)->getValue()); } uint64_t -HttpRequest::getHeaderValueAsUint64(const std::string& header) const { - // This will throw an exception if the header doesn't exist. - std::string header_value = getHeaderValue(header); - +HttpRequest::getHeaderValueAsUint64(const std::string& header_name) const { try { - return (boost::lexical_cast(header_value)); + return (getHeader(header_name)->getUint64Value()); - } catch (const boost::bad_lexical_cast& ex) { + } catch (const std::exception& ex) { // The specified header does exist, but the value is not a number. - isc_throw(HttpRequestError, header << " HTTP header value " - << header_value << " is not a valid number"); + isc_throw(HttpRequestError, ex.what()); } } @@ -185,6 +206,20 @@ HttpRequest::getBody() const { return (context_->body_); } +bool +HttpRequest::isPersistent() const { + HttpHeaderPtr conn = getHeaderSafe("connection"); + std::string conn_value; + if (conn) { + conn_value = conn->getLowerCaseValue(); + } + + HttpVersion ver = getHttpVersion(); + + return (((ver == HttpVersion::HTTP_10()) && (conn_value == "keep-alive")) || + ((HttpVersion::HTTP_10() < ver) && (conn_value.empty() || (conn_value != "close")))); +} + void HttpRequest::checkCreated() const { if (!created_) { diff --git a/src/lib/http/request.h b/src/lib/http/request.h index 07cd29c7e3..afd13c7514 100644 --- a/src/lib/http/request.h +++ b/src/lib/http/request.h @@ -8,6 +8,7 @@ #define HTTP_REQUEST_H #include +#include #include #include #include @@ -179,20 +180,43 @@ public: /// @brief Returns HTTP version number (major and minor). HttpVersion getHttpVersion() const; + /// @brief Returns object encapsulating HTTP header. + /// + /// @param header_name HTTP header name. + /// + /// @return Non-null pointer to the header. + /// @throw HttpRequestNonExistingHeader if header with the specified name + /// doesn't exist. + /// @throw HttpRequestError if the request hasn't been created. + HttpHeaderPtr getHeader(const std::string& header_name) const; + + /// @brief Returns object encapsulating HTTP header. + /// + /// This variant doesn't throw an exception if the header doesn't exist. + /// It will throw if the request hasn't been created using @c create() + /// method. + /// + /// @param header_name HTTP header name. + /// + /// @return Pointer to the specified header, or null if such header doesn't + /// exist. + /// @throw HttpRequestError if the request hasn't been created. + HttpHeaderPtr getHeaderSafe(const std::string& header_name) const; + /// @brief Returns a value of the specified HTTP header. /// - /// @param header Name of the HTTP header. + /// @param header_name Name of the HTTP header. /// /// @throw HttpRequestError if the header doesn't exist. - std::string getHeaderValue(const std::string& header) const; + std::string getHeaderValue(const std::string& header_name) const; /// @brief Returns a value of the specified HTTP header as number. /// - /// @param header Name of the HTTP header. + /// @param header_name Name of the HTTP header. /// /// @throw HttpRequestError if the header doesn't exist or if the /// header value is not number. - uint64_t getHeaderValueAsUint64(const std::string& header) const; + uint64_t getHeaderValueAsUint64(const std::string& header_name) const; /// @brief Returns HTTP message body as string. std::string getBody() const; @@ -207,6 +231,17 @@ public: return (finalized_); } + /// @brief Checks if the client has requested persistent connection. + /// + /// For the HTTP/1.0 case, the connection is persistent if the client has + /// included Connection: keep-alive header. For the HTTP/1.1 case, the + /// connection is assumed to be persistent unless Connection: close header + /// has been included. + /// + /// @return true if the client has requested persistent connection, false + /// otherwise. + bool isPersistent() const; + //@} protected: @@ -264,14 +299,17 @@ protected: /// If the set is empty, all versions are allowed. std::set required_versions_; + /// @brief Map of HTTP headers indexed by lower case header names. + typedef std::map HttpHeaderMap; + /// @brief Map holding required HTTP headers. /// - /// The key of this map specifies the HTTP header name. The value - /// specifies the HTTP header value. If the value is empty, the - /// header is required but the value of the header is not checked. - /// If the value is non-empty, the value in the HTTP request must - /// be equal to the value in the map. - std::map required_headers_; + /// The key of this map specifies the lower case HTTP header name. + /// If the value of the HTTP header is empty, the header is required + /// but the value of the header is not checked. If the value is + /// non-empty, the value in the HTTP request must be equal (case + /// insensitive) to the value in the map. + HttpHeaderMap required_headers_; /// @brief Flag indicating whether @ref create was called. bool created_; @@ -283,7 +321,7 @@ protected: Method method_; /// @brief Parsed HTTP headers. - std::map headers_; + HttpHeaderMap headers_; /// @brief Pointer to the @ref HttpRequestContext holding parsed /// data. diff --git a/src/lib/http/tests/Makefile.am b/src/lib/http/tests/Makefile.am index d34853e118..a9c637deea 100644 --- a/src/lib/http/tests/Makefile.am +++ b/src/lib/http/tests/Makefile.am @@ -22,6 +22,7 @@ TESTS += libhttp_unittests libhttp_unittests_SOURCES = connection_pool_unittests.cc libhttp_unittests_SOURCES += date_time_unittests.cc +libhttp_unittests_SOURCES += http_header_unittests.cc libhttp_unittests_SOURCES += listener_unittests.cc libhttp_unittests_SOURCES += post_request_unittests.cc libhttp_unittests_SOURCES += post_request_json_unittests.cc diff --git a/src/lib/http/tests/connection_pool_unittests.cc b/src/lib/http/tests/connection_pool_unittests.cc index 3cf2797e20..42fda5ff22 100644 --- a/src/lib/http/tests/connection_pool_unittests.cc +++ b/src/lib/http/tests/connection_pool_unittests.cc @@ -30,6 +30,12 @@ typedef TestHttpResponseBase Response; /// @brief Pointer to test HTTP response. typedef boost::shared_ptr ResponsePtr; +/// @brief Request timeout used in tests. +const long CONN_REQUEST_TIMEOUT = 1000; + +/// @brief Idle connecion timeout used in tests. +const long CONN_IDLE_TIMEOUT = 1000; + /// @brief Implementation of the @ref HttpResponseCreator. class TestHttpResponseCreator : public HttpResponseCreator { public: @@ -114,12 +120,14 @@ TEST_F(HttpConnectionPoolTest, startStop) { connection_pool_, response_creator_, HttpAcceptorCallback(), - 1000)); + CONN_REQUEST_TIMEOUT, + CONN_IDLE_TIMEOUT)); HttpConnectionPtr conn2(new HttpConnection(io_service_, acceptor_, connection_pool_, response_creator_, HttpAcceptorCallback(), - 1000)); + CONN_REQUEST_TIMEOUT, + CONN_IDLE_TIMEOUT)); // The pool should be initially empty. TestHttpConnectionPool pool; ASSERT_TRUE(pool.connections_.empty()); @@ -152,12 +160,14 @@ TEST_F(HttpConnectionPoolTest, stopAll) { connection_pool_, response_creator_, HttpAcceptorCallback(), - 1000)); + CONN_REQUEST_TIMEOUT, + CONN_IDLE_TIMEOUT)); HttpConnectionPtr conn2(new HttpConnection(io_service_, acceptor_, connection_pool_, response_creator_, HttpAcceptorCallback(), - 1000)); + CONN_REQUEST_TIMEOUT, + CONN_IDLE_TIMEOUT)); TestHttpConnectionPool pool; ASSERT_NO_THROW(pool.start(conn1)); ASSERT_NO_THROW(pool.start(conn2)); @@ -176,12 +186,14 @@ TEST_F(HttpConnectionPoolTest, stopInvalid) { connection_pool_, response_creator_, HttpAcceptorCallback(), - 1000)); + CONN_REQUEST_TIMEOUT, + CONN_IDLE_TIMEOUT)); HttpConnectionPtr conn2(new HttpConnection(io_service_, acceptor_, connection_pool_, response_creator_, HttpAcceptorCallback(), - 1000)); + CONN_REQUEST_TIMEOUT, + CONN_IDLE_TIMEOUT)); TestHttpConnectionPool pool; ASSERT_NO_THROW(pool.start(conn1)); ASSERT_NO_THROW(pool.stop(conn2)); diff --git a/src/lib/http/tests/http_header_unittests.cc b/src/lib/http/tests/http_header_unittests.cc new file mode 100644 index 0000000000..df9d5bbfbb --- /dev/null +++ b/src/lib/http/tests/http_header_unittests.cc @@ -0,0 +1,54 @@ +// 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 + +using namespace isc; +using namespace isc::http; + +namespace { + +// Test that HTTP header can be created. +TEST(HttpHeader, create) { + HttpHeader hdr("Content-Type", "application/json"); + EXPECT_EQ("Content-Type", hdr.getName()); + EXPECT_EQ("application/json", hdr.getValue()); +} + +// Test that the numeric value can be retrieved from a header and that +// an exception is thrown if the header value is not a valid number. +TEST(HttpHeader, getUint64Value) { + HttpHeader hdr64("Content-Length", "64"); + EXPECT_EQ(64, hdr64.getUint64Value()); + + HttpHeader hdr_foo("Content-Length", "foo"); + EXPECT_THROW(hdr_foo.getUint64Value(), isc::BadValue); +} + +// Test that header name can be retrieved in lower case. +TEST(HttpHeader, getLowerCaseName) { + HttpHeader hdr("ConnectioN", "Keep-Alive"); + EXPECT_EQ("connection", hdr.getLowerCaseName()); +} + +// Test that header value can be retrieved in lower case. +TEST(HttpHeader, getLowerCaseValue) { + HttpHeader hdr("Connection", "Keep-Alive"); + EXPECT_EQ("keep-alive", hdr.getLowerCaseValue()); +} + +// Test that header value comparison is case insensitive. +TEST(HttpHeader, equalsCaseInsensitive) { + HttpHeader hdr("Connection", "KeEp-ALIve"); + EXPECT_TRUE(hdr.isValueEqual("keep-alive")); + EXPECT_TRUE(hdr.isValueEqual("KEEP-ALIVE")); + EXPECT_TRUE(hdr.isValueEqual("kEeP-AlIvE")); +} + +} // end of anonymous namespace diff --git a/src/lib/http/tests/listener_unittests.cc b/src/lib/http/tests/listener_unittests.cc index 3b6aa025ce..df3812f894 100644 --- a/src/lib/http/tests/listener_unittests.cc +++ b/src/lib/http/tests/listener_unittests.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +19,7 @@ #include #include #include +#include #include using namespace boost::asio::ip; @@ -36,6 +38,9 @@ const unsigned short SERVER_PORT = 18123; /// @brief Request Timeout used in most of the tests (ms). const long REQUEST_TIMEOUT = 10000; +/// @brief Persistent connection idle timeout used in most of the tests (ms). +const long IDLE_TIMEOUT = 10000; + /// @brief Test timeout (ms). const long TEST_TIMEOUT = 10000; @@ -244,6 +249,36 @@ public: }); } + /// @brief Checks if the TCP connection is still open. + /// + /// Tests the TCP connection by trying to read from the socket. + /// + /// @return true if the TCP connection is open. + bool isConnectionAlive() { + // Remember the current non blocking setting. + const bool non_blocking_orig = socket_.non_blocking(); + // Set the socket to non blocking mode. We're going to test if the socket + // returns would_block status on the attempt to read from it. + socket_.non_blocking(true); + + // We need to provide a buffer for a call to read. + char data[2]; + boost::system::error_code ec; + boost::asio::read(socket_, boost::asio::buffer(data, sizeof(data)), ec); + + // Revert the original non_blocking flag on the socket. + socket_.non_blocking(non_blocking_orig); + + // If the connection is alive we'd typically get would_block status code. + // If there are any data that haven't been read we may also get success + // status. We're guessing that try_again may also be returned by some + // implementations in some situations. Any other error code indicates a + // problem with the connection so we assume that the connection has been + // closed. + return (!ec || (ec.value() == boost::asio::error::try_again) || + (ec.value() == boost::asio::error::would_block)); + } + /// @brief Close connection. void close() { socket_.close(); @@ -280,8 +315,8 @@ public: /// Starts test timer which detects timeouts. HttpListenerTest() : io_service_(), factory_(new TestHttpResponseCreatorFactory()), - test_timer_(io_service_), clients_() { - test_timer_.setup(boost::bind(&HttpListenerTest::timeoutHandler, this), + test_timer_(io_service_), run_io_service_timer_(io_service_), clients_() { + test_timer_.setup(boost::bind(&HttpListenerTest::timeoutHandler, this, true), TEST_TIMEOUT, IntervalTimer::ONE_SHOT); } @@ -299,6 +334,8 @@ public: /// /// This method creates HttpClient instance and retains it in the clients_ /// list. + /// + /// @param request String containing the HTTP request to be sent. void startRequest(const std::string& request) { HttpClientPtr client(new HttpClient(io_service_)); clients_.push_back(client); @@ -308,11 +345,45 @@ public: /// @brief Callback function invoke upon test timeout. /// /// It stops the IO service and reports test timeout. - void timeoutHandler() { - ADD_FAILURE() << "Timeout occurred while running the test!"; + /// + /// @param fail_on_timeout Specifies if test failure should be reported. + void timeoutHandler(const bool fail_on_timeout) { + if (fail_on_timeout) { + ADD_FAILURE() << "Timeout occurred while running the test!"; + } io_service_.stop(); } + /// @brief Runs IO service with optional timeout. + /// + /// @param timeout Optional value specifying for how long the io service + /// should be ran. + void runIOService(long timeout = 0) { + if (timeout > 0) { + run_io_service_timer_.setup(boost::bind(&HttpListenerTest::timeoutHandler, + this, false), + timeout, IntervalTimer::ONE_SHOT); + } + io_service_.run(); + io_service_.get_io_service().reset(); + io_service_.poll(); + } + + /// @brief Returns HTTP OK response expected by unit tests. + /// + /// @param http_version HTTP version. + /// + /// @return HTTP OK response expected by unit tests. + std::string httpOk(const HttpVersion& http_version) { + std::ostringstream s; + s << "HTTP/" << http_version.major_ << "." << http_version.minor_ << " 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"; + return (s.str()); + } + /// @brief IO service used in the tests. IOService io_service_; @@ -322,6 +393,10 @@ public: /// @brief Asynchronous timer service to detect timeouts. IntervalTimer test_timer_; + /// @brief Asynchronous timer for running IO service for a specified amount + /// of time. + IntervalTimer run_io_service_timer_; + /// @brief List of client connections. std::list clients_; }; @@ -335,21 +410,282 @@ TEST_F(HttpListenerTest, listen) { "{ }"; HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, - factory_, REQUEST_TIMEOUT); + factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT)); ASSERT_NO_THROW(listener.start()); ASSERT_EQ(SERVER_ADDRESS, listener.getLocalAddress().toText()); ASSERT_EQ(SERVER_PORT, listener.getLocalPort()); ASSERT_NO_THROW(startRequest(request)); - ASSERT_NO_THROW(io_service_.run()); + ASSERT_NO_THROW(runIOService()); ASSERT_EQ(1, clients_.size()); HttpClientPtr client = *clients_.begin(); ASSERT_TRUE(client); - EXPECT_EQ("HTTP/1.1 200 OK\r\n" - "Content-Length: 0\r\n" + EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse()); + + listener.stop(); + io_service_.poll(); +} + +// This test verifies that persistent HTTP connection can be established when +// "Conection: Keep-Alive" header value is specified. +TEST_F(HttpListenerTest, keepAlive) { + + // The first request contains the keep-alive header which instructs the server + // to maintain the TCP connection after sending a response. + std::string request = "POST /foo/bar HTTP/1.0\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n" + "Connection: Keep-Alive\r\n\r\n" + "{ }"; + + HttpListener 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 with the keep-alive header. + ASSERT_NO_THROW(startRequest(request)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + HttpClientPtr client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ(httpOk(HttpVersion::HTTP_10()), client->getResponse()); + + // We have sent keep-alive header so we expect that the connection with + // the server remains active. + ASSERT_TRUE(client->isConnectionAlive()); + + // Test that we can send another request via the same connection. This time + // it lacks the keep-alive header, so the server should close the connection + // after sending the response. + request = "POST /foo/bar HTTP/1.0\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n\r\n" + "{ }"; + + // Send request reusing the existing connection. + ASSERT_NO_THROW(client->sendRequest(request)); + ASSERT_NO_THROW(runIOService()); + EXPECT_EQ(httpOk(HttpVersion::HTTP_10()), client->getResponse()); + + // Connection should have been closed by the server. + EXPECT_FALSE(client->isConnectionAlive()); + + listener.stop(); + io_service_.poll(); +} + +// This test verifies that persistent HTTP connection is established by default +// when HTTP/1.1 is in use. +TEST_F(HttpListenerTest, persistentConnection) { + + // 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" + "{ }"; + + HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, + factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT)); + + ASSERT_NO_THROW(listener.start()); + + // Send the first request. + ASSERT_NO_THROW(startRequest(request)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + HttpClientPtr client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse()); + + // HTTP/1.1 connection is persistent by default. + ASSERT_TRUE(client->isConnectionAlive()); + + // Test that we can send another request via the same connection. This time + // it includes the "Connection: close" header which instructs the server to + // close the connection after responding. + request = "POST /foo/bar HTTP/1.1\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n" + "Connection: close\r\n\r\n" + "{ }"; + + // Send request reusing the existing connection. + ASSERT_NO_THROW(client->sendRequest(request)); + ASSERT_NO_THROW(runIOService()); + EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse()); + + // Connection should have been closed by the server. + EXPECT_FALSE(client->isConnectionAlive()); + + listener.stop(); + io_service_.poll(); +} + +// This test verifies that "keep-alive" connection is closed by the server after +// an idle time. +TEST_F(HttpListenerTest, keepAliveTimeout) { + + // The first request contains the keep-alive header which instructs the server + // to maintain the TCP connection after sending a response. + std::string request = "POST /foo/bar HTTP/1.0\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n" + "Connection: Keep-Alive\r\n\r\n" + "{ }"; + + // Specify the idle timeout of 500ms. + HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, + factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(500)); + + ASSERT_NO_THROW(listener.start()); + + // Send the request with the keep-alive header. + ASSERT_NO_THROW(startRequest(request)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + HttpClientPtr client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ(httpOk(HttpVersion::HTTP_10()), client->getResponse()); + + // We have sent keep-alive header so we expect that the connection with + // the server remains active. + ASSERT_TRUE(client->isConnectionAlive()); + + // Run IO service for 1000ms. The idle time is set to 500ms, so the connection + // should be closed by the server while we wait here. + runIOService(1000); + + // Make sure the connection has been closed. + EXPECT_FALSE(client->isConnectionAlive()); + + // Check if we can re-establish the connection and send another request. + clients_.clear(); + request = "POST /foo/bar HTTP/1.0\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n\r\n" + "{ }"; + + ASSERT_NO_THROW(startRequest(request)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ(httpOk(HttpVersion::HTTP_10()), client->getResponse()); + + EXPECT_FALSE(client->isConnectionAlive()); + + listener.stop(); + io_service_.poll(); +} + +// This test verifies that persistent connection is closed by the server after +// an idle time. +TEST_F(HttpListenerTest, persistentConnectionTimeout) { + + // 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" + "{ }"; + + // Specify the idle timeout of 500ms. + HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, + factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(500)); + + ASSERT_NO_THROW(listener.start()); + + // Send the request. + ASSERT_NO_THROW(startRequest(request)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + HttpClientPtr client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse()); + + // The connection should remain active. + ASSERT_TRUE(client->isConnectionAlive()); + + // Run IO service for 1000ms. The idle time is set to 500ms, so the connection + // should be closed by the server while we wait here. + runIOService(1000); + + // Make sure the connection has been closed. + EXPECT_FALSE(client->isConnectionAlive()); + + // Check if we can re-establish the connection and send another request. + clients_.clear(); + request = "POST /foo/bar HTTP/1.1\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n" + "Connection: close\r\n\r\n" + "{ }"; + + ASSERT_NO_THROW(startRequest(request)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse()); + + EXPECT_FALSE(client->isConnectionAlive()); + + listener.stop(); + io_service_.poll(); +} + +// This test verifies that HTTP/1.1 connection remains open even if there is an +// error in the message body. +TEST_F(HttpListenerTest, persistentConnectionBadBody) { + + // 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: 12\r\n\r\n" + "{ \"a\": abc }"; + + HttpListener 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)); + ASSERT_NO_THROW(runIOService()); + ASSERT_EQ(1, clients_.size()); + HttpClientPtr client = *clients_.begin(); + ASSERT_TRUE(client); + EXPECT_EQ("HTTP/1.1 400 Bad Request\r\n" + "Content-Length: 40\r\n" "Content-Type: application/json\r\n" "Date: Tue, 19 Dec 2016 18:53:35 GMT\r\n" - "\r\n", + "\r\n" + "{ \"result\": 400, \"text\": \"Bad Request\" }", client->getResponse()); + + // The connection should remain active. + ASSERT_TRUE(client->isConnectionAlive()); + + // Make sure that we can send another request. This time we specify the + // "close" connection-token to force the connection to close. + request = "POST /foo/bar HTTP/1.1\r\n" + "Content-Type: application/json\r\n" + "Content-Length: 3\r\n" + "Connection: close\r\n\r\n" + "{ }"; + + // Send request reusing the existing connection. + ASSERT_NO_THROW(client->sendRequest(request)); + ASSERT_NO_THROW(runIOService()); + EXPECT_EQ(httpOk(HttpVersion::HTTP_11()), client->getResponse()); + + EXPECT_FALSE(client->isConnectionAlive()); + listener.stop(); io_service_.poll(); } @@ -357,7 +693,8 @@ TEST_F(HttpListenerTest, listen) { // This test verifies that the HTTP listener can't be started twice. TEST_F(HttpListenerTest, startTwice) { HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, - factory_, REQUEST_TIMEOUT); + factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT)); ASSERT_NO_THROW(listener.start()); EXPECT_THROW(listener.start(), HttpListenerError); } @@ -372,10 +709,11 @@ TEST_F(HttpListenerTest, badRequest) { "{ }"; HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, - factory_, REQUEST_TIMEOUT); + factory_, HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT)); ASSERT_NO_THROW(listener.start()); ASSERT_NO_THROW(startRequest(request)); - ASSERT_NO_THROW(io_service_.run()); + ASSERT_NO_THROW(runIOService()); ASSERT_EQ(1, clients_.size()); HttpClientPtr client = *clients_.begin(); ASSERT_TRUE(client); @@ -393,7 +731,8 @@ TEST_F(HttpListenerTest, badRequest) { TEST_F(HttpListenerTest, invalidFactory) { EXPECT_THROW(HttpListener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT, HttpResponseCreatorFactoryPtr(), - REQUEST_TIMEOUT), + HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT)), HttpListenerError); } @@ -401,7 +740,18 @@ TEST_F(HttpListenerTest, invalidFactory) { // Request Timeout. TEST_F(HttpListenerTest, invalidRequestTimeout) { EXPECT_THROW(HttpListener(io_service_, IOAddress(SERVER_ADDRESS), - SERVER_PORT, factory_, 0), + SERVER_PORT, factory_, HttpListener::RequestTimeout(0), + HttpListener::IdleTimeout(IDLE_TIMEOUT)), + HttpListenerError); +} + +// This test verifies that the timeout of 0 can't be specified for the +// idle persistent connection timeout. +TEST_F(HttpListenerTest, invalidIdleTimeout) { + EXPECT_THROW(HttpListener(io_service_, IOAddress(SERVER_ADDRESS), + SERVER_PORT, factory_, + HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(0)), HttpListenerError); } @@ -419,7 +769,9 @@ TEST_F(HttpListenerTest, addressInUse) { // Listener should report an error when we try to start it because another // acceptor is bound to that port and address. HttpListener listener(io_service_, IOAddress(SERVER_ADDRESS), - SERVER_PORT + 1, factory_, REQUEST_TIMEOUT); + SERVER_PORT + 1, factory_, + HttpListener::RequestTimeout(REQUEST_TIMEOUT), + HttpListener::IdleTimeout(IDLE_TIMEOUT)); EXPECT_THROW(listener.start(), HttpListenerError); } @@ -435,10 +787,11 @@ TEST_F(HttpListenerTest, requestTimeout) { // 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_, 1000); + factory_, HttpListener::RequestTimeout(1000), + HttpListener::IdleTimeout(IDLE_TIMEOUT)); ASSERT_NO_THROW(listener.start()); ASSERT_NO_THROW(startRequest(request)); - ASSERT_NO_THROW(io_service_.run()); + ASSERT_NO_THROW(runIOService()); ASSERT_EQ(1, clients_.size()); HttpClientPtr client = *clients_.begin(); ASSERT_TRUE(client); diff --git a/src/lib/http/tests/request_parser_unittests.cc b/src/lib/http/tests/request_parser_unittests.cc index 8860ff8760..049a1bd676 100644 --- a/src/lib/http/tests/request_parser_unittests.cc +++ b/src/lib/http/tests/request_parser_unittests.cc @@ -224,6 +224,22 @@ TEST_F(HttpRequestParserTest, getLowerCase) { EXPECT_EQ(1, request_.getHttpVersion().minor_); } +// This test verifies that headers are case insensitive. +TEST_F(HttpRequestParserTest, headersCaseInsensitive) { + std::string http_req = "get /foo/bar HTTP/1.1\r\n" + "Content-type: text/html\r\n" + "connection: keep-Alive\r\n\r\n"; + + ASSERT_NO_FATAL_FAILURE(doParse(http_req)); + + EXPECT_EQ(HttpRequest::Method::HTTP_GET, request_.getMethod()); + EXPECT_EQ("/foo/bar", request_.getUri()); + EXPECT_EQ("text/html", request_.getHeader("Content-Type")->getValue()); + EXPECT_EQ("keep-alive", request_.getHeader("Connection")->getLowerCaseValue()); + EXPECT_EQ(1, request_.getHttpVersion().major_); + EXPECT_EQ(1, request_.getHttpVersion().minor_); +} + // This test verifies that other value of the HTTP version can be // specified in the request. TEST_F(HttpRequestParserTest, http20) { diff --git a/src/lib/http/tests/request_unittests.cc b/src/lib/http/tests/request_unittests.cc index ae6bbf20f9..fc65b44372 100644 --- a/src/lib/http/tests/request_unittests.cc +++ b/src/lib/http/tests/request_unittests.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -18,7 +19,51 @@ using namespace isc::http::test; namespace { -typedef HttpRequestTestBase HttpRequestTest; +class HttpRequestTest : public HttpRequestTestBase { +public: + + /// @brief Tests connection persistence for the given HTTP version + /// and header value. + /// + /// This method creates a dummy HTTP request and sets the specified + /// version and header. Next, it returns the value if @c isPersistent + /// method for this request. The unit test verifies this value for + /// correctness. + /// + /// @param http_version HTTP version. + /// @param http_header HTTP header to be included in the request. If + /// the header has an empty value, it is not included. + /// + /// @return true if request indicates that connection is to be + /// persistent. + bool isPersistent(const HttpVersion& http_version, + const HttpHeader& http_header = HttpHeader("Connection")) { + try { + // We need to add some JSON body. + std::string json_body = "{ \"param1\": \"foo\" }"; + + // Set method, path, version and content length. + setContextBasics("POST", "/isc/org", http_version); + addHeaderToContext("Content-Length", json_body.length()); + + // If additional header has been specified (typically "Connection"), + // include it. + if (!http_header.getValue().empty()) { + addHeaderToContext(http_header.getName(), http_header.getValue()); + } + // Attach JSON body. + request_.context()->body_ = json_body; + request_.create(); + + } catch (...) { + ADD_FAILURE() << "failed to create HTTP request while testing" + " connection persistence"; + } + + return (request_.isPersistent()); + } + +}; TEST_F(HttpRequestTest, minimal) { setContextBasics("GET", "/isc/org", HttpVersion(1, 1)); @@ -155,4 +200,30 @@ TEST_F(HttpRequestTest, requiresBody) { EXPECT_TRUE(request_.requiresBody()); } +TEST_F(HttpRequestTest, isPersistentHttp10) { + // In HTTP 1.0 the connection is by default non-persistent. + EXPECT_FALSE(isPersistent(HttpVersion(1, 0))); +} + +TEST_F(HttpRequestTest, isPersistentHttp11) { + // In HTTP 1.1 the connection is by default persistent. + EXPECT_TRUE(isPersistent(HttpVersion(1, 1))); +} + +TEST_F(HttpRequestTest, isPersistentHttp10KeepAlive) { + // In HTTP 1.0 the client indicates that the connection is desired to be + // persistent by including "Connection: keep-alive" header. + EXPECT_TRUE( + isPersistent(HttpVersion(1, 0), HttpHeader("Connection", "Keep-alive")) + ); +} + +TEST_F(HttpRequestTest, isPersistentHttp11Close) { + // In HTTP 1.1 the client would include "Connection: close" header if it + // desires to close the connection. + EXPECT_FALSE( + isPersistent(HttpVersion(1, 1), HttpHeader("Connection", "close")) + ); +} + }