From: Francis Dupont Date: Sat, 17 Dec 2016 00:05:26 +0000 (+0100) Subject: [5017] Some fixes (including a not yet detected error in classify CONFIG[0]) using... X-Git-Tag: trac5088_base~1^2~18 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a28938503ff711994ade37baaf904993858f08ea;p=thirdparty%2Fkea.git [5017] Some fixes (including a not yet detected error in classify CONFIG[0]) using parser --- diff --git a/src/bin/dhcp4/dhcp4_lexer.ll b/src/bin/dhcp4/dhcp4_lexer.ll index e3aaffe4ce..1f33425905 100644 --- a/src/bin/dhcp4/dhcp4_lexer.ll +++ b/src/bin/dhcp4/dhcp4_lexer.ll @@ -882,6 +882,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} \"next-server\" { switch(driver.ctx_) { + case isc::dhcp::Parser4Context::DHCP4: case isc::dhcp::Parser4Context::SUBNET4: case isc::dhcp::Parser4Context::RESERVATIONS: case isc::dhcp::Parser4Context::CLIENT_CLASSES: diff --git a/src/bin/dhcp4/dhcp4_parser.yy b/src/bin/dhcp4/dhcp4_parser.yy index 7757165929..b9b51e34f2 100644 --- a/src/bin/dhcp4/dhcp4_parser.yy +++ b/src/bin/dhcp4/dhcp4_parser.yy @@ -399,12 +399,12 @@ decline_probation_period: DECLINE_PROBATION_PERIOD COLON INTEGER { }; echo_client_id: ECHO_CLIENT_ID COLON BOOLEAN { - ElementPtr echo(new BoolElement($3, ctx.loc2pos(@1))); + ElementPtr echo(new BoolElement($3, ctx.loc2pos(@3))); ctx.stack_.back()->set("echo-client-id", echo); }; match_client_id: MATCH_CLIENT_ID COLON BOOLEAN { - ElementPtr match(new BoolElement($3, ctx.loc2pos(@1))); + ElementPtr match(new BoolElement($3, ctx.loc2pos(@3))); ctx.stack_.back()->set("match-client-id", match); }; @@ -1124,7 +1124,7 @@ reservation_param: duid next_server: NEXT_SERVER { ctx.enter(ctx.NO_KEYWORD); } COLON STRING { - ElementPtr next_server(new StringElement($4, ctx.loc2pos(@1))); + ElementPtr next_server(new StringElement($4, ctx.loc2pos(@4))); ctx.stack_.back()->set("next-server", next_server); ctx.leave(); }; @@ -1132,7 +1132,7 @@ next_server: NEXT_SERVER { server_hostname: SERVER_HOSTNAME { ctx.enter(ctx.NO_KEYWORD); } COLON STRING { - ElementPtr srv(new StringElement($4, ctx.loc2pos(@1))); + ElementPtr srv(new StringElement($4, ctx.loc2pos(@4))); ctx.stack_.back()->set("server-hostname", srv); ctx.leave(); }; @@ -1140,7 +1140,7 @@ server_hostname: SERVER_HOSTNAME { boot_file_name: BOOT_FILE_NAME { ctx.enter(ctx.NO_KEYWORD); } COLON STRING { - ElementPtr bootfile(new StringElement($4, ctx.loc2pos(@1))); + ElementPtr bootfile(new StringElement($4, ctx.loc2pos(@4))); ctx.stack_.back()->set("boot-file-name", bootfile); ctx.leave(); }; @@ -1148,7 +1148,7 @@ boot_file_name: BOOT_FILE_NAME { ip_address: IP_ADDRESS { ctx.enter(ctx.NO_KEYWORD); } COLON STRING { - ElementPtr addr(new StringElement($4, ctx.loc2pos(@1))); + ElementPtr addr(new StringElement($4, ctx.loc2pos(@4))); ctx.stack_.back()->set("ip-address", addr); ctx.leave(); }; diff --git a/src/bin/dhcp4/tests/classify_unittest.cc b/src/bin/dhcp4/tests/classify_unittest.cc index 513c8a734e..54da33b286 100644 --- a/src/bin/dhcp4/tests/classify_unittest.cc +++ b/src/bin/dhcp4/tests/classify_unittest.cc @@ -69,8 +69,7 @@ const char* CONFIGS[] = { " \"name\": \"pxe4\"," " \"test\": \"option[93].hex == 0x0001\"," " \"boot-file-name\": \"ipxe.efi\"" - "}," - "]," + "}]," "\"subnet4\": [ { " " \"subnet\": \"10.0.0.0/24\", " " \"id\": 1," diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index b517abcb4a..03e47113bc 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -31,6 +31,7 @@ #include "marker_file.h" #include "test_libraries.h" #include "test_data_files_config.h" +#include "dhcp4_test_utils.h" #include #include @@ -207,25 +208,25 @@ public: params["space"] = DHCP4_OPTION_SPACE; params["code"] = "56"; params["data"] = "ABCDEF0105"; - params["csv-format"] = "False"; + params["csv-format"] = "false"; } else if (parameter == "space") { params["name"] = "dhcp-message"; params["space"] = param_value; params["code"] = "56"; params["data"] = "ABCDEF0105"; - params["csv-format"] = "False"; + params["csv-format"] = "false"; } else if (parameter == "code") { params["name"] = "dhcp-message"; params["space"] = DHCP4_OPTION_SPACE; params["code"] = param_value; params["data"] = "ABCDEF0105"; - params["csv-format"] = "False"; + params["csv-format"] = "false"; } else if (parameter == "data") { params["name"] = "dhcp-message"; params["space"] = DHCP4_OPTION_SPACE; params["code"] = "56"; params["data"] = param_value; - params["csv-format"] = "False"; + params["csv-format"] = "false"; } else if (parameter == "csv-format") { params["name"] = "dhcp-message"; params["space"] = DHCP4_OPTION_SPACE; @@ -352,7 +353,7 @@ public: const std::string& parameter) { ConstElementPtr x; std::string config = createConfigWithOption(param_value, parameter); - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json = parseDHCP4(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 1); EXPECT_TRUE(errorContainsPosition(x, "")); @@ -369,7 +370,7 @@ public: testInvalidOptionParam(const std::map& params) { ConstElementPtr x; std::string config = createConfigWithOption(params); - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json = parseDHCP4(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 1); EXPECT_TRUE(errorContainsPosition(x, "")); @@ -463,9 +464,10 @@ public: bool executeConfiguration(const std::string& config, const char* operation) { CfgMgr::instance().clear(); + ConstElementPtr json; ConstElementPtr status; try { - ElementPtr json = Element::fromJSON(config); + json = parseJSON(config); status = configureDhcp4Server(*srv_, json); } catch (const std::exception& ex) { ADD_FAILURE() << "Unable to " << operation << ". " @@ -601,11 +603,11 @@ public: // config update. TEST_F(Dhcp4ParserTest, version) { - ConstElementPtr x; - - EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, - Element::fromJSON("{\"version\": 0}"))); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4("{\"version\": 0}")); + ConstElementPtr x; + EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); // returned value must be 0 (configuration accepted) checkResult(x, 0); } @@ -617,10 +619,13 @@ TEST_F(Dhcp4ParserTest, bogusCommand) { ConstElementPtr x; EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, - Element::fromJSON("{\"bogus\": 5}"))); + parseJSON("{\"bogus\": 5}"))); // returned value must be 1 (configuration parse error) checkResult(x, 1); + + // it should be refused by syntax too + EXPECT_THROW(parseDHCP4("{\"bogus\": 5}"), Dhcp4ParseError); } /// The goal of this test is to verify if wrongly defined subnet will @@ -628,14 +633,15 @@ TEST_F(Dhcp4ParserTest, bogusCommand) { /// pool definition. TEST_F(Dhcp4ParserTest, emptySubnet) { - ConstElementPtr status; - - EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, - Element::fromJSON("{ " + genIfaceConfig() + "," + + ConstElementPtr json; + EXPECT_NO_THROW(json = parseDHCP4("{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ ], " - "\"valid-lifetime\": 4000 }"))); + "\"valid-lifetime\": 4000 }")); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // returned value should be 0 (success) checkResult(status, 0); @@ -648,7 +654,6 @@ TEST_F(Dhcp4ParserTest, emptySubnet) { /// Check that the renew-timer doesn't have to be specified, in which case /// it is marked unspecified in the Subnet. TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) { - ConstElementPtr status; string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " @@ -657,8 +662,10 @@ TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // returned value should be 0 (success) @@ -682,7 +689,6 @@ TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) { /// Check that the rebind-timer doesn't have to be specified, in which case /// it is marked unspecified in the Subnet. TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) { - ConstElementPtr status; string config = "{ " + genIfaceConfig() + "," + "\"renew-timer\": 1000, " @@ -691,8 +697,10 @@ TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // returned value should be 0 (success) @@ -716,8 +724,6 @@ TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) { /// parameter timer definitions. TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) { - ConstElementPtr status; - string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -726,8 +732,10 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // check if returned status is OK @@ -775,7 +783,8 @@ TEST_F(Dhcp4ParserTest, multipleSubnets) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); int cnt = 0; // Number of reconfigurations @@ -832,7 +841,8 @@ TEST_F(Dhcp4ParserTest, multipleSubnetsExplicitIDs) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); int cnt = 0; // Number of reconfigurations do { @@ -886,7 +896,8 @@ TEST_F(Dhcp4ParserTest, multipleSubnetsOverlapingIDs) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 1); @@ -969,7 +980,8 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { // CASE 1: Configure 4 subnets, then reconfigure and remove the // last one. - ElementPtr json = Element::fromJSON(config4); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config4)); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 0); @@ -981,7 +993,7 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { CfgMgr::instance().clear(); // Do the reconfiguration (the last subnet is removed) - json = Element::fromJSON(config_first3); + ASSERT_NO_THROW(json = parseDHCP4(config_first3)); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 0); @@ -1000,14 +1012,14 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { /// from in between (not first, not last) /// @todo: Uncomment subnet removal test as part of #3281. - json = Element::fromJSON(config4); + ASSERT_NO_THROW(json = parseDHCP4(config4)); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 0); CfgMgr::instance().clear(); // Do reconfiguration - json = Element::fromJSON(config_second_removed); + ASSERT_NO_THROW(json = parseDHCP4(config_second_removed)); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 0); @@ -1028,8 +1040,6 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { // consideration. TEST_F(Dhcp4ParserTest, nextServerGlobal) { - ConstElementPtr status; - string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1039,8 +1049,10 @@ TEST_F(Dhcp4ParserTest, nextServerGlobal) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // check if returned status is OK @@ -1058,8 +1070,6 @@ TEST_F(Dhcp4ParserTest, nextServerGlobal) { // consideration. TEST_F(Dhcp4ParserTest, nextServerSubnet) { - ConstElementPtr status; - string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1069,8 +1079,10 @@ TEST_F(Dhcp4ParserTest, nextServerSubnet) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // check if returned status is OK @@ -1089,8 +1101,6 @@ TEST_F(Dhcp4ParserTest, nextServerSubnet) { TEST_F(Dhcp4ParserTest, nextServerNegative) { IfaceMgrTestConfig test_config(true); - ConstElementPtr status; - // Config with junk instead of next-server address string config_bogus1 = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " @@ -1127,11 +1137,15 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json1 = Element::fromJSON(config_bogus1); - ElementPtr json2 = Element::fromJSON(config_bogus2); - ElementPtr json3 = Element::fromJSON(config_bogus3); + ConstElementPtr json1; + ASSERT_NO_THROW(json1 = parseDHCP4(config_bogus1)); + ConstElementPtr json2; + ASSERT_NO_THROW(json2 = parseDHCP4(config_bogus2)); + ConstElementPtr json3; + ASSERT_NO_THROW(json3 = parseDHCP4(config_bogus3)); // check if returned status is always a failure + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json1)); checkResult(status, 1); EXPECT_TRUE(errorContainsPosition(status, "")); @@ -1153,8 +1167,6 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) { // specific value. TEST_F(Dhcp4ParserTest, nextServerOverride) { - ConstElementPtr status; - string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1165,8 +1177,10 @@ TEST_F(Dhcp4ParserTest, nextServerOverride) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // check if returned status is OK @@ -1670,7 +1684,7 @@ TEST_F(Dhcp4ParserTest, optionDefArray) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": True," + " \"array\": true," " \"space\": \"isc\"" " } ]" "}"; @@ -1851,7 +1865,7 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulatedSpaceAndArray) { " \"name\": \"foo\"," " \"code\": 100," " \"type\": \"uint32\"," - " \"array\": True," + " \"array\": true," " \"space\": \"isc\"," " \"encapsulate\": \"valid-space-name\"" " } ]" @@ -1994,12 +2008,12 @@ TEST_F(Dhcp4ParserTest, optionDataDefaultsGlobal) { "\"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"ABCDEF0105\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"default-ip-ttl\"," " \"data\": \"01\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," @@ -2066,12 +2080,12 @@ TEST_F(Dhcp4ParserTest, optionDataDefaultsSubnet) { " \"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"ABCDEF0105\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"default-ip-ttl\"," " \"data\": \"01\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]" " } ]," "\"valid-lifetime\": 4000 }"; @@ -2137,7 +2151,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { "\"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"ABCDEF0105\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"foo\"," @@ -2330,7 +2344,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { "\"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"AB\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," @@ -2338,12 +2352,12 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { " \"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"ABCDEF0105\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"default-ip-ttl\"," " \"data\": \"01\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]" " } ]," "\"valid-lifetime\": 4000 }"; @@ -2481,7 +2495,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { " \"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"0102030405060708090A\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]" " }," " {" @@ -2490,7 +2504,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { " \"option-data\": [ {" " \"name\": \"default-ip-ttl\"," " \"data\": \"FF\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]" " } ]," "\"valid-lifetime\": 4000 }"; @@ -2638,7 +2652,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) { params["code"] = "41"; // Specify option values in a CSV (user friendly) format. params["data"] = "192.0.2.10, 192.0.2.1, 192.0.2.3"; - params["csv-format"] = "True"; + params["csv-format"] = "true"; std::string config = createConfigWithOption(params); ElementPtr json = Element::fromJSON(config); @@ -2794,7 +2808,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { "\"renew-timer\": 1000," "\"option-data\": [ {" " \"name\": \"vendor-encapsulated-options\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"foo\"," @@ -2806,7 +2820,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { " \"space\": \"vendor-encapsulated-options-space\"," " \"code\": 2," " \"data\": \"192.168.2.1\"," - " \"csv-format\": True" + " \"csv-format\": true" " } ]," "\"option-def\": [ {" " \"name\": \"foo\"," @@ -2890,14 +2904,14 @@ TEST_F(Dhcp4ParserTest, vendorOptionsHex) { " \"space\": \"vendor-4491\"," // VENDOR_ID_CABLE_LABS = 4491 " \"code\": 100," // just a random code " \"data\": \"ABCDEF0105\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"option-two\"," " \"space\": \"vendor-1234\"," " \"code\": 100," " \"data\": \"1234\"," - " \"csv-format\": False" + " \"csv-format\": false" " } ]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1-192.0.2.10\" } ]," @@ -3016,7 +3030,7 @@ buildHooksLibrariesConfig(const std::vector& libraries) { "\"option-data\": [ {" " \"name\": \"dhcp-message\"," " \"data\": \"ABCDEF0105\"," - " \"csv-format\": False" + " \"csv-format\": false" " }," " {" " \"name\": \"foo\"," diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 77bcb09035..b8c409a2bd 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -117,7 +118,8 @@ public: ASSERT_NO_THROW(server_.reset(new NakedControlledDhcpv4Srv())); - ConstElementPtr config = Element::fromJSON(config_txt); + ConstElementPtr config; + ASSERT_NO_THROW(config = parseDHCP4(config_txt)); ConstElementPtr answer = server_->processConfig(config); ASSERT_TRUE(answer); diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 0f7033c011..7bc6ff97e3 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -1067,7 +1067,7 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsis) { " \"space\": \"vendor-4491\"," " \"code\": 2," " \"data\": \"10.253.175.16\"," - " \"csv-format\": True" + " \"csv-format\": true" " }]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"10.254.226.0/25\" } ]," @@ -1079,7 +1079,8 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsis) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1221,7 +1222,8 @@ TEST_F(Dhcpv4SrvTest, nextServerOverride) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config, true)); EXPECT_NO_THROW(status = configureDhcp4Server(srv, json)); @@ -1269,7 +1271,8 @@ TEST_F(Dhcpv4SrvTest, nextServerGlobal) { " \"subnet\": \"192.0.2.0/24\" } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config, true)); EXPECT_NO_THROW(status = configureDhcp4Server(srv, json)); @@ -1358,7 +1361,7 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsORO) { " \"space\": \"vendor-4491\"," " \"code\": 2," " \"data\": \"192.0.2.1, 192.0.2.2\"," - " \"csv-format\": True" + " \"csv-format\": true" " }]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.0/25\" } ]," @@ -1370,7 +1373,8 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsORO) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); EXPECT_NO_THROW(x = configureDhcp4Server(srv, json)); ASSERT_TRUE(x); @@ -1448,7 +1452,7 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsisDefinitions) { " \"code\": "; string config_postfix = "," " \"data\": \"192.0.2.1\"," - " \"csv-format\": True" + " \"csv-format\": true" " }]," "\"subnet4\": [ { " " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.50\" } ]," @@ -1468,8 +1472,10 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsisDefinitions) { // definition, the config should fail. string config_bogus = config_prefix + "99" + config_postfix; - ElementPtr json_bogus = Element::fromJSON(config_bogus); - ElementPtr json_valid = Element::fromJSON(config_valid); + ConstElementPtr json_bogus; + ASSERT_NO_THROW(json_bogus = parseDHCP4(config_bogus)); + ConstElementPtr json_valid; + ASSERT_NO_THROW(json_valid = parseDHCP4(config_valid)); NakedDhcpv4Srv srv(0); @@ -1543,7 +1549,8 @@ TEST_F(Dhcpv4SrvTest, matchClassification) { " \"data\": \"true\" } ], " " \"test\": \"option[12].text == 'foo'\" } ] }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1629,7 +1636,8 @@ TEST_F(Dhcpv4SrvTest, matchClassificationOptionName) { "{ \"name\": \"router\", " " \"test\": \"option[host-name].text == 'foo'\" } ] }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1679,7 +1687,8 @@ TEST_F(Dhcpv4SrvTest, matchClassificationOptionDef) { " \"code\": 250, " " \"type\": \"string\" } ] }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1734,7 +1743,8 @@ TEST_F(Dhcpv4SrvTest, subnetClassPriority) { " \"data\": \"true\" } ], " " \"test\": \"option[12].text == 'foo'\" } ] }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1806,7 +1816,8 @@ TEST_F(Dhcpv4SrvTest, subnetGlobalPriority) { " { \"name\": \"ip-forwarding\", " " \"data\": \"true\" } ] }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1877,7 +1888,8 @@ TEST_F(Dhcpv4SrvTest, classGlobalPriority) { " \"data\": \"true\" } ], " " \"test\": \"option[12].text == 'foo'\" } ] }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc index 7800554ce1..ba8349c8e8 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc @@ -599,7 +599,8 @@ Dhcpv4SrvTest::configure(const std::string& config, const bool commit) { void Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv, const bool commit) { - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + ASSERT_NO_THROW(json = parseJSON(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index b190c55dd0..0133fb886b 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -478,6 +479,62 @@ public: NakedDhcpv4Srv srv_; }; +/// @brief Runs parser in JSON mode, useful for parser testing +/// +/// @param in string to be parsed +/// @return ElementPtr structure representing parsed JSON +inline isc::data::ConstElementPtr +parseJSON(const std::string& in) +{ + isc::dhcp::Parser4Context ctx; + return (ctx.parseString(in, isc::dhcp::Parser4Context::PARSER_JSON)); +} + +/// @brief Runs parser in Dhcp4 mode +/// +/// This is a simplified Dhcp4 mode, so no outer { } and "Dhcp4" is +/// needed. This format is used by most of the tests. +/// +/// @param in string to be parsed +/// @param verbose display the exception message when it fails +/// @return ElementPtr structure representing parsed JSON +inline isc::data::ConstElementPtr +parseDHCP4(const std::string& in, bool verbose = false) +{ + try { + isc::dhcp::Parser4Context ctx; + return (ctx.parseString(in, isc::dhcp::Parser4Context::SUBPARSER_DHCP4)); + } + catch (const std::exception& ex) { + if (verbose) { + std::cout << "EXCEPTION: " << ex.what() << std::endl; + } + throw; + } +} + +/// @brief Runs parser in option definition mode +/// +/// This function parses specified text as JSON that defines option definitions. +/// +/// @param in string to be parsed +/// @param verbose display the exception message when it fails +/// @return ElementPtr structure representing parsed JSON +inline isc::data::ConstElementPtr +parseOPTION_DEF(const std::string& in, bool verbose = false) +{ + try { + isc::dhcp::Parser4Context ctx; + return (ctx.parseString(in, isc::dhcp::Parser4Context::PARSER_OPTION_DEF)); + } + catch (const std::exception& ex) { + if (verbose) { + std::cout << "EXCEPTION: " << ex.what() << std::endl; + } + throw; + } +} + }; // end of isc::dhcp::test namespace }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index 9970cbb1c0..b807b43ea7 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -1161,7 +1161,8 @@ TEST_F(HooksDhcpv4SrvTest, subnet4SelectSimple) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + EXPECT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted @@ -1239,7 +1240,8 @@ TEST_F(HooksDhcpv4SrvTest, subnet4SelectChange) { " } ]," "\"valid-lifetime\": 4000 }"; - ElementPtr json = Element::fromJSON(config); + ConstElementPtr json; + EXPECT_NO_THROW(json = parseDHCP4(config)); ConstElementPtr status; // Configure the server and make sure the config is accepted diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index c42a022c88..8cd9a617e0 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,7 @@ public: LeaseMgrFactory::destroy(); isc::log::setDefaultLoggingOutput(); static_cast(remove(TEST_FILE)); + static_cast(remove(TEST_INCLUDE)); }; /// @brief writes specified content to a well known file @@ -71,11 +73,12 @@ public: /// Writes specified content to TEST_FILE. Tests will /// attempt to read that file. /// + /// @param file_name name of file to be written /// @param content content to be written to file - void writeFile(const std::string& content) { - static_cast(remove(TEST_FILE)); + void writeFile(const std::string& file_name, const std::string& content) { + static_cast(remove(file_name.c_str())); - ofstream out(TEST_FILE, ios::trunc); + ofstream out(file_name.c_str(), ios::trunc); EXPECT_TRUE(out.is_open()); out << content; out.close(); @@ -98,9 +101,11 @@ public: /// Name of a config file used during tests static const char* TEST_FILE; + static const char* TEST_INCLUDE; }; const char* JSONFileBackendTest::TEST_FILE = "test-config.json"; +const char* JSONFileBackendTest::TEST_INCLUDE = "test-include.json"; // This test checks if configuration can be read from a JSON file. TEST_F(JSONFileBackendTest, jsonFile) { @@ -128,7 +133,7 @@ TEST_F(JSONFileBackendTest, jsonFile) { "\"valid-lifetime\": 4000 }" "}"; - writeFile(config); + writeFile(TEST_FILE, config); // Now initialize the server boost::scoped_ptr srv; @@ -179,8 +184,9 @@ TEST_F(JSONFileBackendTest, jsonFile) { EXPECT_EQ(Lease::TYPE_V4, pools3.at(0)->getType()); } -// This test checks if configuration can be read from a JSON file. -TEST_F(JSONFileBackendTest, comments) { +// This test checks if configuration can be read from a JSON file +// using hash (#) line comments +TEST_F(JSONFileBackendTest, hashComments) { string config_hash_comments = "# This is a comment. It should be \n" "#ignored. Real config starts in line below\n" @@ -198,10 +204,56 @@ TEST_F(JSONFileBackendTest, comments) { "\"valid-lifetime\": 4000 }" "}"; - /// @todo: Implement C++-style (// ...) comments - /// @todo: Implement C-style (/* ... */) comments + writeFile(TEST_FILE, config_hash_comments); + + // Now initialize the server + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv4Srv(0)) + ); + + // And configure it using config with comments. + EXPECT_NO_THROW(srv->init(TEST_FILE)); + + // Now check if the configuration has been applied correctly. + const Subnet4Collection* subnets = + CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); + ASSERT_TRUE(subnets); + ASSERT_EQ(1, subnets->size()); + + // Check subnet 1. + EXPECT_EQ("192.0.2.0", subnets->at(0)->get().first.toText()); + EXPECT_EQ(22, subnets->at(0)->get().second); + + // Check pools in the first subnet. + const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_V4); + ASSERT_EQ(1, pools1.size()); + EXPECT_EQ("192.0.2.0", pools1.at(0)->getFirstAddress().toText()); + EXPECT_EQ("192.0.2.255", pools1.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_V4, pools1.at(0)->getType()); +} - writeFile(config_hash_comments); +// This test checks if configuration can be read from a JSON file +// using C++ line (//) comments. +TEST_F(JSONFileBackendTest, cppLineComments) { + + string config_cpp_line_comments = "// This is a comment. It should be \n" + "//ignored. Real config starts in line below\n" + "{ \"Dhcp4\": {" + "\"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "// comments in the middle should be ignored, too\n" + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ]," + " \"subnet\": \"192.0.2.0/22\" " + " } ]," + "\"valid-lifetime\": 4000 }" + "}"; + + writeFile(TEST_FILE, config_cpp_line_comments); // Now initialize the server boost::scoped_ptr srv; @@ -230,6 +282,142 @@ TEST_F(JSONFileBackendTest, comments) { EXPECT_EQ(Lease::TYPE_V4, pools1.at(0)->getType()); } +// This test checks if configuration can be read from a JSON file +// using C block (/* */) comments +TEST_F(JSONFileBackendTest, cBlockComments) { + + string config_c_block_comments = "/* This is a comment. It should be \n" + "ignored. Real config starts in line below*/\n" + "{ \"Dhcp4\": {" + "\"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "/* comments in the middle should be ignored, too*/\n" + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ]," + " \"subnet\": \"192.0.2.0/22\" " + " } ]," + "\"valid-lifetime\": 4000 }" + "}"; + + writeFile(TEST_FILE, config_c_block_comments); + + // Now initialize the server + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv4Srv(0)) + ); + + // And configure it using config with comments. + EXPECT_NO_THROW(srv->init(TEST_FILE)); + + // Now check if the configuration has been applied correctly. + const Subnet4Collection* subnets = + CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); + ASSERT_TRUE(subnets); + ASSERT_EQ(1, subnets->size()); + + // Check subnet 1. + EXPECT_EQ("192.0.2.0", subnets->at(0)->get().first.toText()); + EXPECT_EQ(22, subnets->at(0)->get().second); + + // Check pools in the first subnet. + const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_V4); + ASSERT_EQ(1, pools1.size()); + EXPECT_EQ("192.0.2.0", pools1.at(0)->getFirstAddress().toText()); + EXPECT_EQ("192.0.2.255", pools1.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_V4, pools1.at(0)->getType()); +} + +// This test checks if configuration can be read from a JSON file +// using an include file. +TEST_F(JSONFileBackendTest, include) { + + string config_hash_comments = "{ \"Dhcp4\": {" + "\"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "," + "\"valid-lifetime\": 4000 }" + "}"; + string include = "\n" + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ]," + " \"subnet\": \"192.0.2.0/22\" " + " } ]\n"; + + writeFile(TEST_FILE, config_hash_comments); + writeFile(TEST_INCLUDE, include); + + // Now initialize the server + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv4Srv(0)) + ); + + // And configure it using config with comments. + EXPECT_NO_THROW(srv->init(TEST_FILE)); + + // Now check if the configuration has been applied correctly. + const Subnet4Collection* subnets = + CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); + ASSERT_TRUE(subnets); + ASSERT_EQ(1, subnets->size()); + + // Check subnet 1. + EXPECT_EQ("192.0.2.0", subnets->at(0)->get().first.toText()); + EXPECT_EQ(22, subnets->at(0)->get().second); + + // Check pools in the first subnet. + const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_V4); + ASSERT_EQ(1, pools1.size()); + EXPECT_EQ("192.0.2.0", pools1.at(0)->getFirstAddress().toText()); + EXPECT_EQ("192.0.2.255", pools1.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_V4, pools1.at(0)->getType()); +} + +// This test checks if recursive include of a file is detected +TEST_F(JSONFileBackendTest, recursiveInclude) { + + string config_recursive_include = "{ \"Dhcp4\": {" + "\"interfaces-config\": {" + " \"interfaces\": [ ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ]," + " \"subnet\": \"192.0.2.0/22\" " + " } ]," + "\"valid-lifetime\": 4000 }" + "}"; + string include = "\"eth\", "; + string msg = "configuration error using file '" + string(TEST_FILE) + + "': Too many nested include."; + + writeFile(TEST_FILE, config_recursive_include); + writeFile(TEST_INCLUDE, include); + + // Now initialize the server + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv4Srv(0)) + ); + + // And configure it using config with comments. + try { + srv->init(TEST_FILE); + FAIL() << "Expected Dhcp4ParseError but nothing was raised"; + } + catch (const Exception& ex) { + EXPECT_EQ(msg, ex.what()); + } +} + // This test checks if configuration detects failure when trying: // - empty file // - empty filename @@ -263,15 +451,15 @@ TEST_F(JSONFileBackendTest, configBroken) { EXPECT_THROW(srv->init(""), BadValue); // Try to configure it using empty file. Should fail. - writeFile(config_empty); + writeFile(TEST_FILE, config_empty); EXPECT_THROW(srv->init(TEST_FILE), BadValue); // Now try to load a config that does not have Dhcp4 component. - writeFile(config_v4); + writeFile(TEST_FILE, config_v4); EXPECT_THROW(srv->init(TEST_FILE), BadValue); // Now try to load a config with Dhcp4 full of nonsense. - writeFile(config_nonsense); + writeFile(TEST_FILE, config_nonsense); EXPECT_THROW(srv->init(TEST_FILE), BadValue); } @@ -347,7 +535,7 @@ TEST_F(JSONFileBackendTest, timers) { "\"subnet4\": [ ]," "\"valid-lifetime\": 4000 }" "}"; - writeFile(config); + writeFile(TEST_FILE, config); // Create an instance of the server and intialize it. boost::scoped_ptr srv; @@ -416,7 +604,7 @@ TEST_F(JSONFileBackendTest, defaultLeaseDbBackend) { "\"subnet4\": [ ]," "\"valid-lifetime\": 4000 }" "}"; - writeFile(config); + writeFile(TEST_FILE, config); // Create an instance of the server and intialize it. boost::scoped_ptr srv; @@ -518,7 +706,7 @@ void JSONFileBackendMySQLTest:: testBackendReconfiguration(const std::string& backend_first, const std::string& backend_second) { - writeFile(createConfiguration(backend_first)); + writeFile(TEST_FILE, createConfiguration(backend_first)); // Create an instance of the server and intialize it. boost::scoped_ptr srv; @@ -533,7 +721,7 @@ testBackendReconfiguration(const std::string& backend_first, LeaseMgrFactory::instance().getType()); // New configuration modifies the lease database backend type. - writeFile(createConfiguration(backend_second)); + writeFile(TEST_FILE, createConfiguration(backend_second)); // Explicitly calling signal handler for SIGHUP to trigger server // reconfiguration.