From 3029c65e5688255d56933cab0ae6a0a6b9f534a6 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 17 Oct 2015 21:05:18 +0200 Subject: [PATCH 01/50] [3927] spurious indents --- src/bin/dhcp4/json_config_parser.cc | 4 ++-- src/bin/dhcp6/json_config_parser.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index d62e03e39b..ba6f28c668 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -373,8 +373,8 @@ namespace dhcp { /// @return parser for specified global DHCPv4 parameter /// @throw NotImplemented if trying to create a parser for unknown /// config element - DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, - ConstElementPtr element) { +DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, + ConstElementPtr element) { DhcpConfigParser* parser = NULL; if ((config_id.compare("valid-lifetime") == 0) || (config_id.compare("renew-timer") == 0) || diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index cc269ae91c..c7bf0de63f 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -665,8 +665,8 @@ namespace dhcp { /// @return parser for specified global DHCPv6 parameter /// @throw NotImplemented if trying to create a parser for unknown config /// element - DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, - ConstElementPtr element) { +DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, + ConstElementPtr element) { DhcpConfigParser* parser = NULL; if ((config_id.compare("preferred-lifetime") == 0) || (config_id.compare("valid-lifetime") == 0) || From 75552a69de8ffee46eed528445e1107a738c3a47 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 17 Oct 2015 21:47:15 +0200 Subject: [PATCH 02/50] [3927] blank line --- src/lib/dhcpsrv/parsers/dhcp_parsers.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 30d530aab3..5eb5aa863f 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -745,6 +745,7 @@ private: /// Instance of option definition being created by this parser. OptionDefinitionPtr option_definition_; + /// Name of the space the option definition belongs to. std::string option_space_name_; From df49e49ac2f0dfbadccdf99733b0beafc7b9375f Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 18 Oct 2015 19:57:20 +0200 Subject: [PATCH 03/50] [3927] Simplified option-def tests --- src/bin/dhcp4/tests/config_parser_unittest.cc | 126 ++++------------ src/bin/dhcp6/tests/config_parser_unittest.cc | 136 ++++-------------- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 50 +++++-- 3 files changed, 88 insertions(+), 224 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 933146facc..b298ddca3c 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1330,10 +1330,7 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1372,10 +1369,8 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"record\"," - " \"array\": False," " \"record-types\": \"uint16, ipv4-address, ipv6-address, string\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1422,19 +1417,13 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo-2\"," " \"code\": 101," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1488,19 +1477,13 @@ TEST_F(Dhcp4ParserTest, optionDefDuplicate) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo-2\"," " \"code\": 100," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1529,9 +1512,7 @@ TEST_F(Dhcp4ParserTest, optionDefArray) { " \"code\": 100," " \"type\": \"uint32\"," " \"array\": True," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1571,8 +1552,6 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"sub-opts-space\"" " } ]" @@ -1613,10 +1592,7 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidName) { " \"name\": \"invalid%name\"," " \"code\": 100," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1640,10 +1616,7 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidType) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"sting\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1667,10 +1640,8 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidRecordType) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"record\"," - " \"array\": False," " \"record-types\": \"uint32,uint8,sting\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1694,8 +1665,6 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidEncapsulatedSpace) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"invalid%space%name\"" " } ]" @@ -1724,7 +1693,6 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulatedSpaceAndArray) { " \"code\": 100," " \"type\": \"uint32\"," " \"array\": True," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"valid-space-name\"" " } ]" @@ -1750,8 +1718,6 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulateOwnSpace) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"isc\"" " } ]" @@ -1781,10 +1747,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { " \"name\": \"foo\"," " \"code\": 109," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp4\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp4\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1818,10 +1781,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { " \"name\": \"routers\"," " \"code\": 3," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp4\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp4\"" " } ]" "}"; json = Element::fromJSON(config); @@ -1843,10 +1803,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { " \"name\": \"nis-server-addr\"," " \"code\": 65," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp4\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp4\"" " } ]" "}"; json = Element::fromJSON(config); @@ -1968,10 +1925,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { " \"name\": \"foo\"," " \"code\": 56," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," @@ -2048,19 +2002,13 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 1," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 2," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; @@ -2107,8 +2055,6 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { " \"name\": \"base-option\"," " \"code\": 222," " \"type\": \"uint8\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"dhcp4\"," " \"encapsulate\": \"isc\"" "}," @@ -2116,19 +2062,13 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 1," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 2," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," @@ -2625,19 +2565,13 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 1," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-encapsulated-options-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-encapsulated-options-space\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 2," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-encapsulated-options-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-encapsulated-options-space\"" " } ]" "}"; @@ -2688,19 +2622,13 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 1," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-encapsulated-options-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-encapsulated-options-space\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 2," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-encapsulated-options-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-encapsulated-options-space\"" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," @@ -2841,10 +2769,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsCsv) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-4491\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-4491\"" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," @@ -2927,10 +2852,7 @@ buildHooksLibrariesConfig(const std::vector& libraries) { " \"name\": \"foo\"," " \"code\": 56," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index f2e945191a..48e3d5ace8 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -205,10 +205,7 @@ public: " \"name\": \"bool-option\"," " \"code\": 1000," " \"type\": \"boolean\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp6\"" "} ]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ]," @@ -1572,10 +1569,7 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"ipv6-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1612,10 +1606,8 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"record\"," - " \"array\": False," " \"record-types\": \"uint16, ipv4-address, ipv6-address, string\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1661,19 +1653,13 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo-2\"," " \"code\": 101," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1725,19 +1711,13 @@ TEST_F(Dhcp6ParserTest, optionDefDuplicate) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo-2\"," " \"code\": 100," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1766,9 +1746,7 @@ TEST_F(Dhcp6ParserTest, optionDefArray) { " \"code\": 100," " \"type\": \"uint32\"," " \"array\": True," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1806,8 +1784,6 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"sub-opts-space\"" " } ]" @@ -1847,10 +1823,7 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidName) { " \"name\": \"invalid%name\"," " \"code\": 100," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1874,10 +1847,7 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidType) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"sting\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1901,10 +1871,8 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidRecordType) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"record\"," - " \"array\": False," " \"record-types\": \"uint32,uint8,sting\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -1928,8 +1896,6 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidEncapsulatedSpace) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"invalid%space%name\"" " } ]" @@ -1958,7 +1924,6 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulatedSpaceAndArray) { " \"code\": 100," " \"type\": \"uint32\"," " \"array\": True," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"valid-space-name\"" " } ]" @@ -1984,8 +1949,6 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulateOwnSpace) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"isc\"," " \"encapsulate\": \"isc\"" " } ]" @@ -2016,10 +1979,7 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp6\"" " } ]" "}"; ElementPtr json = Element::fromJSON(config); @@ -2053,10 +2013,7 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { " \"name\": \"foo\"," " \"code\": 3," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp6\"" " } ]" "}"; json = Element::fromJSON(config); @@ -2078,10 +2035,7 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { " \"name\": \"geolocation\"," " \"code\": 63," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp6\"" " } ]" "}"; json = Element::fromJSON(config); @@ -2212,10 +2166,7 @@ TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) { " \"name\": \"foo\"," " \"code\": 38," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ]," @@ -2293,19 +2244,13 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 110," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 111," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]" "}"; @@ -2353,8 +2298,6 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { " \"name\": \"base-option\"," " \"code\": 100," " \"type\": \"uint8\"," - " \"array\": False," - " \"record-types\": \"\"," " \"space\": \"dhcp6\"," " \"encapsulate\": \"isc\"" "}," @@ -2362,19 +2305,13 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 110," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 111," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ]," @@ -2811,10 +2748,7 @@ TEST_F(Dhcp6ParserTest, vendorOptionsCsv) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-4491\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-4491\"" " } ]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ]," @@ -2884,19 +2818,13 @@ TEST_F(Dhcp6ParserTest, DISABLED_stdOptionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 110," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-opts-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-opts-space\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 111," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-opts-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-opts-space\"" " } ]" "}"; @@ -2946,19 +2874,13 @@ TEST_F(Dhcp6ParserTest, DISABLED_stdOptionDataEncapsulate) { " \"name\": \"foo\"," " \"code\": 110," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-opts-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-opts-space\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 111," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-opts-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-opts-space\"" " } ]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ]," @@ -3064,19 +2986,13 @@ buildHooksLibrariesConfig(const std::vector& libraries) { " \"name\": \"foo\"," " \"code\": 110," " \"type\": \"uint32\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-opts-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-opts-space\"" " }," " {" " \"name\": \"foo2\"," " \"code\": 111," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-opts-space\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-opts-space\"" " } ]" "}"); diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index e85bd3787f..daa65f3cb4 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -458,10 +458,10 @@ public: std::string error_text_; }; -/// @brief Check Basic parsing of option definitions. +/// @brief Check basic parsing of option definitions. /// /// Note that this tests basic operation of the OptionDefinitionListParser and -/// OptionDefinitionParser. It uses a simple configuration consisting of one +/// OptionDefinitionParser. It uses a simple configuration consisting of /// one definition and verifies that it is parsed and committed to storage /// correctly. TEST_F(ParseConfigTest, basicOptionDefTest) { @@ -497,6 +497,39 @@ TEST_F(ParseConfigTest, basicOptionDefTest) { EXPECT_TRUE(def->getEncapsulatedSpace().empty()); } +/// @brief Check minimal parsing of option definitions. +/// +/// Same than basic but without optional parameters set to their default. +TEST_F(ParseConfigTest, minimalOptionDefTest) { + + // Configuration string. + std::string config = + "{ \"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 100," + " \"type\": \"ipv4-address\"," + " \"space\": \"isc\"" + " } ]" + "}"; + + // Verify that the configuration string parses. + int rcode = parseConfiguration(config); + ASSERT_TRUE(rcode == 0); + + + // Verify that the option definition can be retrieved. + OptionDefinitionPtr def = + CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100); + ASSERT_TRUE(def); + + // Verify that the option definition is correct. + EXPECT_EQ("foo", def->getName()); + EXPECT_EQ(100, def->getCode()); + EXPECT_FALSE(def->getArrayType()); + EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, def->getType()); + EXPECT_TRUE(def->getEncapsulatedSpace().empty()); +} + /// @brief Check Basic parsing of options. /// /// Note that this tests basic operation of the OptionDataListParser and @@ -511,10 +544,7 @@ TEST_F(ParseConfigTest, basicOptionDataTest) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"ipv4-address\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"isc\"," - " \"encapsulate\": \"\"" + " \"space\": \"isc\"" " } ], " " \"option-data\": [ {" " \"name\": \"foo\"," @@ -772,9 +802,7 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) { " \"code\": 2345," " \"type\": \"ipv6-address\"," " \"array\": True," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp6\"" " } ]," " \"option-data\": [ {" " \"name\": \"foo-name\"," @@ -800,9 +828,7 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) { " \"code\": 2345," " \"type\": \"ipv6-address\"," " \"array\": True," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"space\": \"dhcp6\"" " } ]," " \"option-data\": [ {" " \"code\": 2345," From 8e360e6613730ca2f0241462e9a7ed9c11be34ae Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 18 Oct 2015 21:39:57 +0200 Subject: [PATCH 04/50] [3927] Simplified option-data tests --- src/bin/dhcp4/tests/config_parser_unittest.cc | 65 +++-------------- src/bin/dhcp4/tests/decline_unittest.cc | 5 +- src/bin/dhcp4/tests/dora_unittest.cc | 45 +++--------- src/bin/dhcp4/tests/fqdn_unittest.cc | 10 +-- src/bin/dhcp4/tests/inform_unittest.cc | 40 +++-------- src/bin/dhcp4/tests/release_unittest.cc | 5 +- src/bin/dhcp6/tests/config_parser_unittest.cc | 70 ++++--------------- src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 22 ++---- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 38 +++++++++- 9 files changed, 86 insertions(+), 214 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index b298ddca3c..d1e59b204c 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1836,15 +1836,11 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"dhcp-message\"," - " \"space\": \"dhcp4\"," - " \"code\": 56," " \"data\": \"ABCDEF0105\"," " \"csv-format\": False" " }," " {" " \"name\": \"default-ip-ttl\"," - " \"space\": \"dhcp4\"," - " \"code\": 23," " \"data\": \"01\"," " \"csv-format\": False" " } ]," @@ -1909,17 +1905,13 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"dhcp-message\"," - " \"space\": \"dhcp4\"," - " \"code\": 56," " \"data\": \"ABCDEF0105\"," " \"csv-format\": False" " }," " {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 56," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -1987,16 +1979,12 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { "\"option-data\": [ {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 1," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"isc\"," - " \"code\": 2," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2032,24 +2020,17 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"base-option\"," - " \"space\": \"dhcp4\"," - " \"code\": 222," - " \"data\": \"11\"," - " \"csv-format\": True" + " \"data\": \"11\"" " }," " {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 1," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"isc\"," - " \"code\": 2," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"base-option\"," @@ -2120,8 +2101,6 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { "\"renew-timer\": 1000, " "\"option-data\": [ {" " \"name\": \"dhcp-message\"," - " \"space\": \"dhcp4\"," - " \"code\": 56," " \"data\": \"AB\"," " \"csv-format\": False" " } ]," @@ -2130,15 +2109,11 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { " \"subnet\": \"192.0.2.0/24\", " " \"option-data\": [ {" " \"name\": \"dhcp-message\"," - " \"space\": \"dhcp4\"," - " \"code\": 56," " \"data\": \"ABCDEF0105\"," " \"csv-format\": False" " }," " {" " \"name\": \"default-ip-ttl\"," - " \"space\": \"dhcp4\"," - " \"code\": 23," " \"data\": \"01\"," " \"csv-format\": False" " } ]" @@ -2277,8 +2252,6 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { " \"subnet\": \"192.0.2.0/24\", " " \"option-data\": [ {" " \"name\": \"dhcp-message\"," - " \"space\": \"dhcp4\"," - " \"code\": 56," " \"data\": \"0102030405060708090A\"," " \"csv-format\": False" " } ]" @@ -2288,8 +2261,6 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { " \"subnet\": \"192.0.3.0/24\", " " \"option-data\": [ {" " \"name\": \"default-ip-ttl\"," - " \"space\": \"dhcp4\"," - " \"code\": 23," " \"data\": \"FF\"," " \"csv-format\": False" " } ]" @@ -2550,16 +2521,12 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { "\"option-data\": [ {" " \"name\": \"foo\"," " \"space\": \"vendor-encapsulated-options-space\"," - " \"code\": 1," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"vendor-encapsulated-options-space\"," - " \"code\": 2," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2599,17 +2566,12 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"vendor-encapsulated-options\"," - " \"space\": \"dhcp4\"," - " \"code\": 43," - " \"data\": \"\"," " \"csv-format\": False" " }," " {" " \"name\": \"foo\"," " \"space\": \"vendor-encapsulated-options-space\"," - " \"code\": 1," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," @@ -2762,8 +2724,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsCsv) { " \"name\": \"foo\"," " \"space\": \"vendor-4491\"," " \"code\": 100," - " \"data\": \"this is a string vendor-opt\"," - " \"csv-format\": True" + " \"data\": \"this is a string vendor-opt\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2836,17 +2797,13 @@ buildHooksLibrariesConfig(const std::vector& libraries) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"dhcp-message\"," - " \"space\": \"dhcp4\"," - " \"code\": 56," " \"data\": \"ABCDEF0105\"," " \"csv-format\": False" " }," " {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 56," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," diff --git a/src/bin/dhcp4/tests/decline_unittest.cc b/src/bin/dhcp4/tests/decline_unittest.cc index d420749f20..f2faff9079 100644 --- a/src/bin/dhcp4/tests/decline_unittest.cc +++ b/src/bin/dhcp4/tests/decline_unittest.cc @@ -53,10 +53,7 @@ const char* DECLINE_CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " } ]" " } ]" "}" diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index 36f3090fc3..051bb43edf 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -77,31 +77,19 @@ const char* DORA_CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " }," " {" " \"name\": \"domain-name-servers\"," - " \"code\": 6," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " }," " {" " \"name\": \"log-servers\"," - " \"code\": 7," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"code\": 8," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " } ]" " } ]" "}", @@ -116,31 +104,19 @@ const char* DORA_CONFIGS[] = { " \"subnet\": \"192.0.2.0/24\", " " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"192.0.2.200,192.0.2.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"192.0.2.200,192.0.2.201\"" " }," " {" " \"name\": \"domain-name-servers\"," - " \"code\": 6," - " \"data\": \"192.0.2.202,192.0.2.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"192.0.2.202,192.0.2.203\"" " }," " {" " \"name\": \"log-servers\"," - " \"code\": 7," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"code\": 8," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " } ]" " } ]" "}", @@ -174,10 +150,7 @@ const char* DORA_CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " } ]" " } ]" "}", diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc index 52eb4af6f6..6acd7ee4b0 100644 --- a/src/bin/dhcp4/tests/fqdn_unittest.cc +++ b/src/bin/dhcp4/tests/fqdn_unittest.cc @@ -47,10 +47,7 @@ const char* CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " } ]," " \"reservations\": [" " {" @@ -74,10 +71,7 @@ const char* CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " } ]," " \"reservations\": [" " {" diff --git a/src/bin/dhcp4/tests/inform_unittest.cc b/src/bin/dhcp4/tests/inform_unittest.cc index e1fd87215a..ce5f604381 100644 --- a/src/bin/dhcp4/tests/inform_unittest.cc +++ b/src/bin/dhcp4/tests/inform_unittest.cc @@ -58,31 +58,19 @@ const char* INFORM_CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " }," " {" " \"name\": \"domain-name-servers\"," - " \"code\": 6," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " }," " {" " \"name\": \"log-servers\"," - " \"code\": 7," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"code\": 8," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " } ]" " } ]" "}", @@ -96,31 +84,19 @@ const char* INFORM_CONFIGS[] = { " \"subnet\": \"192.0.2.0/24\", " " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"192.0.2.200,192.0.2.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"192.0.2.200,192.0.2.201\"" " }," " {" " \"name\": \"domain-name-servers\"," - " \"code\": 6," - " \"data\": \"192.0.2.202,192.0.2.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"192.0.2.202,192.0.2.203\"" " }," " {" " \"name\": \"log-servers\"," - " \"code\": 7," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"code\": 8," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " } ]" " } ]" "}" diff --git a/src/bin/dhcp4/tests/release_unittest.cc b/src/bin/dhcp4/tests/release_unittest.cc index c1ac1fda9a..5b6985610f 100644 --- a/src/bin/dhcp4/tests/release_unittest.cc +++ b/src/bin/dhcp4/tests/release_unittest.cc @@ -53,10 +53,7 @@ const char* RELEASE_CONFIGS[] = { " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " } ]" " } ]" "}" diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 48e3d5ace8..130d0bace4 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -2068,17 +2068,12 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"subscriber-id\"," - " \"space\": \"dhcp6\"," - " \"code\": 38," " \"data\": \"ABCDEF0105\"," " \"csv-format\": False" " }," " {" " \"name\": \"preference\"," - " \"space\": \"dhcp6\"," - " \"code\": 7," - " \"data\": \"01\"," - " \"csv-format\": True" + " \"data\": \"01\"" " } ]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ]," @@ -2150,17 +2145,13 @@ TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"subscriber-id\"," - " \"space\": \"dhcp6\"," - " \"code\": 38," " \"data\": \"ABCDEF0105\"," " \"csv-format\": False" " }," " {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 38," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2229,16 +2220,12 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { "\"option-data\": [ {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 110," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"isc\"," - " \"code\": 111," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2275,24 +2262,17 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"base-option\"," - " \"space\": \"dhcp6\"," - " \"code\": 100," - " \"data\": \"11\"," - " \"csv-format\": True" + " \"data\": \"11\"" " }," " {" " \"name\": \"foo\"," " \"space\": \"isc\"," - " \"code\": 110," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"isc\"," - " \"code\": 111," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"base-option\"," @@ -2366,8 +2346,6 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { " \"subnet\": \"2001:db8:1::/64\", " " \"option-data\": [ {" " \"name\": \"subscriber-id\"," - " \"space\": \"dhcp6\"," - " \"code\": 38," " \"data\": \"0102030405060708090A\"," " \"csv-format\": False" " } ]" @@ -2377,8 +2355,6 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { " \"subnet\": \"2001:db8:2::/64\", " " \"option-data\": [ {" " \"name\": \"user-class\"," - " \"space\": \"dhcp6\"," - " \"code\": 15," " \"data\": \"FFFEFDFCFB\"," " \"csv-format\": False" " } ]" @@ -2741,8 +2717,7 @@ TEST_F(Dhcp6ParserTest, vendorOptionsCsv) { " \"name\": \"foo\"," " \"space\": \"vendor-4491\"," " \"code\": 100," - " \"data\": \"this is a string vendor-opt\"," - " \"csv-format\": True" + " \"data\": \"this is a string vendor-opt\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2803,16 +2778,12 @@ TEST_F(Dhcp6ParserTest, DISABLED_stdOptionDataEncapsulate) { "\"option-data\": [ {" " \"name\": \"foo\"," " \"space\": \"vendor-opts-space\"," - " \"code\": 110," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"vendor-opts-space\"," - " \"code\": 111," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2851,24 +2822,17 @@ TEST_F(Dhcp6ParserTest, DISABLED_stdOptionDataEncapsulate) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"vendor-opts\"," - " \"space\": \"dhcp6\"," - " \"code\": 17," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo\"," " \"space\": \"vendor-opts-space\"," - " \"code\": 110," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"vendor-opts-space\"," - " \"code\": 111," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2971,16 +2935,12 @@ buildHooksLibrariesConfig(const std::vector& libraries) { "\"option-data\": [ {" " \"name\": \"foo\"," " \"space\": \"vendor-opts-space\"," - " \"code\": 110," - " \"data\": \"1234\"," - " \"csv-format\": True" + " \"data\": \"1234\"" " }," " {" " \"name\": \"foo2\"," " \"space\": \"vendor-opts-space\"," - " \"code\": 111," - " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"data\": \"192.168.2.1\"" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index dfa145c7a3..0d3c1bdf7d 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -300,15 +300,10 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) { " \"interface\": \"eth0\", " " \"option-data\": [ {" " \"name\": \"dns-servers\"," - " \"space\": \"dhcp6\"," - " \"code\": 23," - " \"data\": \"2001:db8:1234:FFFF::1, 2001:db8:1234:FFFF::2\"," - " \"csv-format\": True" + " \"data\": \"2001:db8:1234:FFFF::1, 2001:db8:1234:FFFF::2\"" " }," " {" " \"name\": \"subscriber-id\"," - " \"space\": \"dhcp6\"," - " \"code\": 38," " \"data\": \"1234\"," " \"csv-format\": False" " } ]" @@ -1564,17 +1559,12 @@ TEST_F(Dhcpv6SrvTest, vendorOptionsORO) { " \"name\": \"config-file\"," " \"code\": 33," " \"type\": \"string\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"vendor-4491\"," - " \"encapsulate\": \"\"" + " \"space\": \"vendor-4491\"" " } ]," " \"option-data\": [ {" " \"name\": \"config-file\"," " \"space\": \"vendor-4491\"," - " \"code\": 33," - " \"data\": \"normal_erouter_v6.cm\"," - " \"csv-format\": True" + " \"data\": \"normal_erouter_v6.cm\"" " }]," "\"subnet6\": [ { " " \"pools\": [ { \"pool\": \"2001:db8:1::/64\" } ]," @@ -2254,11 +2244,7 @@ TEST_F(Dhcpv6SrvTest, rsooOverride) { " \"option-def\": [ {" " \"name\": \"foo\"," " \"code\": 120," - " \"type\": \"binary\"," - " \"array\": False," - " \"record-types\": \"\"," - " \"space\": \"dhcp6\"," - " \"encapsulate\": \"\"" + " \"type\": \"binary\"" " } ]," " \"option-data\": [ {" " \"code\": 120," diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index daa65f3cb4..001a39303b 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -530,7 +530,7 @@ TEST_F(ParseConfigTest, minimalOptionDefTest) { EXPECT_TRUE(def->getEncapsulatedSpace().empty()); } -/// @brief Check Basic parsing of options. +/// @brief Check basic parsing of options. /// /// Note that this tests basic operation of the OptionDataListParser and /// OptionDataParser. It uses a simple configuration consisting of one @@ -569,6 +569,40 @@ TEST_F(ParseConfigTest, basicOptionDataTest) { EXPECT_EQ(val, opt_ptr->toText()); } +/// @brief Check minimal parsing of options. +/// +/// Same than basic but without optional parameters set to their default. +TEST_F(ParseConfigTest, minimalOptionDataTest) { + + // Configuration string. + std::string config = + "{ \"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 100," + " \"type\": \"ipv4-address\"," + " \"space\": \"isc\"" + " } ], " + " \"option-data\": [ {" + " \"name\": \"foo\"," + " \"space\": \"isc\"," + " \"data\": \"192.0.2.0\"" + " } ]" + "}"; + + // Verify that the configuration string parses. + int rcode = parseConfiguration(config); + ASSERT_TRUE(rcode == 0); + + // Verify that the option can be retrieved. + OptionPtr opt_ptr = getOptionPtr("isc", 100); + ASSERT_TRUE(opt_ptr); + + // Verify that the option definition is correct. + std::string val = "type=00100, len=00004: 192.0.2.0 (ipv4-address)"; + + EXPECT_EQ(val, opt_ptr->toText()); +} + // This test checks behavior of the configuration parser for option data // for different values of csv-format parameter and when there is an option // definition present. @@ -720,7 +754,6 @@ TEST_F(ParseConfigTest, optionDataNoName) { "{ \"option-data\": [ {" " \"space\": \"dhcp6\"," " \"code\": 23," - " \"csv-format\": True," " \"data\": \"2001:db8:1::1\"" " } ]" "}"; @@ -741,7 +774,6 @@ TEST_F(ParseConfigTest, optionDataNoCode) { "{ \"option-data\": [ {" " \"space\": \"dhcp6\"," " \"name\": \"dns-servers\"," - " \"csv-format\": True," " \"data\": \"2001:db8:1::1\"" " } ]" "}"; From f5c8bd14f4aac4c4f69131529ff9a6a3aaf49695 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 18 Oct 2015 22:10:47 +0200 Subject: [PATCH 05/50] [3927] Tentative to fix option-def descriptions --- doc/guide/dhcp4-srv.xml | 16 ++++++++-------- doc/guide/dhcp6-srv.xml | 9 +++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 2347f9e271..ada3e3dd98 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -1160,14 +1160,14 @@ It is merely echoed by the server should be left blank. Note that the above set of comments define the format of the new option and do not set its values. - - - In the current release the default values are not propagated to the - parser when the new configuration is being set. Therefore, all - parameters must be specified at all times, even if their values are - left blank. - - + + The name, code and + type parameters are required, all others are + optional. The array default value is + false. The record-types + and encapsulate default values are blank + (i.e. ""). The default space is "dhcp4". + Once the new option format is defined, its value is set in the same way as for a standard option. For example the following diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index aa47eb8853..a88c255697 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -1022,6 +1022,15 @@ temporarily override a list of interface names and listen on all interfaces. set of comments define the format of the new option and do not set its values. + + The name, code and + type parameters are required, all others are + optional. The array default value is + false. The record-types + and encapsulate default values are blank + (i.e. ""). The default space is "dhcp4". + + Once the new option format is defined, its value is set in the same way as for a standard option. For example the following commands set a global value that applies to all subnets. From e5be011922ec7c808ae1102feb98600debdcd6a5 Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Fri, 30 Oct 2015 00:20:48 -0700 Subject: [PATCH 06/50] [trac4090] Add support for TokenSubstring Add support and testing for TokenSubstring. This token takes three paramaters (string, start and length) and produces a new string based on the original string. It allows negative values for start and length causing it to count from the end of the string for start and to get characters before the start point for length. --- src/lib/eval/eval_messages.mes | 4 + src/lib/eval/tests/token_unittest.cc | 207 +++++++++++++++++++++++++++ src/lib/eval/token.cc | 75 ++++++++++ src/lib/eval/token.h | 48 ++++++- 4 files changed, 333 insertions(+), 1 deletion(-) diff --git a/src/lib/eval/eval_messages.mes b/src/lib/eval/eval_messages.mes index cca2b0e19e..1ce1ca3d12 100644 --- a/src/lib/eval/eval_messages.mes +++ b/src/lib/eval/eval_messages.mes @@ -18,3 +18,7 @@ $NAMESPACE isc::dhcp This debug message indicates that the expression has been evaluated to said value. This message is mostly useful during debugging of the client classification expressions. + +% EVAL_SUBSTRING_BAD_PARAM_CONVERSION starting %1, length %2 +This debug message indicates that the parameter for the starting postion +or length of the substring couldn't be converted to an integer. diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index 6b29763c62..d3bb997bf3 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/src/lib/eval/tests/token_unittest.cc @@ -60,6 +60,40 @@ public: OptionPtr option_str4_; ///< A string option for DHCPv4 OptionPtr option_str6_; ///< A string option for DHCPv6 + + /// @brief Verify that the substring eval works properly + /// + /// This function takes the parameters and sets up the value + /// stack then executes the eval and checks the results. + /// + /// @param test_string The string to operate on + /// @param test_start The postion to start when getting a substring + /// @param test_length The length of the substring to get + /// @param result_string The expected result of the eval + void verifySubstringEval(const std::string& test_string, + const std::string& test_start, + const std::string& test_length, + const std::string& result_string) { + + // create the token + ASSERT_NO_THROW(t_.reset(new TokenSubstring())); + + // push values on stack + values_.push(test_string); + values_.push(test_start); + values_.push(test_length); + + // evaluate the token + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // verify results + ASSERT_EQ(1, values_.size()); + EXPECT_EQ(result_string, values_.top()); + + // remove result + values_.pop(); + } + /// @todo: Add more option types here }; @@ -197,3 +231,176 @@ TEST_F(TokenTest, optionEqualTrue) { } }; + +// This test checks if an a token representing a substring request +// throws an excpetion if there aren't enough values on the stack. +// The stack from the top is: length, start, string. +// The actual packet is not used. +TEST_F(TokenTest, substringNotEnoughValues) { + ASSERT_NO_THROW(t_.reset(new TokenSubstring())); + + // Subsring requires three values on the stack, try + // with 0, 1 and 2 all should thorw an exception + EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalBadStack); + + values_.push(""); + EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalBadStack); + + values_.push("0"); + EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalBadStack); + + // Three should work + values_.push("0"); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // As we had an empty string to start with we should have an empty + // one after the evaluate + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("", values_.top()); +} + +// Test getting the whole string in different ways +TEST_F(TokenTest, substringWholeString) { + // Get the whole string + verifySubstringEval("foobar", "0", "6", "foobar"); + + // Get the whole string with "all" + verifySubstringEval("foobar", "0", "all", "foobar"); + + // Get the whole string with an extra long number + verifySubstringEval("foobar", "0", "123456", "foobar"); + + // Get the whole string counting from the back + verifySubstringEval("foobar", "-6", "all", "foobar"); +} + +// Test getting a suffix, in this case the last 3 characters +TEST_F(TokenTest, substringTrailer) { + verifySubstringEval("foobar", "3", "3", "bar"); + verifySubstringEval("foobar", "3", "all", "bar"); + verifySubstringEval("foobar", "-3", "all", "bar"); + verifySubstringEval("foobar", "-3", "123", "bar"); +} + +// Test getting the middle of the string in different ways +TEST_F(TokenTest, substringMiddle) { + verifySubstringEval("foobar", "1", "4", "ooba"); + verifySubstringEval("foobar", "-5", "4", "ooba"); + verifySubstringEval("foobar", "-1", "-4", "ooba"); + verifySubstringEval("foobar", "5", "-4", "ooba"); +} + +// Test getting the last letter in different ways +TEST_F(TokenTest, substringLastLetter) { + verifySubstringEval("foobar", "5", "all", "r"); + verifySubstringEval("foobar", "5", "1", "r"); + verifySubstringEval("foobar", "5", "5", "r"); + verifySubstringEval("foobar", "-1", "all", "r"); + verifySubstringEval("foobar", "-1", "1", "r"); + verifySubstringEval("foobar", "-1", "5", "r"); +} + +// Test we get only what is available if we ask for a longer string +TEST_F(TokenTest, substringLength) { + // Test off the front + verifySubstringEval("foobar", "0", "-4", ""); + verifySubstringEval("foobar", "1", "-4", "f"); + verifySubstringEval("foobar", "2", "-4", "fo"); + verifySubstringEval("foobar", "3", "-4", "foo"); + + // and the back + verifySubstringEval("foobar", "3", "4", "bar"); + verifySubstringEval("foobar", "4", "4", "ar"); + verifySubstringEval("foobar", "5", "4", "r"); + verifySubstringEval("foobar", "6", "4", ""); +} + +// Test that we get nothing if the starting postion is out of the string +TEST_F(TokenTest, substringStartingPosition) { + // Off the front + verifySubstringEval("foobar", "-7", "1", ""); + verifySubstringEval("foobar", "-7", "-11", ""); + verifySubstringEval("foobar", "-7", "all", ""); + + // and the back + verifySubstringEval("foobar", "6", "1", ""); + verifySubstringEval("foobar", "6", "-11", ""); + verifySubstringEval("foobar", "6", "all", ""); +} + +// Check what happens if we use strings that aren't numbers for start or length +// We should return the empty string +TEST_F(TokenTest, substringBadParams) { + verifySubstringEval("foobar", "0ick", "all", ""); + verifySubstringEval("foobar", "ick0", "all", ""); + verifySubstringEval("foobar", "ick", "all", ""); + verifySubstringEval("foobar", "0", "ick", ""); + verifySubstringEval("foobar", "0", "0ick", ""); + verifySubstringEval("foobar", "0", "ick0", ""); + verifySubstringEval("foobar", "0", "allaboard", ""); +} + +// lastly check that we don't get anything if the string is empty +TEST_F(TokenTest, substringStartingEmpty) { + verifySubstringEval("", "0", "all", ""); + verifySubstringEval("foobar", "0", "0", ""); +} + +// Check if we can use the substring and equal tokens together +// We put the result on the stack first then the substring values +// then evaluate the substring which should leave the original +// result on the bottom with the substring result on next. +// Evaulating the equals should produce true for the first +// and false for the second. +// throws an excpetion if there aren't enough values on the stack. +// The stack from the top is: length, start, string. +// The actual packet is not used. +TEST_F(TokenTest, substringEquals) { + TokenPtr tequal; + + ASSERT_NO_THROW(t_.reset(new TokenSubstring())); + ASSERT_NO_THROW(tequal.reset(new TokenEqual())); + + // The final expected value + values_.push("ooba"); + + // The substring values + // Subsring requires three values on the stack, try + // with 0, 1 and 2 all should thorw an exception + values_.push("foobar"); + values_.push("1"); + values_.push("4"); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // we should have two values on the stack + ASSERT_EQ(2, values_.size()); + + // next the equals eval + EXPECT_NO_THROW(tequal->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("true", values_.top()); + + // get rid of the result + values_.pop(); + + // and try it again but with a bad final value + // The final expected value + values_.push("foob"); + + // The substring values + // Subsring requires three values on the stack, try + // with 0, 1 and 2 all should thorw an exception + values_.push("foobar"); + values_.push("1"); + values_.push("4"); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // we should have two values on the stack + ASSERT_EQ(2, values_.size()); + + // next the equals eval + EXPECT_NO_THROW(tequal->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("false", values_.top()); + +} diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 66ebf1c5b1..3d3266a7dd 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -13,6 +13,7 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include #include using namespace isc::dhcp; @@ -53,3 +54,77 @@ TokenEqual::evaluate(const Pkt& /*pkt*/, ValueStack& values) { else values.push("false"); } + +void +TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) { + + if (values.size() < 3) { + isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " + "3 values for substring operator, got " << values.size()); + } + + string len_str = values.top(); + values.pop(); + string start_str = values.top(); + values.pop(); + string string_str = values.top(); + values.pop(); + + // If we have no string to start with we push an empty string and leave + if (string_str.empty()) { + values.push(""); + return; + } + + // Convert the starting position and legnth from strings to numbers + // the length may also be "all" in which case simply make it the + // legnth of the string. + // If we have a problem push an empty string and leave + int start_pos, length; + try { + start_pos = boost::lexical_cast(start_str); + if ("all" == len_str) { + length = string_str.length(); + } else { + length = boost::lexical_cast(len_str); + } + } catch (const boost::bad_lexical_cast&) { +#if 0 + // Logging not yet built + LOG_DEBUG(eval_logger, EVAL_DBG_TRACE, + EVAL_SUBSTRING_BAD_PARAM_CONVERSION) + .arg(start_str) + .arg(len_str); +#endif + values.push(""); + return; + } + + // If the starting postion is outside of the string push an + // empty string and leave + if (((start_pos >= 0) && (start_pos >= string_str.length())) | + ((start_pos < 0) && (-start_pos > string_str.length()))) { + values.push(""); + return; + } + + // Adjust the values to be something for substr. We first figure out + // the staring postion, then update it and the length to get the + // characters before or after it depnding on the sign of length + if (start_pos < 0) { + start_pos = string_str.length() + start_pos; + } + + if (length < 0) { + length = -length; + if (length <= start_pos){ + start_pos -= length; + } else { + length = start_pos; + start_pos = 0; + } + } + + // and finally get the substring + values.push(string_str.substr(start_pos, length)); +} diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index 930c707fc5..dee4c46212 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -56,7 +56,7 @@ public: /// - option[123] (a token that extracts value of option 123) /// - == (an operator that compares two other tokens) /// - substring(a,b,c) (an operator that takes three arguments: a string, -/// first and last character) +/// first character and length) class Token { public: @@ -158,6 +158,52 @@ public: void evaluate(const Pkt& pkt, ValueStack& values); }; +/// @brief Token that represents the substring operator (returns a portion +/// of the supplied string) +/// +/// This token represents substring(str, start, len) An operator that takes three +/// arguments: a string, the first character and the length. +class TokenSubstring : public Token { +public: + /// @brief Constructor (does nothing) + TokenSubstring() {} + + /// @brief Extract a substring from a string + /// + /// Evaluation does not use packet information, but rather consumes the last + /// three parameters and requires at least three parameters to be present on + /// stack. From the top it expects the values on the stack as: + /// len + /// start + /// str + /// + /// str is the string to extract a substring from. If it is empty an empty + /// string is pushed onto the value stack. + /// + /// start is the postion from which the code starts extracting the substring. + /// 0 is before the first character and a negative number starts from the end, + /// with -1 being before the last character. If the starting point is + /// outside of the original string an empty string is pushed onto the value + /// stack. + /// + /// length is the number of characters from the string to extract. + /// "all" means all remaining characters from start to the end of string. + /// A negative number means to go from start towards the beginning of + /// string, but doesn't include start. + /// If length is longer than the remaining portion of string + /// then the entire remaining portion is placed on the value stack. + /// Note: a negative value of length only indicates which characters to + /// extract, it does NOT indicate any attempt to reverse the string. + /// For example substring("foobar", 4, -2) will result in "ob". + /// + /// @throw EvalBadStack if there's less than 3 values on stack + /// + /// @brief pkt (unused) + /// @brief values - stack of values (3 arguments will be poped, 1 result + /// will be pushed) + void evaluate(const Pkt& pkt, ValueStack& values); +}; + }; // end of isc::dhcp namespace }; // end of isc namespace From bdd7b963e9ab873d40efbe09380d92507b41515f Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 13:20:31 +0900 Subject: [PATCH 07/50] [4105] 4o6 configuration structure, unit-tests implemented --- src/bin/dhcp4/tests/config_parser_unittest.cc | 139 ++++++++++++++++++ src/lib/dhcpsrv/subnet.h | 35 +++++ 2 files changed, 174 insertions(+) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index d89c8a2ba4..50633286bd 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -3782,4 +3782,143 @@ TEST_F(Dhcp4ParserTest, expiredLeasesProcessingError) { EXPECT_TRUE(errorContainsPosition(status, "")); } + +// Checks if the DHCPv4 is able to parse the configuration without 4o6 parameters +// and do not set 4o6 fields at all. +TEST_F(Dhcp4ParserTest, 4o6default) { + + ConstElementPtr status; + + // Just a plain v4 config (no 4o6 parameters) + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // check if returned status is OK + checkResult(status, 0); + + // Now check if the configuration was indeed handled and we have + // expected pool configured. + Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> + getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); + ASSERT_TRUE(subnet); + + Cfg4o6& dhcp4o6 = subnet->get4o6(); + EXPECT_FALSE(dhcp4o6.enabled_); +} + +// Checks if the DHCPv4 is able to parse the configuration with 4o6 subnet +// defined. +TEST_F(Dhcp4ParserTest, 4o6subnet) { + + ConstElementPtr status; + + // Just a plain v4 config (no 4o6 parameters) + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-subnet\": \"2001:db8::123/45\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // check if returned status is OK + checkResult(status, 0); + + // Now check if the configuration was indeed handled and we have + // expected pool configured. + Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> + getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); + ASSERT_TRUE(subnet); + + Cfg4o6& dhcp4o6 = subnet->get4o6(); + EXPECT_TRUE(dhcp4o6.enabled_); + EXPECT_EQ(IOAddress("2001:db8::123"), dhcp4o6.subnet4o6_.first); + EXPECT_EQ(45, dhcp4o6.subnet4o6_.second); +} + +// Checks if the DHCPv4 is able to parse the configuration with 4o6 network +// interface defined. +TEST_F(Dhcp4ParserTest, 4o6iface) { + + ConstElementPtr status; + + // Just a plain v4 config (no 4o6 parameters) + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-interface\": \"ethX\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // check if returned status is OK + checkResult(status, 0); + + // Now check if the configuration was indeed handled and we have + // expected pool configured. + Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> + getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); + ASSERT_TRUE(subnet); + + Cfg4o6& dhcp4o6 = subnet->get4o6(); + EXPECT_TRUE(dhcp4o6.enabled_); + EXPECT_EQ("ethX", dhcp4o6.iface4o6_); +} + +// Checks if the DHCPv4 is able to parse the configuration with both 4o6 network +// interface and v6 subnet defined. +TEST_F(Dhcp4ParserTest, 4o6subnetIface) { + + ConstElementPtr status; + + // Just a plain v4 config (no 4o6 parameters) + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-subnet\": \"2001:db8::543/21\"," + " \"4o6-interface\": \"ethX\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // check if returned status is OK + checkResult(status, 0); + + // Now check if the configuration was indeed handled and we have + // expected pool configured. + Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> + getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); + ASSERT_TRUE(subnet); + + Cfg4o6& dhcp4o6 = subnet->get4o6(); + EXPECT_TRUE(dhcp4o6.enabled_); + EXPECT_EQ(IOAddress("2001:db8::543"), dhcp4o6.subnet4o6_.first); + EXPECT_EQ(21, dhcp4o6.subnet4o6_.second); + EXPECT_EQ("ethX", dhcp4o6.iface4o6_); +} + } diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 55e4ddd716..caa86f4f19 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -507,6 +507,29 @@ private: /// @brief A generic pointer to either Subnet4 or Subnet6 object typedef boost::shared_ptr SubnetPtr; +/// @brief This structure contains information about DHCP4o6 (RFC7341) +/// +/// DHCP4o6 is completely optional. If it is not enabled, this structure +/// does not contain any information. +struct Cfg4o6 { + + /// the default constructor. + /// + /// Initializes fields to their default value. + Cfg4o6() + :enabled_(false), subnet4o6_(std::make_pair(asiolink::IOAddress("::"), 128u)) { + } + + /// Specifies if 4o6 is enabled on this subnet. + bool enabled_; + + /// Specifies the network interface used as subnet selector + std::string iface4o6_; + + /// Specifies the IPv6 subnet used for subnet selection + std::pair subnet4o6_; +}; + /// @brief A configuration holder for IPv4 subnet. /// /// This class represents an IPv4 subnet. @@ -559,6 +582,14 @@ public: return (match_client_id_); } + /// @brief Returns DHCP4o6 configuration parameters. + /// + /// This structure is always available. If the 4o6 is not enabled, its + /// enabled_ field will be set to false. + Cfg4o6& get4o6() { + return (dhcp4o6_); + } + private: /// @brief Returns default address for pool selection @@ -581,6 +612,10 @@ private: /// @brief Should server use client identifiers for client lease /// lookup. bool match_client_id_; + + + /// @brief All the information related to DHCP4o6 + Cfg4o6 dhcp4o6_; }; /// @brief A pointer to a @c Subnet4 object From 4e77daebb1cf1a1faf3bba33a2a0ead4ed06d00a Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Mon, 14 Sep 2015 16:36:42 +0200 Subject: [PATCH 08/50] [fd4o6] Added DHCPv4-over-DHCPv6 packet class --- src/lib/dhcp/Makefile.am | 3 ++- src/lib/dhcp/pkt4o6.cc | 50 ++++++++++++++++++++++++++++++++++ src/lib/dhcp/pkt4o6.h | 58 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 src/lib/dhcp/pkt4o6.cc create mode 100644 src/lib/dhcp/pkt4o6.h diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index 73f1b00590..349070ec0c 100644 --- a/src/lib/dhcp/Makefile.am +++ b/src/lib/dhcp/Makefile.am @@ -15,7 +15,7 @@ CLEANFILES = *.gcno *.gcda lib_LTLIBRARIES = libkea-dhcp++.la libkea_dhcp___la_SOURCES = libkea_dhcp___la_SOURCES += classify.cc classify.h -libkea_dhcp___la_SOURCES += dhcp6.h dhcp4.h +libkea_dhcp___la_SOURCES += dhcp6.h dhcp4.h dhcp4o6.h libkea_dhcp___la_SOURCES += duid.cc duid.h libkea_dhcp___la_SOURCES += hwaddr.cc hwaddr.h libkea_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h @@ -48,6 +48,7 @@ libkea_dhcp___la_SOURCES += protocol_util.cc protocol_util.h libkea_dhcp___la_SOURCES += pkt.cc pkt.h libkea_dhcp___la_SOURCES += pkt6.cc pkt6.h libkea_dhcp___la_SOURCES += pkt4.cc pkt4.h +libkea_dhcp___la_SOURCES += pkt4o6.cc pkt4o6.h libkea_dhcp___la_SOURCES += pkt_filter.h pkt_filter.cc libkea_dhcp___la_SOURCES += pkt_filter6.h pkt_filter6.cc libkea_dhcp___la_SOURCES += pkt_filter_inet.cc pkt_filter_inet.h diff --git a/src/lib/dhcp/pkt4o6.cc b/src/lib/dhcp/pkt4o6.cc new file mode 100644 index 0000000000..f328e82f7d --- /dev/null +++ b/src/lib/dhcp/pkt4o6.cc @@ -0,0 +1,50 @@ +// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +using namespace std; +using namespace isc::dhcp; +using namespace isc::asiolink; + +namespace { + +/// @brief Default address used in Pkt4 constructor +const IOAddress DEFAULT_ADDRESS("0.0.0.0"); +} + +namespace isc { +namespace dhcp { + +Pkt4o6::Pkt4o6(const uint8_t* data, size_t len, const Pkt6Ptr& pkt6) + :Pkt4(data, len), pkt6_(pkt6) +{ + setIface(pkt6->getIface()); + setIndex(pkt6->getIndex()); + setRemoteAddr(pkt6->getRemoteAddr()); +} + +} // end of namespace isc::dhcp + +} // end of namespace isc diff --git a/src/lib/dhcp/pkt4o6.h b/src/lib/dhcp/pkt4o6.h new file mode 100644 index 0000000000..7e6f1877d2 --- /dev/null +++ b/src/lib/dhcp/pkt4o6.h @@ -0,0 +1,58 @@ +// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef PKT4O6_H +#define PKT4O6_H + +#include +#include + +#include + +namespace isc { + +namespace dhcp { + +/// @brief Represents DHCPv4-over-DHCPv6 packet +/// +/// This class derives from @c Pkt4 in order to be handled by +/// the DHCPv4 server code. It includes a shared pointer to the +/// DHCPv6 message too. +class Pkt4o6 : public Pkt4 { +public: + + /// @brief Constructor, used in message reception. + /// + /// @param data pointer to received data + /// @param len size of buffer to be allocated for this packet + /// @param pkt6 encapsulating DHCPv6 message. + Pkt4o6(const uint8_t* data, size_t len, const Pkt6Ptr& pkt6); + + /// @brief Returns encapsulating DHCPv6 message + const Pkt6Ptr& getPkt6() { return (pkt6_); } + +protected: + /// Encapsulating DHCPv6 message + const Pkt6Ptr& pkt6_; + +}; // Pkt4o6 class + +/// @brief A pointer to Pkt4o6 object. +typedef boost::shared_ptr Pkt4o6Ptr; + +} // isc::dhcp namespace + +} // isc namespace + +#endif From 72ba32da5f6a946ca7f131a450663cb345dd51cc Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 13:29:05 +0900 Subject: [PATCH 09/50] [4027] incorporated part of c0044e3 from fd4o6 branch (not cherry-picking it because the commit has other irrelevant changes) --- src/lib/dhcp/pkt4o6.cc | 36 ++++++++++++++++++++++++------------ src/lib/dhcp/pkt4o6.h | 31 ++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/lib/dhcp/pkt4o6.cc b/src/lib/dhcp/pkt4o6.cc index f328e82f7d..7fb087a6e3 100644 --- a/src/lib/dhcp/pkt4o6.cc +++ b/src/lib/dhcp/pkt4o6.cc @@ -13,20 +13,17 @@ // PERFORMANCE OF THIS SOFTWARE. #include -#include -#include -#include -#include + +#include +#include #include #include +#include -#include -#include -#include - -using namespace std; -using namespace isc::dhcp; using namespace isc::asiolink; +using namespace isc::dhcp; +using namespace isc::util; +using namespace std; namespace { @@ -37,14 +34,29 @@ const IOAddress DEFAULT_ADDRESS("0.0.0.0"); namespace isc { namespace dhcp { -Pkt4o6::Pkt4o6(const uint8_t* data, size_t len, const Pkt6Ptr& pkt6) - :Pkt4(data, len), pkt6_(pkt6) +Pkt4o6::Pkt4o6(const OptionBuffer& pkt4, const Pkt6Ptr& pkt6) + :Pkt4(&pkt4[0], pkt4.size()), pkt6_(pkt6) { + static_cast(pkt6->delOption(D6O_DHCPV4_MSG)); setIface(pkt6->getIface()); setIndex(pkt6->getIndex()); setRemoteAddr(pkt6->getRemoteAddr()); } +Pkt4o6::Pkt4o6(const Pkt4Ptr& pkt4, const Pkt6Ptr& pkt6) + :Pkt4(*pkt4), pkt6_(pkt6) { +} + +void Pkt4o6::pack() { + Pkt4::pack(); + OutputBuffer& buf = getBuffer(); + const uint8_t* ptr = static_cast(buf.getData()); + OptionBuffer msg(ptr, ptr + buf.getLength()); + OptionPtr dhcp4_msg(new Option(Option::V6, D6O_DHCPV4_MSG, msg)); + pkt6_->addOption(dhcp4_msg); + pkt6_->pack(); +} + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcp/pkt4o6.h b/src/lib/dhcp/pkt4o6.h index 7e6f1877d2..f90ae4eb61 100644 --- a/src/lib/dhcp/pkt4o6.h +++ b/src/lib/dhcp/pkt4o6.h @@ -34,13 +34,34 @@ public: /// @brief Constructor, used in message reception. /// - /// @param data pointer to received data - /// @param len size of buffer to be allocated for this packet - /// @param pkt6 encapsulating DHCPv6 message. - Pkt4o6(const uint8_t* data, size_t len, const Pkt6Ptr& pkt6); + /// @param pkt4 DHCPv4 message + /// @param pkt6 encapsulating unpacked DHCPv6 message + /// the DHCPv4 message option will be removed + Pkt4o6(const OptionBuffer& pkt4, const Pkt6Ptr& pkt6); + + /// @brief Constructor, used in replying to a message + /// + /// @param pkt4 DHCPv4 message + /// @param pkt6 DHCPv6 message + Pkt4o6(const Pkt4Ptr& pkt4, const Pkt6Ptr& pkt6); /// @brief Returns encapsulating DHCPv6 message - const Pkt6Ptr& getPkt6() { return (pkt6_); } + const Pkt6Ptr& getPkt6() const { return (pkt6_); } + + /// @brief Prepares on-wire format of DHCPv4-over-DHCPv6 packet. + /// + /// Calls pack() on both DHCPv4 and DHCPv6 parts + /// Inserts the DHCPv4-message option + /// @ref pkt4::pack and @ref pkt6::pack + virtual void pack(); + + /// @brief Checks if a DHCPv4 message has beeb transported over DHCPv6 + /// + /// @return Boolean value which indicates whether the message is + /// transported over DHCPv6 (true) or native DHCPv4 (false) + virtual bool isDhcp4o6() const { + return (true); + } protected: /// Encapsulating DHCPv6 message From 9d3df95a21a93ea6314b0fc142c8b89fc19f6468 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 8 Oct 2015 05:15:28 +0200 Subject: [PATCH 10/50] [fd4o6] Adjusted Pkt6 pointer type --- src/lib/dhcp/pkt4o6.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcp/pkt4o6.h b/src/lib/dhcp/pkt4o6.h index f90ae4eb61..4608ee55bb 100644 --- a/src/lib/dhcp/pkt4o6.h +++ b/src/lib/dhcp/pkt4o6.h @@ -46,7 +46,7 @@ public: Pkt4o6(const Pkt4Ptr& pkt4, const Pkt6Ptr& pkt6); /// @brief Returns encapsulating DHCPv6 message - const Pkt6Ptr& getPkt6() const { return (pkt6_); } + Pkt6Ptr getPkt6() const { return (pkt6_); } /// @brief Prepares on-wire format of DHCPv4-over-DHCPv6 packet. /// @@ -65,7 +65,7 @@ public: protected: /// Encapsulating DHCPv6 message - const Pkt6Ptr& pkt6_; + Pkt6Ptr pkt6_; }; // Pkt4o6 class From 5fcb4e02c45d43d419bb0a165eb4d59be8ee8316 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 13:46:05 +0900 Subject: [PATCH 11/50] [4027] enable DHCP4o6 option macros --- src/lib/dhcp/dhcp6.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index 6e053f69b1..86df12f99a 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/src/lib/dhcp/dhcp6.h @@ -110,8 +110,8 @@ //#define D6O_ADDRSEL 84 /* RFC7078 */ //#define D6O_ADDRSEL_TABLE 85 /* RFC7078 */ //#define D6O_V6_PCP_SERVER 86 /* RFC7291 */ -//#define D6O_DHCPV4_MSG 87 /* RFC7341 */ -//#define D6O_DHCPV4_O_DHCPV6_SERVER 88 /* RFC7341 */ +#define D6O_DHCPV4_MSG 87 /* RFC7341 */ +#define D6O_DHCPV4_O_DHCPV6_SERVER 88 /* RFC7341 */ //#define D6O_S46_RULE 89 /* RFC7598 */ //#define D6O_S46_BR 90 /* RFC7598 */ //#define D6O_S46_DMR 91 /* RFC7598 */ From 8c1b9097d87057686d6e0caf7cd4ff76883a22ae Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 31 Oct 2015 05:47:08 +0100 Subject: [PATCH 12/50] [4107] Ported changes from fd4o6 (so not finished) --- src/lib/dhcp/dhcp6.h | 12 ++++++++---- src/lib/dhcp/pkt6.cc | 10 ++++++++++ src/lib/dhcp/std_option_defs.h | 3 +++ src/lib/dhcp/tests/libdhcp++_unittest.cc | 6 ++++++ src/lib/dhcp/tests/pkt6_unittest.cc | 8 ++++++++ 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index 6e053f69b1..c18d53b9ec 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/src/lib/dhcp/dhcp6.h @@ -110,8 +110,8 @@ //#define D6O_ADDRSEL 84 /* RFC7078 */ //#define D6O_ADDRSEL_TABLE 85 /* RFC7078 */ //#define D6O_V6_PCP_SERVER 86 /* RFC7291 */ -//#define D6O_DHCPV4_MSG 87 /* RFC7341 */ -//#define D6O_DHCPV4_O_DHCPV6_SERVER 88 /* RFC7341 */ +#define D6O_DHCPV4_MSG 87 /* RFC7341 */ +#define D6O_DHCPV4_O_DHCPV6_SERVER 88 /* RFC7341 */ //#define D6O_S46_RULE 89 /* RFC7598 */ //#define D6O_S46_BR 90 /* RFC7598 */ //#define D6O_S46_DMR 91 /* RFC7598 */ @@ -195,8 +195,8 @@ //#define DHCPV6_RECONFIGURE_REQUEST 18 //#define DHCPV6_RECONFIGURE_REPLY 19 /* RFC 7341 */ -//#define DHCPV6_DHCPV4_QUERY 20 -//#define DHCPV6_DHCPV4_RESPONSE 21 +#define DHCPV6_DHCPV4_QUERY 20 +#define DHCPV6_DHCPV4_RESPONSE 21 /* draft-ietf-dhc-dhcpv6-active-leasequery-04 */ //#define DHCPV6_ACTIVELEASEQUERY 22 //#define DHCPV6_STARTTLS 23 @@ -298,4 +298,8 @@ extern const int dhcpv6_type_name_max; #define IRT_DEFAULT 86400 #define IRT_MINIMUM 600 +/* DHCPv4-query message flags */ + +#define DHCPV4_QUERY_FLAGS_UNICAST (1 << 23) + #endif /* DHCP6_H */ diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index 91d1e18049..33b4633722 100644 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -289,6 +289,8 @@ Pkt6::unpackUDP() { case DHCPV6_DECLINE: case DHCPV6_RECONFIGURE: case DHCPV6_INFORMATION_REQUEST: + case DHCPV6_DHCPV4_QUERY: + case DHCPV6_DHCPV4_RESPONSE: default: // assume that uknown messages are not using relay format { return (unpackMsg(data_.begin(), data_.end())); @@ -586,6 +588,8 @@ Pkt6::getName(const uint8_t type) { static const char* REPLY = "REPLY"; static const char* REQUEST = "REQUEST"; static const char* SOLICIT = "SOLICIT"; + static const char* DHCPV4_QUERY = "DHCPV4_QUERY"; + static const char* DHCPV4_RESPONSE = "DHCPV4_RESPONSE"; static const char* UNKNOWN = "UNKNOWN"; switch (type) { @@ -634,6 +638,12 @@ Pkt6::getName(const uint8_t type) { case DHCPV6_SOLICIT: return (SOLICIT); + case DHCPV6_DHCPV4_QUERY: + return (DHCPV4_QUERY); + + case DHCPV6_DHCPV4_RESPONSE: + return (DHCPV4_RESPONSE); + default: ; } diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h index 92f1976c5d..d75048e9a7 100644 --- a/src/lib/dhcp/std_option_defs.h +++ b/src/lib/dhcp/std_option_defs.h @@ -344,6 +344,9 @@ const OptionDefParams OPTION_DEF_PARAMS6[] = { RECORD_DEF(LQ_RELAY_DATA_RECORDS), "" }, { "lq-client-link", D6O_LQ_CLIENT_LINK, OPT_IPV6_ADDRESS_TYPE, true, NO_RECORD_DEF, "" }, + { "dhcpv4-message", D6O_DHCPV4_MSG, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, + { "dhcp4o6-server-addr", D6O_DHCPV4_O_DHCPV6_SERVER, OPT_IPV6_ADDRESS_TYPE, true, + NO_RECORD_DEF, "" } { "bootfile-url", D6O_BOOTFILE_URL, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" }, { "bootfile-param", D6O_BOOTFILE_PARAM, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "client-arch-type", D6O_CLIENT_ARCH_TYPE, OPT_UINT16_TYPE, true, NO_RECORD_DEF, "" }, diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc index dc71f99873..048e31f361 100644 --- a/src/lib/dhcp/tests/libdhcp++_unittest.cc +++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc @@ -1200,6 +1200,12 @@ TEST_F(LibDhcpTest, stdOptionDefs6) { fqdn_buf.begin(), fqdn_buf.end(), typeid(OptionCustom)); + LibDhcpTest::testStdOptionDefs6(D6O_DHCPV4_MSG, begin, end, + typeid(Option)); + + LibDhcpTest::testStdOptionDefs6(D6O_DHCPV4_O_DHCPV6_SERVER, begin, end, + typeid(Option6AddrLst)); + LibDhcpTest::testStdOptionDefs6(D6O_PUBLIC_KEY, begin, end, typeid(Option)); diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index 4cb1548723..6dbc01611b 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -594,6 +594,14 @@ TEST_F(Pkt6Test, getName) { EXPECT_STREQ("DECLINE", Pkt6::getName(type)); break; + case DHCPV6_DHCPV4_QUERY: + EXPECT_STREQ("DHCPV4_QUERY", Pkt6::getName(type)); + break; + + case DHCPV6_DHCPV4_RESPONSE: + EXPECT_STREQ("DHCPV4_RESPONSE", Pkt6::getName(type)); + break; + case DHCPV6_INFORMATION_REQUEST: EXPECT_STREQ("INFORMATION_REQUEST", Pkt6::getName(type)); From 75f18d0a63386aaa88bdc4f93237d8279798bc4f Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 14:08:01 +0900 Subject: [PATCH 13/50] [4105] Subnet4 parser updated to accept 4o6-interface, 4o6-subnet parameters --- src/bin/dhcp4/json_config_parser.cc | 36 +++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index d62e03e39b..27cc682c2f 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -200,6 +200,10 @@ protected: parser = new OptionDataListParser(config_id, options_, AF_INET); } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, boolean_values_); + } else if (config_id.compare("4o6-subnet") == 0) { + parser = new StringParser(config_id, string_values_); + } else if (config_id.compare("4o6-interface") == 0) { + parser = new StringParser(config_id, string_values_); } else { isc_throw(NotImplemented, "unsupported parameter: " << config_id); } @@ -305,6 +309,38 @@ protected: << ")"); } + // Try 4o6 specific parameter: 4o6-interface + try { + string iface4o6 = string_values_->getParam("4o6-interface"); + subnet4->get4o6().iface4o6_ = iface4o6; + subnet4->get4o6().enabled_ = true; + } catch (const DhcpConfigError&) { + // Don't care. 4o6-subnet is optional. + } + + // Try 4o6 specific parameter: 4o6-subnet + try { + string subnet4o6 = string_values_->getParam("4o6-subnet"); + size_t slash = subnet4o6.find("/"); + if (slash == std::string::npos) { + isc_throw(DhcpConfigError, "Missing / in the 4o6-subnet parameter:" + + subnet4o6 +", expected format: prefix6/length"); + } + string prefix = subnet4o6.substr(0, slash); + string lenstr = subnet4o6.substr(slash + 1); + + uint8_t len = 128; + try { + len = boost::lexical_cast(lenstr.c_str()); + } catch (const boost::bad_lexical_cast &) { + isc_throw(DhcpConfigError, "Invalid prefix length specified in " + "4o6-subnet parameter: " + subnet4o6 + ", expected 0..128 value"); + } + subnet4->get4o6().subnet4o6_ = make_pair(IOAddress(prefix), len); + subnet4->get4o6().enabled_ = true; + } catch (const DhcpConfigError&) { + // Don't care. 4o6-subnet is optional. + } // Try setting up client class (if specified) try { From bd535576f1869a9be98337a79abdf295660eb5f1 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 31 Oct 2015 06:35:23 +0100 Subject: [PATCH 14/50] [4107] Added the 2 private options (now update the doc) --- src/lib/dhcp/dhcp6.h | 4 ++++ src/lib/dhcp/libdhcp++.cc | 11 +++++++++++ src/lib/dhcp/libdhcp++.h | 3 +++ src/lib/dhcp/std_option_defs.h | 10 +++++++++- 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index c18d53b9ec..1380b1ae07 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/src/lib/dhcp/dhcp6.h @@ -223,6 +223,10 @@ extern const int dhcpv6_type_name_max; // Taken from http://www.iana.org/assignments/enterprise-numbers #define ENTERPRISE_ID_ISC 2495 +/* DHCPv4-over-DHCPv6 (RFC 7341) inter-process communication */ +#define ISC_V6_4O6_INTERFACE 60000 +#define ISC_V6_4O6_SRC_ADDRESS 60001 + /* Offsets into IA_*'s where Option spaces commence. */ #define IA_NA_OFFSET 12 /* IAID, T1, T2, all 4 octets each */ #define IA_TA_OFFSET 4 /* IAID only, 4 octets */ diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index db484cce36..069f09a04a 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -110,6 +110,11 @@ LibDHCP::getVendorOption6Defs(const uint32_t vendor_id) { initVendorOptsDocsis6(); } + if (vendor_id == ENTERPRISE_ID_ISC && + vendor6_defs_.find(ENTERPRISE_ID_ISC) == vendor6_defs_.end()) { + initVendorOptsIsc6(); + } + VendorOptionDefContainers::const_iterator def = vendor6_defs_.find(vendor_id); if (def == vendor6_defs_.end()) { // No such vendor-id space @@ -737,6 +742,12 @@ LibDHCP::initVendorOptsDocsis6() { initOptionSpace(vendor6_defs_[VENDOR_ID_CABLE_LABS], DOCSIS3_V6_DEFS, DOCSIS3_V6_DEFS_SIZE); } +void +LibDHCP::initVendorOptsIsc6() { + vendor6_defs_[ENTERPRISE_ID_ISC] = OptionDefContainer(); + initOptionSpace(vendor6_defs_[ENTERPRISE_ID_ISC], ISC_V6_DEFS, ISC_V6_DEFS_SIZE); +} + void initOptionSpace(OptionDefContainer& defs, const OptionDefParams* params, size_t params_size) { diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h index 380e0aeacb..57fac075bf 100644 --- a/src/lib/dhcp/libdhcp++.h +++ b/src/lib/dhcp/libdhcp++.h @@ -281,6 +281,9 @@ private: static void initVendorOptsDocsis6(); + /// Initialize private DHCPv6 option definitions. + static void initVendorOptsIsc6(); + /// pointers to factories that produce DHCPv6 options static FactoryMap v4factories_; diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h index d75048e9a7..da27a19408 100644 --- a/src/lib/dhcp/std_option_defs.h +++ b/src/lib/dhcp/std_option_defs.h @@ -346,7 +346,7 @@ const OptionDefParams OPTION_DEF_PARAMS6[] = { NO_RECORD_DEF, "" }, { "dhcpv4-message", D6O_DHCPV4_MSG, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "dhcp4o6-server-addr", D6O_DHCPV4_O_DHCPV6_SERVER, OPT_IPV6_ADDRESS_TYPE, true, - NO_RECORD_DEF, "" } + NO_RECORD_DEF, "" }, { "bootfile-url", D6O_BOOTFILE_URL, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" }, { "bootfile-param", D6O_BOOTFILE_PARAM, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "client-arch-type", D6O_CLIENT_ARCH_TYPE, OPT_UINT16_TYPE, true, NO_RECORD_DEF, "" }, @@ -374,6 +374,14 @@ const OptionDefParams OPTION_DEF_PARAMS6[] = { const int OPTION_DEF_PARAMS_SIZE6 = sizeof(OPTION_DEF_PARAMS6) / sizeof(OPTION_DEF_PARAMS6[0]); +/// @brief Definitions of private DHCPv6 options +const OptionDefParams ISC_V6_DEFS[] = { + { "4o6-interface", ISC_V6_4O6_INTERFACE, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" }, + { "4o6-source-address", ISC_V6_4O6_SRC_ADDRESS, OPT_IPV6_ADDRESS_TYPE, false, NO_RECORD_DEF, "" } +}; + +const int ISC_V6_DEFS_SIZE = sizeof(ISC_V6_DEFS) / sizeof(OptionDefParams); + } // unnamed namespace } // namespace dhcp From f2717fcc392a15bf253df96562ce04878b2cc318 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 31 Oct 2015 06:53:34 +0100 Subject: [PATCH 15/50] [4107] Added new standard options in the guide table --- doc/guide/dhcp6-srv.xml | 2 ++ src/lib/dhcp/std_option_defs.h | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 9e10e671d5..cfcc6a55c3 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -961,6 +961,8 @@ temporarily override a list of interface names and listen on all interfaces. erp-local-domain-name65fqdnfalse rsoo66emptyfalse client-linklayer-addr79binaryfalse + +dhcp4o6-server-addr88ipv6-addresstrue diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h index da27a19408..a12f9d00f6 100644 --- a/src/lib/dhcp/std_option_defs.h +++ b/src/lib/dhcp/std_option_defs.h @@ -344,9 +344,6 @@ const OptionDefParams OPTION_DEF_PARAMS6[] = { RECORD_DEF(LQ_RELAY_DATA_RECORDS), "" }, { "lq-client-link", D6O_LQ_CLIENT_LINK, OPT_IPV6_ADDRESS_TYPE, true, NO_RECORD_DEF, "" }, - { "dhcpv4-message", D6O_DHCPV4_MSG, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, - { "dhcp4o6-server-addr", D6O_DHCPV4_O_DHCPV6_SERVER, OPT_IPV6_ADDRESS_TYPE, true, - NO_RECORD_DEF, "" }, { "bootfile-url", D6O_BOOTFILE_URL, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" }, { "bootfile-param", D6O_BOOTFILE_PARAM, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "client-arch-type", D6O_CLIENT_ARCH_TYPE, OPT_UINT16_TYPE, true, NO_RECORD_DEF, "" }, @@ -356,6 +353,9 @@ const OptionDefParams OPTION_DEF_PARAMS6[] = { { "rsoo", D6O_RSOO, OPT_EMPTY_TYPE, false, NO_RECORD_DEF, "rsoo-opts" }, { "client-linklayer-addr", D6O_CLIENT_LINKLAYER_ADDR, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, + { "dhcpv4-message", D6O_DHCPV4_MSG, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, + { "dhcp4o6-server-addr", D6O_DHCPV4_O_DHCPV6_SERVER, OPT_IPV6_ADDRESS_TYPE, true, + NO_RECORD_DEF, "" }, { "public-key", D6O_PUBLIC_KEY, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "certificate", D6O_CERTIFICATE, OPT_BINARY_TYPE, false, From e648d36192fe8c92244f77b606e7ca031823f9ac Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 16:47:51 +0900 Subject: [PATCH 16/50] [4105] Implement 4o6-interface-id parameter. --- src/bin/dhcp4/json_config_parser.cc | 13 +++++ src/bin/dhcp4/tests/config_parser_unittest.cc | 47 ++++++++++++++++++- src/lib/dhcpsrv/subnet.h | 7 ++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 27cc682c2f..43d5bb5251 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -204,6 +204,8 @@ protected: parser = new StringParser(config_id, string_values_); } else if (config_id.compare("4o6-interface") == 0) { parser = new StringParser(config_id, string_values_); + } else if (config_id.compare("4o6-interface-id") == 0) { + parser = new StringParser(config_id, string_values_); } else { isc_throw(NotImplemented, "unsupported parameter: " << config_id); } @@ -342,6 +344,17 @@ protected: // Don't care. 4o6-subnet is optional. } + // Try 4o6 specific paramter: 4o6-interface-id + try { + std::string ifaceid = string_values_->getParam("4o6-interface-id"); + OptionBuffer tmp(ifaceid.begin(), ifaceid.end()); + OptionPtr opt(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); + subnet4->get4o6().interface_id_ = opt; + subnet4->get4o6().enabled_ = true; + } catch (const DhcpConfigError&) { + + } + // Try setting up client class (if specified) try { string client_class = string_values_->getParam("client-class"); diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 50633286bd..413c98ec7d 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -3909,11 +3909,12 @@ TEST_F(Dhcp4ParserTest, 4o6subnetIface) { checkResult(status, 0); // Now check if the configuration was indeed handled and we have - // expected pool configured. + // expected subnet configured... Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); ASSERT_TRUE(subnet); + // ... and that subnet has 4o6 network interface specified. Cfg4o6& dhcp4o6 = subnet->get4o6(); EXPECT_TRUE(dhcp4o6.enabled_); EXPECT_EQ(IOAddress("2001:db8::543"), dhcp4o6.subnet4o6_.first); @@ -3921,4 +3922,48 @@ TEST_F(Dhcp4ParserTest, 4o6subnetIface) { EXPECT_EQ("ethX", dhcp4o6.iface4o6_); } +// Checks if the DHCPv4 is able to parse the configuration with 4o6 network +// interface-id. +TEST_F(Dhcp4ParserTest, 4o6subnetInterfaceId) { + + ConstElementPtr status; + + // Just a plain v4 config (no 4o6 parameters) + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-interface-id\": \"vlan123\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // check if returned status is OK + checkResult(status, 0); + + // Now check if the configuration was indeed handled and we have + // expected 4o6-interface-id configured. + Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> + getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); + ASSERT_TRUE(subnet); + + Cfg4o6& dhcp4o6 = subnet->get4o6(); + EXPECT_TRUE(dhcp4o6.enabled_); + OptionPtr ifaceid = dhcp4o6.interface_id_; + ASSERT_TRUE(ifaceid); + + vector data = ifaceid->getData(); + const char *exp_data = "vlan123"; + // Let's convert vlan123 to vector format. + // We need to skip the last \0 byte, thuse sizeof() - 1. + vector exp(exp_data, exp_data + sizeof(exp_data) - 1); + + EXPECT_TRUE(exp == data); +} + + } diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index caa86f4f19..f0c72ece5c 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -523,11 +523,14 @@ struct Cfg4o6 { /// Specifies if 4o6 is enabled on this subnet. bool enabled_; - /// Specifies the network interface used as subnet selector + /// Specifies the network interface used as v4 subnet selector. std::string iface4o6_; - /// Specifies the IPv6 subnet used for subnet selection + /// Specifies the IPv6 subnet used for v4 subnet selection. std::pair subnet4o6_; + + /// Specifies the v6 interface-id used for v4 subnet selection. + OptionPtr interface_id_; }; /// @brief A configuration holder for IPv4 subnet. From f2147a121434dbe26f42f866e868b7ff2897ecff Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 17:19:37 +0900 Subject: [PATCH 17/50] [4107] Minor comments update for options 100-102 --- src/lib/dhcp/dhcp6.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index 1380b1ae07..8764428812 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/src/lib/dhcp/dhcp6.h @@ -123,10 +123,9 @@ //#define D6O_4RD 97 /* RFC7600 */ //#define D6O_4RD_MAP_RULE 98 /* RFC7600 */ //#define D6O_4RD_NON_MAP_RULE 99 /* RFC7600 */ -/* draft-ietf-dhc-dhcpv6-active-leasequery-04 */ -//#define D6O_LQ_BASE_TIME 100 -//#define D6O_LQ_START_TIME 101 -//#define D6O_LQ_END_TIME 102 +//#define D6O_LQ_BASE_TIME 100 /* RFC7653 */ +//#define D6O_LQ_START_TIME 101 /* RFC7653 */ +//#define D6O_LQ_END_TIME 102 /* RFC7653 */ /* 103-142 unassigned */ //#define D6O_IPV6_ADDRESS_ANDSF 143 /* RFC6153 */ From 6b35611c3b06ecee3c2497472e6e6a2339ccc055 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 17:20:25 +0900 Subject: [PATCH 18/50] [4027] added unit tests for Pkt4o6Test (with other some small cleanups) --- src/lib/dhcp/pkt4o6.h | 2 +- src/lib/dhcp/pkt6.h | 2 +- src/lib/dhcp/tests/Makefile.am | 1 + src/lib/dhcp/tests/pkt4o6_unittest.cc | 98 +++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 src/lib/dhcp/tests/pkt4o6_unittest.cc diff --git a/src/lib/dhcp/pkt4o6.h b/src/lib/dhcp/pkt4o6.h index 4608ee55bb..70bf6496b2 100644 --- a/src/lib/dhcp/pkt4o6.h +++ b/src/lib/dhcp/pkt4o6.h @@ -63,7 +63,7 @@ public: return (true); } -protected: +private: /// Encapsulating DHCPv6 message Pkt6Ptr pkt6_; diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index 87dab71a67..e517c35eed 100644 --- a/src/lib/dhcp/pkt6.h +++ b/src/lib/dhcp/pkt6.h @@ -244,7 +244,7 @@ public: /// @param option_code code of the requested option /// @param nesting_level see description above /// - /// @return pointer to the option (or NULL if there is no such option) + /// @return pointer to the option (or NULL if there is no such option) OptionPtr getRelayOption(uint16_t option_code, uint8_t nesting_level); /// @brief Return first instance of a specified option diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am index 2e21b53f41..8faf432b14 100644 --- a/src/lib/dhcp/tests/Makefile.am +++ b/src/lib/dhcp/tests/Makefile.am @@ -74,6 +74,7 @@ libdhcp___unittests_SOURCES += option_vendor_class_unittest.cc libdhcp___unittests_SOURCES += pkt_captures4.cc pkt_captures6.cc pkt_captures.h libdhcp___unittests_SOURCES += pkt4_unittest.cc libdhcp___unittests_SOURCES += pkt6_unittest.cc +libdhcp___unittests_SOURCES += pkt4o6_unittest.cc libdhcp___unittests_SOURCES += pkt_filter_unittest.cc libdhcp___unittests_SOURCES += pkt_filter_inet_unittest.cc libdhcp___unittests_SOURCES += pkt_filter_inet6_unittest.cc diff --git a/src/lib/dhcp/tests/pkt4o6_unittest.cc b/src/lib/dhcp/tests/pkt4o6_unittest.cc new file mode 100644 index 0000000000..d75c2c03af --- /dev/null +++ b/src/lib/dhcp/tests/pkt4o6_unittest.cc @@ -0,0 +1,98 @@ +// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include + +#include +#include +#include +#include +#include + +#include + +#include + +using namespace isc::dhcp; + +namespace { +class Pkt4o6Test : public ::testing::Test { +protected: + Pkt4o6Test() : + data6_(6, 0), + pkt6_(new Pkt6(&data6_[0], data6_.size())), + pkt4_(new Pkt4(DHCPDISCOVER, 0x12345678)) + { + pkt4_->pack(); + const uint8_t* cp = static_cast( + pkt4_->getBuffer().getData()); + buffer4_.assign(cp, cp + pkt4_->getBuffer().getLength()); + } + +protected: + const std::vector data6_; + // commonly used test data + Pkt6Ptr pkt6_; + Pkt4Ptr pkt4_; + OptionBuffer buffer4_; +}; + +TEST_F(Pkt4o6Test, construct) { + // Construct 4o6 packet, unpack the data to examine it + boost::scoped_ptr pkt4o6(new Pkt4o6(buffer4_, pkt6_)); + pkt4o6->unpack(); + // Inspect its internal to confirm it's built as expected. We also test + // isDhcp4o6() here. + EXPECT_TRUE(pkt4o6->isDhcp4o6()); + EXPECT_EQ(pkt6_, pkt4o6->getPkt6()); + EXPECT_EQ(DHCPDISCOVER, pkt4o6->getType()); + + // Same check for the other constructor. It relies on the internal + // behavior of Pkt4's copy constructor, so we need to first unpack pkt4. + pkt4_.reset(new Pkt4(&buffer4_[0], buffer4_.size())); + pkt4_->unpack(); + pkt4o6.reset(new Pkt4o6(pkt4_, pkt6_)); + EXPECT_TRUE(pkt4o6->isDhcp4o6()); + EXPECT_EQ(pkt6_, pkt4o6->getPkt6()); + EXPECT_EQ(DHCPDISCOVER, pkt4o6->getType()); +} + +TEST_F(Pkt4o6Test, pack) { + // prepare unpacked DHCPv4 packet (see the note in constructor test) + pkt4_.reset(new Pkt4(&buffer4_[0], buffer4_.size())); + pkt4_->unpack(); + + // Construct 4o6 packet to be tested and pack the data. + Pkt4o6 pkt4o6(pkt4_, pkt6_); + pkt4o6.pack(); + + // The packed data should be: + // 4-byte DHCPv6 message header + // 4-byte header part of DHCPv4 message option + // Raw DHCPv4 message (data stored in buffer4_) + EXPECT_EQ(4 + 4 + buffer4_.size(), + pkt4o6.getPkt6()->getBuffer().getLength()); + + // Check the DHCPv4 message option content (Pkt4o6 class is not responsible + // for making it valid, so we won't examine it) + const u_int8_t* cp = static_cast( + pkt4o6.getPkt6()->getBuffer().getData()); + EXPECT_EQ(0, cp[4]); + EXPECT_EQ(D6O_DHCPV4_MSG, cp[5]); + EXPECT_EQ((buffer4_.size() >> 8) & 0xff, cp[6]); + EXPECT_EQ(buffer4_.size() & 0xff, cp[7]); + EXPECT_EQ(0, memcmp(&cp[8], &buffer4_[0], buffer4_.size())); +} + +} From 3435b6f64d03c8df5216d535356a5ab6f4ed8f31 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 17:22:49 +0900 Subject: [PATCH 19/50] [4027] added some more comments for tests --- src/lib/dhcp/tests/pkt4o6_unittest.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcp/tests/pkt4o6_unittest.cc b/src/lib/dhcp/tests/pkt4o6_unittest.cc index d75c2c03af..4d178b1530 100644 --- a/src/lib/dhcp/tests/pkt4o6_unittest.cc +++ b/src/lib/dhcp/tests/pkt4o6_unittest.cc @@ -41,11 +41,11 @@ protected: } protected: - const std::vector data6_; // commonly used test data - Pkt6Ptr pkt6_; - Pkt4Ptr pkt4_; - OptionBuffer buffer4_; + const std::vector data6_; // data for Pkt6 (content unimportant) + Pkt6Ptr pkt6_; // DHCPv6 message for 4o6 + Pkt4Ptr pkt4_; // DHCPv4 message for 4o6 + OptionBuffer buffer4_; // wire-format data buffer of pkt4_ }; TEST_F(Pkt4o6Test, construct) { From e15b2e1ebc65fac47b005088ed724f494ce30eb9 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 17:32:28 +0900 Subject: [PATCH 20/50] [4107] Added several comments. --- src/lib/dhcp/dhcp6.h | 6 +++--- src/lib/dhcp/std_option_defs.h | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index 8764428812..be4feeffc9 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/src/lib/dhcp/dhcp6.h @@ -222,7 +222,8 @@ extern const int dhcpv6_type_name_max; // Taken from http://www.iana.org/assignments/enterprise-numbers #define ENTERPRISE_ID_ISC 2495 -/* DHCPv4-over-DHCPv6 (RFC 7341) inter-process communication */ +/* DHCPv4-over-DHCPv6 (RFC 7341) inter-process communication. These are option + codes for the ISC vendor specific options used in 4o6 */ #define ISC_V6_4O6_INTERFACE 60000 #define ISC_V6_4O6_SRC_ADDRESS 60001 @@ -301,8 +302,7 @@ extern const int dhcpv6_type_name_max; #define IRT_DEFAULT 86400 #define IRT_MINIMUM 600 -/* DHCPv4-query message flags */ - +/* DHCPv4-query message flags (see RFC7341) */ #define DHCPV4_QUERY_FLAGS_UNICAST (1 << 23) #endif /* DHCP6_H */ diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h index a12f9d00f6..b9fbb98fc6 100644 --- a/src/lib/dhcp/std_option_defs.h +++ b/src/lib/dhcp/std_option_defs.h @@ -374,7 +374,12 @@ const OptionDefParams OPTION_DEF_PARAMS6[] = { const int OPTION_DEF_PARAMS_SIZE6 = sizeof(OPTION_DEF_PARAMS6) / sizeof(OPTION_DEF_PARAMS6[0]); -/// @brief Definitions of private DHCPv6 options +/// @brief Definitions of vendor-specific DHCPv6 options, defined by ISC. +/// 4o6-* options are used for inter-process communication. For details, see +/// http://kea.isc.org/wiki/Dhcp4o6Design +/// +/// @todo: As those options are defined by ISC, they do not belong in std_option_defs.h. +/// We need to move them to a separate file, e.g. isc_option_defs.h const OptionDefParams ISC_V6_DEFS[] = { { "4o6-interface", ISC_V6_4O6_INTERFACE, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" }, { "4o6-source-address", ISC_V6_4O6_SRC_ADDRESS, OPT_IPV6_ADDRESS_TYPE, false, NO_RECORD_DEF, "" } From 1084fad9535966a34ee5ae9982332850e7aa38e1 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 18:47:03 +0900 Subject: [PATCH 21/50] [4027] use a single underscore version of uint8_t for consistency (and perhaps the double-underscore version is non-standard) --- src/lib/dhcp/tests/pkt4o6_unittest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dhcp/tests/pkt4o6_unittest.cc b/src/lib/dhcp/tests/pkt4o6_unittest.cc index 4d178b1530..b6897b9c01 100644 --- a/src/lib/dhcp/tests/pkt4o6_unittest.cc +++ b/src/lib/dhcp/tests/pkt4o6_unittest.cc @@ -86,7 +86,7 @@ TEST_F(Pkt4o6Test, pack) { // Check the DHCPv4 message option content (Pkt4o6 class is not responsible // for making it valid, so we won't examine it) - const u_int8_t* cp = static_cast( + const uint8_t* cp = static_cast( pkt4o6.getPkt6()->getBuffer().getData()); EXPECT_EQ(0, cp[4]); EXPECT_EQ(D6O_DHCPV4_MSG, cp[5]); From b8b57816c2a31d220ef23efde9b29244f1bca24d Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 18:49:07 +0900 Subject: [PATCH 22/50] [4027] Minor corrections. --- src/lib/dhcp/pkt4o6.h | 6 +++--- src/lib/dhcp/tests/pkt4o6_unittest.cc | 10 +++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcp/pkt4o6.h b/src/lib/dhcp/pkt4o6.h index 70bf6496b2..e767a69047 100644 --- a/src/lib/dhcp/pkt4o6.h +++ b/src/lib/dhcp/pkt4o6.h @@ -34,7 +34,7 @@ public: /// @brief Constructor, used in message reception. /// - /// @param pkt4 DHCPv4 message + /// @param pkt4 Content of the DHCPv4-message option /// @param pkt6 encapsulating unpacked DHCPv6 message /// the DHCPv4 message option will be removed Pkt4o6(const OptionBuffer& pkt4, const Pkt6Ptr& pkt6); @@ -52,10 +52,10 @@ public: /// /// Calls pack() on both DHCPv4 and DHCPv6 parts /// Inserts the DHCPv4-message option - /// @ref pkt4::pack and @ref pkt6::pack + /// @ref Pkt4::pack and @ref Pkt6::pack virtual void pack(); - /// @brief Checks if a DHCPv4 message has beeb transported over DHCPv6 + /// @brief Checks if a DHCPv4 message has been transported over DHCPv6 /// /// @return Boolean value which indicates whether the message is /// transported over DHCPv6 (true) or native DHCPv4 (false) diff --git a/src/lib/dhcp/tests/pkt4o6_unittest.cc b/src/lib/dhcp/tests/pkt4o6_unittest.cc index 4d178b1530..e25c6fa48d 100644 --- a/src/lib/dhcp/tests/pkt4o6_unittest.cc +++ b/src/lib/dhcp/tests/pkt4o6_unittest.cc @@ -27,6 +27,9 @@ using namespace isc::dhcp; namespace { + +/// @brief A Fixture class dedicated to testing of the Pkt4o6 class that +/// represents a DHCPv4-over-DHCPv6 packet. class Pkt4o6Test : public ::testing::Test { protected: Pkt4o6Test() : @@ -48,6 +51,7 @@ protected: OptionBuffer buffer4_; // wire-format data buffer of pkt4_ }; +// This test verifies that the constructors are working as expected. TEST_F(Pkt4o6Test, construct) { // Construct 4o6 packet, unpack the data to examine it boost::scoped_ptr pkt4o6(new Pkt4o6(buffer4_, pkt6_)); @@ -68,6 +72,8 @@ TEST_F(Pkt4o6Test, construct) { EXPECT_EQ(DHCPDISCOVER, pkt4o6->getType()); } +// This test verifies that the pack() method handles the building +// process correctly. TEST_F(Pkt4o6Test, pack) { // prepare unpacked DHCPv4 packet (see the note in constructor test) pkt4_.reset(new Pkt4(&buffer4_[0], buffer4_.size())); @@ -86,7 +92,7 @@ TEST_F(Pkt4o6Test, pack) { // Check the DHCPv4 message option content (Pkt4o6 class is not responsible // for making it valid, so we won't examine it) - const u_int8_t* cp = static_cast( + const uint8_t* cp = static_cast( pkt4o6.getPkt6()->getBuffer().getData()); EXPECT_EQ(0, cp[4]); EXPECT_EQ(D6O_DHCPV4_MSG, cp[5]); @@ -95,4 +101,6 @@ TEST_F(Pkt4o6Test, pack) { EXPECT_EQ(0, memcmp(&cp[8], &buffer4_[0], buffer4_.size())); } +/// @todo: Add a test that handles actual DHCP4o6 traffic capture +/// once we get it. We should add the capture to pkt_captures{4,6}.cc } From 0fe570f5f161a0053f95b7d2afad6a43a4c7cd8b Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 18:57:59 +0900 Subject: [PATCH 23/50] [4027] added some references --- src/lib/dhcp/pkt4o6.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/dhcp/pkt4o6.h b/src/lib/dhcp/pkt4o6.h index e767a69047..0e87f8e112 100644 --- a/src/lib/dhcp/pkt4o6.h +++ b/src/lib/dhcp/pkt4o6.h @@ -29,6 +29,10 @@ namespace dhcp { /// This class derives from @c Pkt4 in order to be handled by /// the DHCPv4 server code. It includes a shared pointer to the /// DHCPv6 message too. +/// +/// This is an implementation of the DHCPv4-query/response DHCPv6 messages +/// defined in RFC 7341 (http://ietf.org/rfc/rfc7341.txt). +/// See also http://kea.isc.org/wiki/Dhcp4o6Design for design discussions. class Pkt4o6 : public Pkt4 { public: From 7709b1539364456173826e0f3acdc8e1c471f9c3 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 19:01:02 +0900 Subject: [PATCH 24/50] [4027] added some explanatory comments for Pkt4o6::pack(). --- src/lib/dhcp/pkt4o6.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/dhcp/pkt4o6.cc b/src/lib/dhcp/pkt4o6.cc index 7fb087a6e3..ca26cf98c1 100644 --- a/src/lib/dhcp/pkt4o6.cc +++ b/src/lib/dhcp/pkt4o6.cc @@ -48,10 +48,14 @@ Pkt4o6::Pkt4o6(const Pkt4Ptr& pkt4, const Pkt6Ptr& pkt6) } void Pkt4o6::pack() { + // Convert wire-format Pkt4 data in the form of OptionBuffer. Pkt4::pack(); OutputBuffer& buf = getBuffer(); const uint8_t* ptr = static_cast(buf.getData()); OptionBuffer msg(ptr, ptr + buf.getLength()); + + // Build the DHCPv4 Message option for the DHCPv6 message, and pack the + // entire stuff. OptionPtr dhcp4_msg(new Option(Option::V6, D6O_DHCPV4_MSG, msg)); pkt6_->addOption(dhcp4_msg); pkt6_->pack(); From 47a7a6d11f9a8567367e43ed180847e7b24b9848 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sat, 31 Oct 2015 19:02:23 +0900 Subject: [PATCH 25/50] [4027] removed an unused variable (besides, this type of file-static definition is dangerious - it can lead to static initialization fiasco). --- src/lib/dhcp/pkt4o6.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/lib/dhcp/pkt4o6.cc b/src/lib/dhcp/pkt4o6.cc index ca26cf98c1..2dd3d0ff25 100644 --- a/src/lib/dhcp/pkt4o6.cc +++ b/src/lib/dhcp/pkt4o6.cc @@ -25,12 +25,6 @@ using namespace isc::dhcp; using namespace isc::util; using namespace std; -namespace { - -/// @brief Default address used in Pkt4 constructor -const IOAddress DEFAULT_ADDRESS("0.0.0.0"); -} - namespace isc { namespace dhcp { From 5df470d063350a4002ccdf704ab1669262073f0b Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 31 Oct 2015 11:40:42 +0100 Subject: [PATCH 26/50] [master] Finished trac4027 aka Pkt4o6 merge --- src/lib/dhcp/Makefile.am | 2 +- src/lib/dhcp/pkt.cc | 8 ++++---- src/lib/dhcp/pkt4.h | 8 ++++++++ src/lib/dhcp/pkt6.h | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index 349070ec0c..b892192b9c 100644 --- a/src/lib/dhcp/Makefile.am +++ b/src/lib/dhcp/Makefile.am @@ -15,7 +15,7 @@ CLEANFILES = *.gcno *.gcda lib_LTLIBRARIES = libkea-dhcp++.la libkea_dhcp___la_SOURCES = libkea_dhcp___la_SOURCES += classify.cc classify.h -libkea_dhcp___la_SOURCES += dhcp6.h dhcp4.h dhcp4o6.h +libkea_dhcp___la_SOURCES += dhcp6.h dhcp4.h libkea_dhcp___la_SOURCES += duid.cc duid.h libkea_dhcp___la_SOURCES += hwaddr.cc hwaddr.h libkea_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index dfd1f8aa33..1af44ba989 100644 --- a/src/lib/dhcp/pkt.cc +++ b/src/lib/dhcp/pkt.cc @@ -50,10 +50,10 @@ Pkt::Pkt(const uint8_t* buf, uint32_t len, const isc::asiolink::IOAddress& local { if (len != 0) { - if (buf == NULL) { - isc_throw(InvalidParameter, "data buffer passed to Pkt is NULL"); - } - data_.resize(len); + if (buf == NULL) { + isc_throw(InvalidParameter, "data buffer passed to Pkt is NULL"); + } + data_.resize(len); memcpy(&data_[0], buf, len); } } diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h index 0c7d009e16..a4c0cc2db9 100644 --- a/src/lib/dhcp/pkt4.h +++ b/src/lib/dhcp/pkt4.h @@ -375,6 +375,14 @@ public: /// (true) or non-relayed (false). bool isRelayed() const; + /// @brief Checks if a DHCPv4 message has beeb transported over DHCPv6 + /// + /// @return Boolean value which indicates whether the message is + /// transported over DHCPv6 (true) or native DHCPv4 (false) + virtual bool isDhcp4o6() const { + return (false); + } + private: /// @brief Generic method that validates and sets HW address. diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index e517c35eed..87dab71a67 100644 --- a/src/lib/dhcp/pkt6.h +++ b/src/lib/dhcp/pkt6.h @@ -244,7 +244,7 @@ public: /// @param option_code code of the requested option /// @param nesting_level see description above /// - /// @return pointer to the option (or NULL if there is no such option) + /// @return pointer to the option (or NULL if there is no such option) OptionPtr getRelayOption(uint16_t option_code, uint8_t nesting_level); /// @brief Return first instance of a specified option From 51d5c00f676c71e534659f106c9350d9a1285b83 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 20:24:04 +0900 Subject: [PATCH 27/50] [4105] Changes after review: - getters/setters implemented in Cfg4o6. - extra space removed. --- src/bin/dhcp4/json_config_parser.cc | 12 ++--- src/bin/dhcp4/tests/config_parser_unittest.cc | 24 ++++----- src/lib/dhcpsrv/subnet.h | 52 ++++++++++++++++++- 3 files changed, 69 insertions(+), 19 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 43d5bb5251..df4550841b 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -314,8 +314,8 @@ protected: // Try 4o6 specific parameter: 4o6-interface try { string iface4o6 = string_values_->getParam("4o6-interface"); - subnet4->get4o6().iface4o6_ = iface4o6; - subnet4->get4o6().enabled_ = true; + subnet4->get4o6().setIface4o6(iface4o6); + subnet4->get4o6().enabled(true); } catch (const DhcpConfigError&) { // Don't care. 4o6-subnet is optional. } @@ -338,8 +338,8 @@ protected: isc_throw(DhcpConfigError, "Invalid prefix length specified in " "4o6-subnet parameter: " + subnet4o6 + ", expected 0..128 value"); } - subnet4->get4o6().subnet4o6_ = make_pair(IOAddress(prefix), len); - subnet4->get4o6().enabled_ = true; + subnet4->get4o6().setSubnet4o6(IOAddress(prefix), len); + subnet4->get4o6().enabled(true); } catch (const DhcpConfigError&) { // Don't care. 4o6-subnet is optional. } @@ -349,8 +349,8 @@ protected: std::string ifaceid = string_values_->getParam("4o6-interface-id"); OptionBuffer tmp(ifaceid.begin(), ifaceid.end()); OptionPtr opt(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); - subnet4->get4o6().interface_id_ = opt; - subnet4->get4o6().enabled_ = true; + subnet4->get4o6().setInterfaceId(opt); + subnet4->get4o6().enabled(true); } catch (const DhcpConfigError&) { } diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 413c98ec7d..1cfdc7ddcf 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -3812,7 +3812,7 @@ TEST_F(Dhcp4ParserTest, 4o6default) { ASSERT_TRUE(subnet); Cfg4o6& dhcp4o6 = subnet->get4o6(); - EXPECT_FALSE(dhcp4o6.enabled_); + EXPECT_FALSE(dhcp4o6.enabled()); } // Checks if the DHCPv4 is able to parse the configuration with 4o6 subnet @@ -3845,9 +3845,9 @@ TEST_F(Dhcp4ParserTest, 4o6subnet) { ASSERT_TRUE(subnet); Cfg4o6& dhcp4o6 = subnet->get4o6(); - EXPECT_TRUE(dhcp4o6.enabled_); - EXPECT_EQ(IOAddress("2001:db8::123"), dhcp4o6.subnet4o6_.first); - EXPECT_EQ(45, dhcp4o6.subnet4o6_.second); + EXPECT_TRUE(dhcp4o6.enabled()); + EXPECT_EQ(IOAddress("2001:db8::123"), dhcp4o6.getSubnet4o6().first); + EXPECT_EQ(45, dhcp4o6.getSubnet4o6().second); } // Checks if the DHCPv4 is able to parse the configuration with 4o6 network @@ -3880,8 +3880,8 @@ TEST_F(Dhcp4ParserTest, 4o6iface) { ASSERT_TRUE(subnet); Cfg4o6& dhcp4o6 = subnet->get4o6(); - EXPECT_TRUE(dhcp4o6.enabled_); - EXPECT_EQ("ethX", dhcp4o6.iface4o6_); + EXPECT_TRUE(dhcp4o6.enabled()); + EXPECT_EQ("ethX", dhcp4o6.getIface4o6()); } // Checks if the DHCPv4 is able to parse the configuration with both 4o6 network @@ -3916,10 +3916,10 @@ TEST_F(Dhcp4ParserTest, 4o6subnetIface) { // ... and that subnet has 4o6 network interface specified. Cfg4o6& dhcp4o6 = subnet->get4o6(); - EXPECT_TRUE(dhcp4o6.enabled_); - EXPECT_EQ(IOAddress("2001:db8::543"), dhcp4o6.subnet4o6_.first); - EXPECT_EQ(21, dhcp4o6.subnet4o6_.second); - EXPECT_EQ("ethX", dhcp4o6.iface4o6_); + EXPECT_TRUE(dhcp4o6.enabled()); + EXPECT_EQ(IOAddress("2001:db8::543"), dhcp4o6.getSubnet4o6().first); + EXPECT_EQ(21, dhcp4o6.getSubnet4o6().second); + EXPECT_EQ("ethX", dhcp4o6.getIface4o6()); } // Checks if the DHCPv4 is able to parse the configuration with 4o6 network @@ -3952,8 +3952,8 @@ TEST_F(Dhcp4ParserTest, 4o6subnetInterfaceId) { ASSERT_TRUE(subnet); Cfg4o6& dhcp4o6 = subnet->get4o6(); - EXPECT_TRUE(dhcp4o6.enabled_); - OptionPtr ifaceid = dhcp4o6.interface_id_; + EXPECT_TRUE(dhcp4o6.enabled()); + OptionPtr ifaceid = dhcp4o6.getInterfaceId(); ASSERT_TRUE(ifaceid); vector data = ifaceid->getData(); diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index f0c72ece5c..54ce6a6ee0 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -520,6 +520,57 @@ struct Cfg4o6 { :enabled_(false), subnet4o6_(std::make_pair(asiolink::IOAddress("::"), 128u)) { } + /// @brief Returns whether the DHCP4o6 is enabled or not. + /// @return true if enabled + bool enabled() const { + return (enabled_); + } + + /// @brief Sets the DHCP4o6 enabled status. + /// @param enabled specifies if the DHCP4o6 should be enabled or not + void enabled(bool enabled) { + enabled_ = enabled; + } + + /// @brief Returns the DHCP4o6 interface. + /// @return value of the 4o6-interface parameter. + std::string getIface4o6() const { + return (iface4o6_); + } + + /// @brief Sets the 4o6-interface. + /// @param iface name of the network interface the 4o6 traffic is received on + void setIface4o6(const std::string& iface) { + iface4o6_ = iface; + } + + /// @brief Returns prefix/len for the IPv6 subnet. + /// @return prefix/length pair + std::pair getSubnet4o6() const { + return (subnet4o6_); + } + + /// @brief Sets the prefix/length information (content of the 4o6-subnet). + /// @param subnet IOAddress that represents a prefix + /// @param prefix specifies prefix length + void setSubnet4o6(const asiolink::IOAddress& subnet, uint8_t prefix) { + subnet4o6_ = std::make_pair(subnet, prefix); + } + + /// @brief Returns the interface-id. + /// @return the option representing interface-id (or NULL) + OptionPtr getInterfaceId() const { + return (interface_id_); + } + + /// @brief Sets the interface-id + /// @param opt option to be used as interface-id match + void setInterfaceId(const OptionPtr& opt) { + interface_id_ = opt; + } + +private: + /// Specifies if 4o6 is enabled on this subnet. bool enabled_; @@ -616,7 +667,6 @@ private: /// lookup. bool match_client_id_; - /// @brief All the information related to DHCP4o6 Cfg4o6 dhcp4o6_; }; From 115a4071483ebdc74d5061dd68e2a08bf6f02f29 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 20:33:29 +0900 Subject: [PATCH 28/50] [4105] config parser now uses getOptionalParam() methods. --- src/bin/dhcp4/json_config_parser.cc | 18 ++---- src/bin/dhcp4/tests/config_parser_unittest.cc | 57 +++++++++++++++++++ 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index df4550841b..5f83f7064f 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -312,17 +312,15 @@ protected: } // Try 4o6 specific parameter: 4o6-interface - try { - string iface4o6 = string_values_->getParam("4o6-interface"); + string iface4o6 = string_values_->getOptionalParam("4o6-interface", ""); + if (!iface4o6.empty()) { subnet4->get4o6().setIface4o6(iface4o6); subnet4->get4o6().enabled(true); - } catch (const DhcpConfigError&) { - // Don't care. 4o6-subnet is optional. } // Try 4o6 specific parameter: 4o6-subnet - try { - string subnet4o6 = string_values_->getParam("4o6-subnet"); + string subnet4o6 = string_values_->getOptionalParam("4o6-subnet", ""); + if (!subnet4o6.empty()) { size_t slash = subnet4o6.find("/"); if (slash == std::string::npos) { isc_throw(DhcpConfigError, "Missing / in the 4o6-subnet parameter:" @@ -340,19 +338,15 @@ protected: } subnet4->get4o6().setSubnet4o6(IOAddress(prefix), len); subnet4->get4o6().enabled(true); - } catch (const DhcpConfigError&) { - // Don't care. 4o6-subnet is optional. } // Try 4o6 specific paramter: 4o6-interface-id - try { - std::string ifaceid = string_values_->getParam("4o6-interface-id"); + std::string ifaceid = string_values_->getOptionalParam("4o6-interface-id", ""); + if (!ifaceid.empty()) { OptionBuffer tmp(ifaceid.begin(), ifaceid.end()); OptionPtr opt(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); subnet4->get4o6().setInterfaceId(opt); subnet4->get4o6().enabled(true); - } catch (const DhcpConfigError&) { - } // Try setting up client class (if specified) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 1cfdc7ddcf..2af67cbe99 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -3850,6 +3850,63 @@ TEST_F(Dhcp4ParserTest, 4o6subnet) { EXPECT_EQ(45, dhcp4o6.getSubnet4o6().second); } +// Checks if the DHCPv4 is able to parse the configuration with 4o6 subnet +// defined. +TEST_F(Dhcp4ParserTest, 4o6subnetBogus) { + + ConstElementPtr status; + + // Just a plain v4 config (no 4o6 parameters) + string config[] = { + // Bogus configuration 1: missing / in subnet + "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-subnet\": \"2001:db8::123\" } ]," + "\"valid-lifetime\": 4000 }", + + // Bogus configuration 2: incorrect address + "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-subnet\": \"2001:db8:bogus/45\" } ]," + "\"valid-lifetime\": 4000 }", + + // Bogus configuration 3: incorrect prefix lenght + "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"4o6-subnet\": \"2001:db8::123/200\" } ]," + "\"valid-lifetime\": 4000 }" + }; + + ElementPtr json1 = Element::fromJSON(config[0]); + ElementPtr json2 = Element::fromJSON(config[0]); + ElementPtr json3 = Element::fromJSON(config[0]); + + // Check that the first config is rejected. + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json1)); + checkResult(status, 1); + + // Check that the second config is rejected. + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json2)); + checkResult(status, 1); + + // Check that the third config is rejected. + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json3)); + checkResult(status, 1); +} + + // Checks if the DHCPv4 is able to parse the configuration with 4o6 network // interface defined. TEST_F(Dhcp4ParserTest, 4o6iface) { From 7e2a37608137ad700312c053b895678caa756028 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 20:34:38 +0900 Subject: [PATCH 29/50] [4105] Grammar fixed. --- src/bin/dhcp4/tests/config_parser_unittest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 2af67cbe99..99a41334bf 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -3784,7 +3784,7 @@ TEST_F(Dhcp4ParserTest, expiredLeasesProcessingError) { // Checks if the DHCPv4 is able to parse the configuration without 4o6 parameters -// and do not set 4o6 fields at all. +// and does not set 4o6 fields at all. TEST_F(Dhcp4ParserTest, 4o6default) { ConstElementPtr status; From 89efd6970a604f562530a084e34a76c2fc6b8d10 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Sat, 31 Oct 2015 20:55:34 +0900 Subject: [PATCH 30/50] [master] AUTHORS updated after recent changes. --- AUTHORS | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 1bfc8458f1..d386688339 100755 --- a/AUTHORS +++ b/AUTHORS @@ -76,12 +76,15 @@ We have received the following contributions: - David Gutierrez Rueda, CERN 2014-12: Support for client link-address option in DHCPv6 (RFC6939) - - Adam Kalmus, Gdank University of Technology + - Adam Kalmus, Gdansk University of Technology 2014-12: Extract MAC address from DUID-LL and DUID-LLT types 2015-01: Extract MAC address from remote-id 2015-05: MySQL schema extended to cover host reservation 2015-10: Common MySQL Connector Pool + - Jinmei Tatuya + 2015-10: Pkt4o6 class improvements + Kea uses log4cplus (http://sourceforge.net/projects/log4cplus/) for logging, Boost (http://www.boost.org/) library for almost everything, and can use Botan (http://botan.randombit.net/) or OpenSSL (https://www.openssl.org/) for From 2a9eaa438e8cf23208d9af95ab8839208d59f442 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 1 Nov 2015 01:14:11 +0100 Subject: [PATCH 31/50] [3927] Fixed spurious 4 in DHCPv6 guide --- doc/guide/dhcp6-srv.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index a88c255697..731c16ec94 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -1028,7 +1028,7 @@ temporarily override a list of interface names and listen on all interfaces. optional. The array default value is false. The record-types and encapsulate default values are blank - (i.e. ""). The default space is "dhcp4". + (i.e. ""). The default space is "dhcp6". Once the new option format is defined, its value is set @@ -1068,7 +1068,7 @@ temporarily override a list of interface names and listen on all interfaces. "space": "dhcp6", "type": "record", "array": false, - "record-types": "ipv4-address, uint16, boolean, string", + "record-types": "ipv6-address, uint16, boolean, string", "encapsulate": "" }, ... ], From cedbc8083ee4be55671b5df6ccbc06834ae7baa9 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 1 Nov 2015 01:41:15 +0100 Subject: [PATCH 32/50] [3927] Added a defaultSpaceOptionDefTest unit test --- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 001a39303b..141e32c4f4 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -530,6 +530,39 @@ TEST_F(ParseConfigTest, minimalOptionDefTest) { EXPECT_TRUE(def->getEncapsulatedSpace().empty()); } +/// @brief Check parsing of option definitions using default dhcp6 space. +/// +/// Same than minimal but using the fact the default universe is V6 +/// so the default space is dhcp6 +TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) { + + // Configuration string. + std::string config = + "{ \"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 10000," + " \"type\": \"ipv6-address\"" + " } ]" + "}"; + + // Verify that the configuration string parses. + int rcode = parseConfiguration(config); + ASSERT_TRUE(rcode == 0); + + + // Verify that the option definition can be retrieved. + OptionDefinitionPtr def = + CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("dhcp6", 10000); + ASSERT_TRUE(def); + + // Verify that the option definition is correct. + EXPECT_EQ("foo", def->getName()); + EXPECT_EQ(10000, def->getCode()); + EXPECT_FALSE(def->getArrayType()); + EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, def->getType()); + EXPECT_TRUE(def->getEncapsulatedSpace().empty()); +} + /// @brief Check basic parsing of options. /// /// Note that this tests basic operation of the OptionDataListParser and From 81467147e2a1e8b7615fc597295b45c221199076 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 2 Nov 2015 12:30:58 +0900 Subject: [PATCH 33/50] [master] Use LeaseMgrFactory::instance() rather than constructor. This fixes issues reported by the coverity scans recently. --- src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc | 4 ++-- src/bin/dhcp4/tests/kea_controller_unittest.cc | 2 +- src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 4 ++-- src/bin/dhcp6/tests/kea_controller_unittest.cc | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 9cf01e236e..79a8531589 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -361,7 +361,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlLeasesReclaim) { time(NULL) - 100, SubnetID(1))); // Add leases to the database. - LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); ASSERT_NO_THROW(lease_mgr.addLease(lease0)); ASSERT_NO_THROW(lease_mgr.addLease(lease1)); @@ -419,7 +419,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlLeasesReclaimRemove) { time(NULL) - 100, SubnetID(1))); // Add leases to the database. - LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); ASSERT_NO_THROW(lease_mgr.addLease(lease0)); ASSERT_NO_THROW(lease_mgr.addLease(lease1)); diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 4e75770d5c..96dfa078ed 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -377,7 +377,7 @@ TEST_F(JSONFileBackendTest, timers) { lease_reclaimed->state_ = Lease4::STATE_EXPIRED_RECLAIMED; // Add leases to the database. - LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); ASSERT_NO_THROW(lease_mgr.addLease(lease_expired)); ASSERT_NO_THROW(lease_mgr.addLease(lease_reclaimed)); diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 967bdb2067..7428796271 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -436,7 +436,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlLeasesReclaim) { lease1->cltt_ = time(NULL) - 100; // Add leases to the database. - LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); ASSERT_NO_THROW(lease_mgr.addLease(lease0)); ASSERT_NO_THROW(lease_mgr.addLease(lease1)); @@ -498,7 +498,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlLeasesReclaimRemove) { lease1->cltt_ = time(NULL) - 100; // Add leases to the database. - LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); ASSERT_NO_THROW(lease_mgr.addLease(lease0)); ASSERT_NO_THROW(lease_mgr.addLease(lease1)); diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index 4778670673..e00c9c9809 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -321,7 +321,7 @@ TEST_F(JSONFileBackendTest, timers) { lease_reclaimed->state_ = Lease6::STATE_EXPIRED_RECLAIMED; // Add leases to the database. - LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); ASSERT_NO_THROW(lease_mgr.addLease(lease_expired)); ASSERT_NO_THROW(lease_mgr.addLease(lease_reclaimed)); From 4a91a45e5bb37f3233e27bf5e43587fe14eea3d2 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Mon, 2 Nov 2015 05:10:45 +0100 Subject: [PATCH 34/50] [master] Fixed 4o6subnetInterfaceId for not 64 bit machines (from hackathon94) --- src/bin/dhcp4/tests/config_parser_unittest.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 99a41334bf..18123e380f 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -4014,13 +4014,9 @@ TEST_F(Dhcp4ParserTest, 4o6subnetInterfaceId) { ASSERT_TRUE(ifaceid); vector data = ifaceid->getData(); - const char *exp_data = "vlan123"; - // Let's convert vlan123 to vector format. - // We need to skip the last \0 byte, thuse sizeof() - 1. - vector exp(exp_data, exp_data + sizeof(exp_data) - 1); - - EXPECT_TRUE(exp == data); + EXPECT_EQ(7, data.size()); + const char *exp = "vlan123"; + EXPECT_EQ(0, memcmp(&data[0], exp, data.size())); } - } From b36a613740284c5197e99b8fdd3a78bcad3202b3 Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Mon, 2 Nov 2015 11:22:23 -0800 Subject: [PATCH 35/50] [trac4090] Update per review comments. Update per the review comments except for the logging stuff which will be done next. --- ChangeLog | 4 +++ src/lib/eval/eval_messages.mes | 3 +- src/lib/eval/tests/token_unittest.cc | 9 ++--- src/lib/eval/token.cc | 19 ++++++----- src/lib/eval/token.h | 51 ++++++++++++++++------------ 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/ChangeLog b/ChangeLog index cd2ad8527b..8058ef04bf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +XXXX. [func] sar + Added client classification substring token. + (Trac #4090, git + 1041. [func] tomek A new library, libkea-eval has been edded. It is not functional yet, but its purpose is to provide a generic expression diff --git a/src/lib/eval/eval_messages.mes b/src/lib/eval/eval_messages.mes index 1ce1ca3d12..3d86f758b4 100644 --- a/src/lib/eval/eval_messages.mes +++ b/src/lib/eval/eval_messages.mes @@ -21,4 +21,5 @@ client classification expressions. % EVAL_SUBSTRING_BAD_PARAM_CONVERSION starting %1, length %2 This debug message indicates that the parameter for the starting postion -or length of the substring couldn't be converted to an integer. +or length of the substring couldn't be converted to an integer. In this +case the substring routine returns an empty string. diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index d3bb997bf3..273bb37fef 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/src/lib/eval/tests/token_unittest.cc @@ -233,7 +233,7 @@ TEST_F(TokenTest, optionEqualTrue) { }; // This test checks if an a token representing a substring request -// throws an excpetion if there aren't enough values on the stack. +// throws an exception if there aren't enough values on the stack. // The stack from the top is: length, start, string. // The actual packet is not used. TEST_F(TokenTest, substringNotEnoughValues) { @@ -340,8 +340,9 @@ TEST_F(TokenTest, substringBadParams) { verifySubstringEval("foobar", "0", "allaboard", ""); } -// lastly check that we don't get anything if the string is empty -TEST_F(TokenTest, substringStartingEmpty) { +// lastly check that we don't get anything if the string is empty or +// we don't ask for any characters from it. +TEST_F(TokenTest, substringReturnEmpty) { verifySubstringEval("", "0", "all", ""); verifySubstringEval("foobar", "0", "0", ""); } @@ -352,7 +353,7 @@ TEST_F(TokenTest, substringStartingEmpty) { // result on the bottom with the substring result on next. // Evaulating the equals should produce true for the first // and false for the second. -// throws an excpetion if there aren't enough values on the stack. +// throws an exception if there aren't enough values on the stack. // The stack from the top is: length, start, string. // The actual packet is not used. TEST_F(TokenTest, substringEquals) { diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 3d3266a7dd..4701976865 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -76,14 +76,15 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) { return; } - // Convert the starting position and legnth from strings to numbers + // Convert the starting position and length from strings to numbers // the length may also be "all" in which case simply make it the - // legnth of the string. + // length of the string. // If we have a problem push an empty string and leave - int start_pos, length; + int start_pos; + int length; try { start_pos = boost::lexical_cast(start_str); - if ("all" == len_str) { + if (len_str == "all") { length = string_str.length(); } else { length = boost::lexical_cast(len_str); @@ -100,19 +101,19 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) { return; } + const int string_length = string_str.length(); // If the starting postion is outside of the string push an // empty string and leave - if (((start_pos >= 0) && (start_pos >= string_str.length())) | - ((start_pos < 0) && (-start_pos > string_str.length()))) { + if ((start_pos < -string_length) || (start_pos >= string_length)) { values.push(""); return; } // Adjust the values to be something for substr. We first figure out - // the staring postion, then update it and the length to get the - // characters before or after it depnding on the sign of length + // the starting postion, then update it and the length to get the + // characters before or after it depending on the sign of length if (start_pos < 0) { - start_pos = string_str.length() + start_pos; + start_pos = string_length + start_pos; } if (length < 0) { diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index dee4c46212..1e34051225 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -150,10 +150,10 @@ public: /// either "true" or "false". It requires at least two parameters to be /// present on stack. /// - /// @throw EvalBadStack if there's less than 2 values on stack + /// @throw EvalBadStack if there are less than 2 values on stack /// - /// @brief pkt (unused) - /// @brief values - stack of values (2 arguments will be poped, 1 result + /// @param pkt (unused) + /// @param values - stack of values (2 arguments will be popped, 1 result /// will be pushed) void evaluate(const Pkt& pkt, ValueStack& values); }; @@ -170,36 +170,45 @@ public: /// @brief Extract a substring from a string /// - /// Evaluation does not use packet information, but rather consumes the last - /// three parameters and requires at least three parameters to be present on - /// stack. From the top it expects the values on the stack as: - /// len - /// start - /// str + /// Evaluation does not use packet information. It requires at least + /// three values to be present on the stack. It will consume the top + /// three values on the stack as parameters and push the resulting substring + /// onto the stack. From the top it expects the values on the stack as: + /// - len + /// - start + /// - str /// - /// str is the string to extract a substring from. If it is empty an empty + /// str is the string to extract a substring from. If it is empty, an empty /// string is pushed onto the value stack. /// /// start is the postion from which the code starts extracting the substring. - /// 0 is before the first character and a negative number starts from the end, - /// with -1 being before the last character. If the starting point is - /// outside of the original string an empty string is pushed onto the value - /// stack. + /// 0 is the first character and a negative number starts from the end, with + /// -1 being the last character. If the starting point is outside of the + /// original string an empty string is pushed onto the value stack. /// /// length is the number of characters from the string to extract. /// "all" means all remaining characters from start to the end of string. /// A negative number means to go from start towards the beginning of - /// string, but doesn't include start. + /// the string, but doesn't include start. /// If length is longer than the remaining portion of string /// then the entire remaining portion is placed on the value stack. - /// Note: a negative value of length only indicates which characters to - /// extract, it does NOT indicate any attempt to reverse the string. - /// For example substring("foobar", 4, -2) will result in "ob". /// - /// @throw EvalBadStack if there's less than 3 values on stack + /// The following examples all use the base string "foobar", the first number + /// is the starting position and the second is the length. Note that + /// a negative length only selects which characters to extract it does not + /// indicate an attempt to reverse the string. + /// - 0, all => "foobar" + /// - 0, 6 => "foobar" + /// - 0, 4 => "foob" + /// - 2, all => "obar" + /// - 2, 6 => "obar" + /// - -1, all => "r" + /// - -1, -4 => "ooba" /// - /// @brief pkt (unused) - /// @brief values - stack of values (3 arguments will be poped, 1 result + /// @throw EvalBadStack if there are less than 3 values on stack + /// + /// @param pkt (unused) + /// @param values - stack of values (3 arguments will be popped, 1 result /// will be pushed) void evaluate(const Pkt& pkt, ValueStack& values); }; From a32f2a2aaaa8a2fb71f049514f1b92205f0f4b12 Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Mon, 2 Nov 2015 13:10:06 -0800 Subject: [PATCH 36/50] [trac4090] Setup logger for eval --- doc/guide/logging.xml | 10 ++++++++++ src/lib/eval/Makefile.am | 11 +++++++++-- src/lib/eval/tests/Makefile.am | 2 ++ src/lib/eval/token.cc | 5 ++--- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/doc/guide/logging.xml b/doc/guide/logging.xml index 63c9cf3a38..0924c152ac 100644 --- a/doc/guide/logging.xml +++ b/doc/guide/logging.xml @@ -219,6 +219,11 @@ kea-dhcp4.dhcpsrv - this is a base logger for the libdhcpsrv library. + + kea-dhcp4.eval - this logger is used + to log messages relating to the evaluation code, primarily used + by the client classification routines. + kea-dhcp4.hooks - this logger is used to log messages related to management of hooks libraries, e.g. @@ -302,6 +307,11 @@ kea-dhcp6.dhcpsrv - this is a base logger for the libdhcpsrv library. + + kea-dhcp6.eval - this logger is used + to log messages relating to the evaluation code, primarily used + by the client classification routines. + kea-dhcp6.hooks - this logger is used to log messages related to management of hooks libraries, e.g. diff --git a/src/lib/eval/Makefile.am b/src/lib/eval/Makefile.am index 1fd66b96f8..b8a833c9af 100644 --- a/src/lib/eval/Makefile.am +++ b/src/lib/eval/Makefile.am @@ -12,14 +12,20 @@ AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG) lib_LTLIBRARIES = libkea-eval.la libkea_eval_la_SOURCES = +libkea_eval_la_SOURCES += eval_log.cc eval_log.h libkea_eval_la_SOURCES += token.cc token.h +nodist_libkea_eval_la_SOURCES = eval_messages.h eval_messages.cc + libkea_eval_la_CXXFLAGS = $(AM_CXXFLAGS) libkea_eval_la_CPPFLAGS = $(AM_CPPFLAGS) libkea_eval_la_LIBADD = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la libkea_eval_la_LIBADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la +libkea_eval_la_LIBADD += $(top_builddir)/src/lib/log/libkea-log.la +libkea_eval_la_LIBADD += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS) + libkea_eval_la_LDFLAGS = -no-undefined -version-info 3:0:0 -libkea_eval_la_LDFLAGS += $(LOG4CPLUS_LIBS) $(CRYPTO_LDFLAGS) +libkea_eval_la_LDFLAGS += $(CRYPTO_LDFLAGS) EXTRA_DIST = eval.dox EXTRA_DIST += eval_messages.mes @@ -29,6 +35,7 @@ eval_messages.h eval_messages.cc: s-messages s-messages: eval_messages.mes $(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/eval/eval_messages.mes + touch $@ # Tell Automake that the eval_messages.{cc,h} source files are created in the # build process, so it must create these before doing anything else. Although @@ -39,4 +46,4 @@ s-messages: eval_messages.mes # first. BUILT_SOURCES = eval_messages.h eval_messages.cc -CLEANFILES = eval_messages.h eval_messages.cc +CLEANFILES = eval_messages.h eval_messages.cc s-messages diff --git a/src/lib/eval/tests/Makefile.am b/src/lib/eval/tests/Makefile.am index d220b2be46..c127429b61 100644 --- a/src/lib/eval/tests/Makefile.am +++ b/src/lib/eval/tests/Makefile.am @@ -2,6 +2,8 @@ SUBDIRS = . AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) +AM_CPPFLAGS += -DLOGGING_SPEC_FILE=\"$(abs_top_srcdir)/src/lib/dhcpsrv/logging.spec\" + AM_CXXFLAGS = $(KEA_CXXFLAGS) # Some versions of GCC warn about some versions of Boost regarding diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 4701976865..5720158ca3 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -13,6 +13,7 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include #include #include @@ -90,13 +91,11 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) { length = boost::lexical_cast(len_str); } } catch (const boost::bad_lexical_cast&) { -#if 0 - // Logging not yet built LOG_DEBUG(eval_logger, EVAL_DBG_TRACE, EVAL_SUBSTRING_BAD_PARAM_CONVERSION) .arg(start_str) .arg(len_str); -#endif + values.push(""); return; } From 358927b0a2616be496002777dad09250202339b4 Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Mon, 2 Nov 2015 13:25:34 -0800 Subject: [PATCH 37/50] [trac4090] Add the forgotten eval_log.c and eval_log.h files --- src/lib/eval/eval_log.cc | 26 +++++++++++++++++++++ src/lib/eval/eval_log.h | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 src/lib/eval/eval_log.cc create mode 100644 src/lib/eval/eval_log.h diff --git a/src/lib/eval/eval_log.cc b/src/lib/eval/eval_log.cc new file mode 100644 index 0000000000..35128f6ae5 --- /dev/null +++ b/src/lib/eval/eval_log.cc @@ -0,0 +1,26 @@ +// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +/// Defines the logger used by the Eval (classification) code + +#include + +namespace isc { +namespace dhcp { + +isc::log::Logger eval_logger("eval"); + +} // namespace dhcp +} // namespace isc + diff --git a/src/lib/eval/eval_log.h b/src/lib/eval/eval_log.h new file mode 100644 index 0000000000..fb39198d25 --- /dev/null +++ b/src/lib/eval/eval_log.h @@ -0,0 +1,49 @@ +// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef EVAL_LOG_H +#define EVAL_LOG_H + +#include +#include + +namespace isc { +namespace dhcp { + +/// @brief Eval debug Logging levels +/// +/// Defines the levels used to output debug messages in the eval (classification) code. +/// Note that higher numbers equate to more verbose (and detailed) output. + +// The first level traces normal operations, +const int EVAL_DBG_TRACE = DBGLVL_TRACE_BASIC; + +// The next level traces each call to hook code. +const int EVAL_DBG_CALLS = DBGLVL_TRACE_BASIC_DATA; + +// Additional information on the calls. Report each call to a callout (even +// if there are multiple callouts on a hook) and each status return. +const int EVAL_DBG_EXTENDED_CALLS = DBGLVL_TRACE_DETAIL_DATA; + +/// @brief Eval Logger +/// +/// Define the logger used to log messages. We could define it in multiple +/// modules, but defining in a single module and linking to it saves time and +/// space. +extern isc::log::Logger eval_logger; + +} // namespace dhcp +} // namespace isc + +#endif // EVAL_LOG_H From d87d517b8edf0307d0d3a70c2be2ea19b398308f Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Mon, 2 Nov 2015 16:38:50 -0800 Subject: [PATCH 38/50] [trac4090] Update per second set of review comments Remove change log entry as it isn't needed Update eval log description in user guide. --- ChangeLog | 4 ---- doc/guide/logging.xml | 8 ++++---- src/lib/eval/Makefile.am | 1 + 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8058ef04bf..cd2ad8527b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,3 @@ -XXXX. [func] sar - Added client classification substring token. - (Trac #4090, git - 1041. [func] tomek A new library, libkea-eval has been edded. It is not functional yet, but its purpose is to provide a generic expression diff --git a/doc/guide/logging.xml b/doc/guide/logging.xml index 0924c152ac..25dd24749e 100644 --- a/doc/guide/logging.xml +++ b/doc/guide/logging.xml @@ -221,8 +221,8 @@ kea-dhcp4.eval - this logger is used - to log messages relating to the evaluation code, primarily used - by the client classification routines. + to log messages relating to the client classification expression + evaluation code. kea-dhcp4.hooks - this logger is used @@ -309,8 +309,8 @@ kea-dhcp6.eval - this logger is used - to log messages relating to the evaluation code, primarily used - by the client classification routines. + to log messages relating to the client classification expression + evaluation code. kea-dhcp6.hooks - this logger is used diff --git a/src/lib/eval/Makefile.am b/src/lib/eval/Makefile.am index b8a833c9af..05c4902e08 100644 --- a/src/lib/eval/Makefile.am +++ b/src/lib/eval/Makefile.am @@ -22,6 +22,7 @@ libkea_eval_la_CPPFLAGS = $(AM_CPPFLAGS) libkea_eval_la_LIBADD = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la libkea_eval_la_LIBADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la libkea_eval_la_LIBADD += $(top_builddir)/src/lib/log/libkea-log.la +libkea_eval_la_LIBADD += $(top_builddir)/src/lib/util/libkea-util.la libkea_eval_la_LIBADD += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS) libkea_eval_la_LDFLAGS = -no-undefined -version-info 3:0:0 From 97e6332fd0fd1f1da4e16c63610884dbffe28d4c Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Tue, 3 Nov 2015 09:48:11 +0100 Subject: [PATCH 39/50] [4091] Renamed main (cf #4114) --- src/lib/eval/tests/Makefile.am | 2 +- src/lib/eval/tests/{main.cc => run_unittests.cc} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/lib/eval/tests/{main.cc => run_unittests.cc} (100%) diff --git a/src/lib/eval/tests/Makefile.am b/src/lib/eval/tests/Makefile.am index c127429b61..c9bd2bcd13 100644 --- a/src/lib/eval/tests/Makefile.am +++ b/src/lib/eval/tests/Makefile.am @@ -26,7 +26,7 @@ if HAVE_GTEST TESTS += libeval_unittests -libeval_unittests_SOURCES = token_unittest.cc main.cc +libeval_unittests_SOURCES = token_unittest.cc run_unittests.cc libeval_unittests_CXXFLAGS = $(AM_CXXFLAGS) libeval_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) libeval_unittests_LDFLAGS = $(AM_LDFLAGS) $(CRYPTO_LDFLAGS) $(GTEST_LDFLAGS) diff --git a/src/lib/eval/tests/main.cc b/src/lib/eval/tests/run_unittests.cc similarity index 100% rename from src/lib/eval/tests/main.cc rename to src/lib/eval/tests/run_unittests.cc From 4e42ee425f77c48c1acea2d5b50e61e39623835d Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Tue, 3 Nov 2015 10:25:17 +0100 Subject: [PATCH 40/50] [4091] Implemented, need tests --- src/lib/eval/token.cc | 24 ++++++++++++++++++++++++ src/lib/eval/token.h | 26 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 5720158ca3..81126a2c19 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -26,6 +27,29 @@ TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { values.push(value_); } +void +TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { + // Transform string of hexadecimal digits into binary format + std::vector binary; + try { + // The decodeHex function expects that the string contains an + // even number of digits. If we don't meet this requirement, + // we have to insert a leading 0. + if (!repr_.empty() && repr_.length() % 2) { + repr_ = repr_.insert(0, "0"); + } + util::encode::decodeHex(repr_, binary); + } catch (...) { + values.push(""); + return; + } + // Convert to a string + std::string chars(binary.size(), '\0'); + std::memmove(&chars[0], &binary[0], binary.size()); + // Literals only push, nothing to pop + values.push(chars_); +} + void TokenOption::evaluate(const Pkt& pkt, ValueStack& values) { OptionPtr opt = pkt.getOption(option_code_); diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index 1e34051225..699d335eb5 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -101,6 +101,32 @@ protected: std::string value_; ///< Constant value }; +/// @brief Token representing a constant string in hexadecimal format +/// +/// This token holds value of a constant string giving in an hexadecimal +/// format, for instance 0x666f6f is "foo" +class TokenHexString : public Token { +public: + /// Value is set during token construction. + /// + /// @param str constant string to be represented + /// (must be a string of hexadecimal digits or decoding will fail) + TokenHexString(const std::string& str) + :repr_(str){ + } + + /// @brief Token evaluation (puts value of the constant string on + /// the stack after decoding or an empty string if decoding fails + /// (note it should not if the parser is correct) + /// + /// @param pkt (ignored) + /// @param values (represented string will be pushed here) + void evaluate(const Pkt& pkt, ValueStack& values); + +protected: + std::string repr_; ///< Constant value +}; + /// @brief Token that represents a value of an option /// /// This represents a reference to a given option, e.g. in the expression From 812b9eb21b4c06c49abaadbc8d3cf1f3ea53d7a8 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 4 Nov 2015 03:24:03 +0100 Subject: [PATCH 41/50] [3927] Addressed minor comments --- src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 141e32c4f4..59419df691 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -481,7 +481,7 @@ TEST_F(ParseConfigTest, basicOptionDefTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(rcode == 0); + ASSERT_EQ(0, rcode); // Verify that the option definition can be retrieved. @@ -514,7 +514,7 @@ TEST_F(ParseConfigTest, minimalOptionDefTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(rcode == 0); + ASSERT_EQ(0, rcode); // Verify that the option definition can be retrieved. @@ -547,7 +547,7 @@ TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(rcode == 0); + ASSERT_TRUE(0, rcode); // Verify that the option definition can be retrieved. @@ -590,13 +590,13 @@ TEST_F(ParseConfigTest, basicOptionDataTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(rcode == 0); + ASSERT_TRUE(0, rcode); // Verify that the option can be retrieved. OptionPtr opt_ptr = getOptionPtr("isc", 100); ASSERT_TRUE(opt_ptr); - // Verify that the option definition is correct. + // Verify that the option data is correct. std::string val = "type=00100, len=00004: 192.0.2.0 (ipv4-address)"; EXPECT_EQ(val, opt_ptr->toText()); @@ -624,13 +624,13 @@ TEST_F(ParseConfigTest, minimalOptionDataTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(rcode == 0); + ASSERT_TRUE(0, rcode); // Verify that the option can be retrieved. OptionPtr opt_ptr = getOptionPtr("isc", 100); ASSERT_TRUE(opt_ptr); - // Verify that the option definition is correct. + // Verify that the option data is correct. std::string val = "type=00100, len=00004: 192.0.2.0 (ipv4-address)"; EXPECT_EQ(val, opt_ptr->toText()); From 82912d2fafaafaaf3f7c63a742bae6414120fad7 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 4 Nov 2015 05:02:40 +0100 Subject: [PATCH 42/50] [3927] Fixed typo in previous change --- src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 59419df691..4840e6d339 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -547,7 +547,7 @@ TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(0, rcode); + ASSERT_EQ(0, rcode); // Verify that the option definition can be retrieved. @@ -590,7 +590,7 @@ TEST_F(ParseConfigTest, basicOptionDataTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(0, rcode); + ASSERT_EQ(0, rcode); // Verify that the option can be retrieved. OptionPtr opt_ptr = getOptionPtr("isc", 100); @@ -624,7 +624,7 @@ TEST_F(ParseConfigTest, minimalOptionDataTest) { // Verify that the configuration string parses. int rcode = parseConfiguration(config); - ASSERT_TRUE(0, rcode); + ASSERT_EQ(0, rcode); // Verify that the option can be retrieved. OptionPtr opt_ptr = getOptionPtr("isc", 100); From c7460e849258ec77cf1215a2baf840d98f1ab77b Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 4 Nov 2015 06:13:50 +0100 Subject: [PATCH 43/50] [master] Finished merge of trac3927 (default in option-def) --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index cd2ad8527b..91dd3a3706 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +1042. [doc] fdupont + User Guide: parameters having default values may be omitted in the + option definitions. + (Trac #3927, git xxx) + 1041. [func] tomek A new library, libkea-eval has been edded. It is not functional yet, but its purpose is to provide a generic expression From b8cfb9d265ce4a56e9cc51c2f74db536bdd4a4a1 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 4 Nov 2015 06:14:54 +0100 Subject: [PATCH 44/50] [master] Updated git hash --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 91dd3a3706..ebae6a6ea8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,7 @@ 1042. [doc] fdupont User Guide: parameters having default values may be omitted in the option definitions. - (Trac #3927, git xxx) + (Trac #3927, git c7460e849258ec77cf1215a2baf840d98f1ab77b) 1041. [func] tomek A new library, libkea-eval has been edded. It is not functional From e20f7d8334f2416886e8e362eee6ba1e0990dc10 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 4 Nov 2015 23:41:05 +0100 Subject: [PATCH 45/50] [4091] Missed changes --- src/lib/eval/tests/token_unittest.cc | 88 ++++++++++++++++++++++++++++ src/lib/eval/token.cc | 13 +++- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index 273bb37fef..53c731803b 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/src/lib/eval/tests/token_unittest.cc @@ -24,6 +24,8 @@ #include #include +#include + using namespace std; using namespace isc::dhcp; @@ -127,6 +129,92 @@ TEST_F(TokenTest, string6) { EXPECT_EQ("foo", values_.top()); } +// This simple test checks that a TokenHexString, representing a constant +// string coded in hexadecimal, can be used in Pkt4 evaluation. +// (The actual packet is not used) +TEST_F(TokenTest, hexstring4) { + TokenPtr empty; + TokenPtr bad; + TokenPtr bell; + TokenPtr foo; + TokenPtr cookie; + + // Store constant empty hexstring "" ("") in the TokenHexString object. + ASSERT_NO_THROW(empty.reset(new TokenHexString(""))); + // Store bad encoded hexstring "xabc" ("") in the TokenHexString object. + ASSERT_NO_THROW(bad.reset(new TokenHexString("xabc"))); + // Store hexstring with an odd number of hexdigits "7" ("\a") + ASSERT_NO_THROW(bell.reset(new TokenHexString("7"))); + // Store constant hexstring "666f6f" ("foo") in the TokenHexString object. + ASSERT_NO_THROW(foo.reset(new TokenHexString("666f6f"))); + // Store constant hexstring "63825363" (DHCP_OPTIONS_COOKIE) + ASSERT_NO_THROW(cookie.reset(new TokenHexString("63825363"))); + + // Make sure that tokens can be evaluated without exceptions. + ASSERT_NO_THROW(empty->evaluate(*pkt4_, values_)); + ASSERT_NO_THROW(bad->evaluate(*pkt4_, values_)); + ASSERT_NO_THROW(bell->evaluate(*pkt4_, values_)); + ASSERT_NO_THROW(foo->evaluate(*pkt4_, values_)); + ASSERT_NO_THROW(cookie->evaluate(*pkt4_, values_)); + + // Check that the evaluation put its value on the values stack. + ASSERT_EQ(5, values_.size()); + uint32_t expected = htonl(DHCP_OPTIONS_COOKIE); + EXPECT_EQ(4, values_.top().size()); + EXPECT_EQ(0, std::memcmp(&expected, &values_.top()[0], 4)); + values_.pop(); + EXPECT_EQ("foo", values_.top()); + values_.pop(); + EXPECT_EQ("\a", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); +} + +// This simple test checks that a TokenHexString, representing a constant +// string coded in hexadecimal, can be used in Pkt6 evaluation. +// (The actual packet is not used) +TEST_F(TokenTest, hexstring6) { + TokenPtr empty; + TokenPtr bad; + TokenPtr bell; + TokenPtr foo; + TokenPtr cookie; + + // Store constant empty hexstring "" ("") in the TokenHexString object. + ASSERT_NO_THROW(empty.reset(new TokenHexString(""))); + // Store bad encoded hexstring "xabc" ("") in the TokenHexString object. + ASSERT_NO_THROW(bad.reset(new TokenHexString("xabc"))); + // Store hexstring with an odd number of hexdigits "7" ("\a") + ASSERT_NO_THROW(bell.reset(new TokenHexString("7"))); + // Store constant hexstring "666f6f" ("foo") in the TokenHexString object. + ASSERT_NO_THROW(foo.reset(new TokenHexString("666f6f"))); + // Store constant hexstring "63825363" (DHCP_OPTIONS_COOKIE) + ASSERT_NO_THROW(cookie.reset(new TokenHexString("63825363"))); + + // Make sure that tokens can be evaluated without exceptions. + ASSERT_NO_THROW(empty->evaluate(*pkt6_, values_)); + ASSERT_NO_THROW(bad->evaluate(*pkt6_, values_)); + ASSERT_NO_THROW(bell->evaluate(*pkt6_, values_)); + ASSERT_NO_THROW(foo->evaluate(*pkt6_, values_)); + ASSERT_NO_THROW(cookie->evaluate(*pkt6_, values_)); + + // Check that the evaluation put its value on the values stack. + ASSERT_EQ(5, values_.size()); + uint32_t expected = htonl(DHCP_OPTIONS_COOKIE); + EXPECT_EQ(4, values_.top().size()); + EXPECT_EQ(0, std::memcmp(&expected, &values_.top()[0], 4)); + values_.pop(); + EXPECT_EQ("foo", values_.top()); + values_.pop(); + EXPECT_EQ("\a", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); +} + // This test checks if a token representing an option value is able to extract // the option from an IPv4 packet and properly store the option's value. TEST_F(TokenTest, optionString4) { diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 81126a2c19..63aa7b7c8f 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include using namespace isc::dhcp; @@ -28,14 +29,19 @@ TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { } void -TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { +TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { + // Eliminate the empty string case first + if (repr_.empty()) { + values.push(""); + return; + } // Transform string of hexadecimal digits into binary format std::vector binary; try { // The decodeHex function expects that the string contains an // even number of digits. If we don't meet this requirement, // we have to insert a leading 0. - if (!repr_.empty() && repr_.length() % 2) { + if (repr_.length() % 2) { repr_ = repr_.insert(0, "0"); } util::encode::decodeHex(repr_, binary); @@ -45,9 +51,10 @@ TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { } // Convert to a string std::string chars(binary.size(), '\0'); + // Note that binary.size() cannot be 0 std::memmove(&chars[0], &binary[0], binary.size()); // Literals only push, nothing to pop - values.push(chars_); + values.push(chars); } void From 85a118414f505a761bbb875ec9eb54aa4e63c0c6 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 5 Nov 2015 01:58:13 +0100 Subject: [PATCH 46/50] [4091] Better odd check --- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 2 +- src/lib/eval/token.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 462945f88f..d39763d91e 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -593,7 +593,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) { // The decodeHex function expects that the string contains an // even number of digits. If we don't meet this requirement, // we have to insert a leading 0. - if (!data_param.empty() && data_param.length() % 2) { + if (!data_param.empty() && ((data_param.length() % 2) != 0)) { data_param = data_param.insert(0, "0"); } util::encode::decodeHex(data_param, binary); diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 63aa7b7c8f..3592ffad35 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -41,7 +41,7 @@ TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { // The decodeHex function expects that the string contains an // even number of digits. If we don't meet this requirement, // we have to insert a leading 0. - if (repr_.length() % 2) { + if ((repr_.length() % 2) != 0) { repr_ = repr_.insert(0, "0"); } util::encode::decodeHex(repr_, binary); From a496523c86e18602a729e20ad03b51d12e66368f Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 5 Nov 2015 02:39:28 +0100 Subject: [PATCH 47/50] [4091] Addressed comments (0x prefix, deciding in constructor) --- src/lib/eval/tests/token_unittest.cc | 64 +++++++++++++++++++--------- src/lib/eval/token.cc | 40 ++++++++++------- src/lib/eval/token.h | 9 ++-- 3 files changed, 73 insertions(+), 40 deletions(-) diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index 53c731803b..7f2d653f68 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/src/lib/eval/tests/token_unittest.cc @@ -135,33 +135,41 @@ TEST_F(TokenTest, string6) { TEST_F(TokenTest, hexstring4) { TokenPtr empty; TokenPtr bad; + TokenPtr nodigit; + TokenPtr baddigit; TokenPtr bell; TokenPtr foo; TokenPtr cookie; // Store constant empty hexstring "" ("") in the TokenHexString object. ASSERT_NO_THROW(empty.reset(new TokenHexString(""))); - // Store bad encoded hexstring "xabc" ("") in the TokenHexString object. - ASSERT_NO_THROW(bad.reset(new TokenHexString("xabc"))); - // Store hexstring with an odd number of hexdigits "7" ("\a") - ASSERT_NO_THROW(bell.reset(new TokenHexString("7"))); - // Store constant hexstring "666f6f" ("foo") in the TokenHexString object. - ASSERT_NO_THROW(foo.reset(new TokenHexString("666f6f"))); - // Store constant hexstring "63825363" (DHCP_OPTIONS_COOKIE) - ASSERT_NO_THROW(cookie.reset(new TokenHexString("63825363"))); + // Store bad encoded hexstring "0abc" (""). + ASSERT_NO_THROW(bad.reset(new TokenHexString("0abc"))); + // Store hexstring with no digits "0x" (""). + ASSERT_NO_THROW(nodigit.reset(new TokenHexString("0x"))); + // Store hexstring with a bad hexdigit "0xxabc" (""). + ASSERT_NO_THROW(baddigit.reset(new TokenHexString("0xxabc"))); + // Store hexstring with an odd number of hexdigits "0x7" ("\a"). + ASSERT_NO_THROW(bell.reset(new TokenHexString("0x7"))); + // Store constant hexstring "0x666f6f" ("foo"). + ASSERT_NO_THROW(foo.reset(new TokenHexString("0x666f6f"))); + // Store constant hexstring "0x63825363" (DHCP_OPTIONS_COOKIE). + ASSERT_NO_THROW(cookie.reset(new TokenHexString("0x63825363"))); // Make sure that tokens can be evaluated without exceptions. ASSERT_NO_THROW(empty->evaluate(*pkt4_, values_)); ASSERT_NO_THROW(bad->evaluate(*pkt4_, values_)); + ASSERT_NO_THROW(nodigit->evaluate(*pkt4_, values_)); + ASSERT_NO_THROW(baddigit->evaluate(*pkt4_, values_)); ASSERT_NO_THROW(bell->evaluate(*pkt4_, values_)); ASSERT_NO_THROW(foo->evaluate(*pkt4_, values_)); ASSERT_NO_THROW(cookie->evaluate(*pkt4_, values_)); // Check that the evaluation put its value on the values stack. - ASSERT_EQ(5, values_.size()); + ASSERT_EQ(7, values_.size()); uint32_t expected = htonl(DHCP_OPTIONS_COOKIE); EXPECT_EQ(4, values_.top().size()); - EXPECT_EQ(0, std::memcmp(&expected, &values_.top()[0], 4)); + EXPECT_EQ(0, memcmp(&expected, &values_.top()[0], 4)); values_.pop(); EXPECT_EQ("foo", values_.top()); values_.pop(); @@ -170,6 +178,10 @@ TEST_F(TokenTest, hexstring4) { EXPECT_EQ("", values_.top()); values_.pop(); EXPECT_EQ("", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); } // This simple test checks that a TokenHexString, representing a constant @@ -178,33 +190,41 @@ TEST_F(TokenTest, hexstring4) { TEST_F(TokenTest, hexstring6) { TokenPtr empty; TokenPtr bad; + TokenPtr nodigit; + TokenPtr baddigit; TokenPtr bell; TokenPtr foo; TokenPtr cookie; // Store constant empty hexstring "" ("") in the TokenHexString object. ASSERT_NO_THROW(empty.reset(new TokenHexString(""))); - // Store bad encoded hexstring "xabc" ("") in the TokenHexString object. - ASSERT_NO_THROW(bad.reset(new TokenHexString("xabc"))); - // Store hexstring with an odd number of hexdigits "7" ("\a") - ASSERT_NO_THROW(bell.reset(new TokenHexString("7"))); - // Store constant hexstring "666f6f" ("foo") in the TokenHexString object. - ASSERT_NO_THROW(foo.reset(new TokenHexString("666f6f"))); - // Store constant hexstring "63825363" (DHCP_OPTIONS_COOKIE) - ASSERT_NO_THROW(cookie.reset(new TokenHexString("63825363"))); + // Store bad encoded hexstring "0abc" (""). + ASSERT_NO_THROW(bad.reset(new TokenHexString("0abc"))); + // Store hexstring with no digits "0x" (""). + ASSERT_NO_THROW(nodigit.reset(new TokenHexString("0x"))); + // Store hexstring with a bad hexdigit "0xxabc" (""). + ASSERT_NO_THROW(baddigit.reset(new TokenHexString("0xxabc"))); + // Store hexstring with an odd number of hexdigits "0x7" ("\a"). + ASSERT_NO_THROW(bell.reset(new TokenHexString("0x7"))); + // Store constant hexstring "0x666f6f" ("foo"). + ASSERT_NO_THROW(foo.reset(new TokenHexString("0x666f6f"))); + // Store constant hexstring "0x63825363" (DHCP_OPTIONS_COOKIE). + ASSERT_NO_THROW(cookie.reset(new TokenHexString("0x63825363"))); // Make sure that tokens can be evaluated without exceptions. ASSERT_NO_THROW(empty->evaluate(*pkt6_, values_)); ASSERT_NO_THROW(bad->evaluate(*pkt6_, values_)); + ASSERT_NO_THROW(nodigit->evaluate(*pkt6_, values_)); + ASSERT_NO_THROW(baddigit->evaluate(*pkt6_, values_)); ASSERT_NO_THROW(bell->evaluate(*pkt6_, values_)); ASSERT_NO_THROW(foo->evaluate(*pkt6_, values_)); ASSERT_NO_THROW(cookie->evaluate(*pkt6_, values_)); // Check that the evaluation put its value on the values stack. - ASSERT_EQ(5, values_.size()); + ASSERT_EQ(7, values_.size()); uint32_t expected = htonl(DHCP_OPTIONS_COOKIE); EXPECT_EQ(4, values_.top().size()); - EXPECT_EQ(0, std::memcmp(&expected, &values_.top()[0], 4)); + EXPECT_EQ(0, memcmp(&expected, &values_.top()[0], 4)); values_.pop(); EXPECT_EQ("foo", values_.top()); values_.pop(); @@ -213,6 +233,10 @@ TEST_F(TokenTest, hexstring6) { EXPECT_EQ("", values_.top()); values_.pop(); EXPECT_EQ("", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); + values_.pop(); + EXPECT_EQ("", values_.top()); } // This test checks if a token representing an option value is able to extract diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 3592ffad35..bef3b32723 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -28,33 +28,43 @@ TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { values.push(value_); } -void -TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { - // Eliminate the empty string case first - if (repr_.empty()) { - values.push(""); +TokenHexString::TokenHexString(const string& str) : value_("") { + // Check "0x" or "0x" in front + if ((str.size() < 2) || + (str[0] != '0') || + ((str[1] != 'x') && (str[1] != 'X'))) { return; } + string digits = str.substr(2); + + // Eliminate the no digit case first + if (digits.empty()) { + return; + } + // Transform string of hexadecimal digits into binary format - std::vector binary; + vector binary; try { // The decodeHex function expects that the string contains an // even number of digits. If we don't meet this requirement, // we have to insert a leading 0. - if ((repr_.length() % 2) != 0) { - repr_ = repr_.insert(0, "0"); + if ((digits.length() % 2) != 0) { + digits = digits.insert(0, "0"); } - util::encode::decodeHex(repr_, binary); + util::encode::decodeHex(digits, binary); } catch (...) { - values.push(""); return; } - // Convert to a string - std::string chars(binary.size(), '\0'); - // Note that binary.size() cannot be 0 - std::memmove(&chars[0], &binary[0], binary.size()); + + // Convert to a string (note that binary.size() cannot be 0) + value_.resize(binary.size()); + memmove(&value_[0], &binary[0], binary.size()); +} + +void +TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { // Literals only push, nothing to pop - values.push(chars); + values.push(value_); } void diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index 699d335eb5..07d6d0f56f 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -110,10 +110,9 @@ public: /// Value is set during token construction. /// /// @param str constant string to be represented - /// (must be a string of hexadecimal digits or decoding will fail) - TokenHexString(const std::string& str) - :repr_(str){ - } + /// (must be "0x" or "0X" followed by a string of hexadecimal digits + /// or decoding will fail) + TokenHexString(const std::string& str); /// @brief Token evaluation (puts value of the constant string on /// the stack after decoding or an empty string if decoding fails @@ -124,7 +123,7 @@ public: void evaluate(const Pkt& pkt, ValueStack& values); protected: - std::string repr_; ///< Constant value + std::string value_; ///< Constant value }; /// @brief Token that represents a value of an option From 5c780bdd6e201b34ee4ceb35c8a40ea43132b08e Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 5 Nov 2015 23:32:15 +0100 Subject: [PATCH 48/50] [4091] Changed to require at least one hexdigit --- src/lib/eval/token.cc | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index bef3b32723..ea01f11a76 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -29,19 +29,14 @@ TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) { } TokenHexString::TokenHexString(const string& str) : value_("") { - // Check "0x" or "0x" in front - if ((str.size() < 2) || + // Check string starts "0x" or "0x" and has at least one additional character. + if ((str.size() < 3) || (str[0] != '0') || ((str[1] != 'x') && (str[1] != 'X'))) { return; } string digits = str.substr(2); - // Eliminate the no digit case first - if (digits.empty()) { - return; - } - // Transform string of hexadecimal digits into binary format vector binary; try { From 406153af95404adb96296df09ec6033b484586e3 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 5 Nov 2015 23:37:26 +0100 Subject: [PATCH 49/50] [master] Finished merge of trac4091 (hex strings) --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index ebae6a6ea8..63dae0b7d1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +1043. [func] fdupont + Implemented support for hex strings in client classification. + (Trac #4091, git xxx) + 1042. [doc] fdupont User Guide: parameters having default values may be omitted in the option definitions. From 88788b9f79afda6f6d163c872b4b8f44fc9b91ea Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 5 Nov 2015 23:40:42 +0100 Subject: [PATCH 50/50] [master] Updated git hash --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 63dae0b7d1..50ee904cc4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,6 @@ 1043. [func] fdupont Implemented support for hex strings in client classification. - (Trac #4091, git xxx) + (Trac #4091, git 406153af95404adb96296df09ec6033b484586e3) 1042. [doc] fdupont User Guide: parameters having default values may be omitted in the