[master] Merge branch 'trac5297'

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -1718,6 +1718,64 @@ TEST_F(Dhcp4ParserTest, badPools) {
     EXPECT_TRUE(errorContainsPosition(status, "<string>"));
 }
 
+// Goal of this test is to verify no pool definitions is invalid
+// and returns a location in the error message.
+TEST_F(Dhcp4ParserTest, noPools) {
+
+    // Configuration string.
+    string config = "{ " + genIfaceConfig() + "," +
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"user-context\": { } } ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    EXPECT_THROW(parseDHCP4(config, true), Dhcp4ParseError);
+}
+
+// Goal of this test is to verify that invalid subnet fails to be parsed.
+TEST_F(Dhcp4ParserTest, badSubnet) {
+
+    // Configuration string.
+    string config = "{ " + genIfaceConfig() + "," +
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ ],"
+        "    \"subnet\": \"192.0.2.0\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ConstElementPtr json;
+    ASSERT_NO_THROW(json = parseDHCP4(config, true));
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    checkResult(status, 1);
+    EXPECT_TRUE(errorContainsPosition(status, "<string>"));
+}
+
+// Goal of this test is to verify that unknown interface fails
+// to be parsed.
+TEST_F(Dhcp4ParserTest, unknownInterface) {
+
+    // Configuration string.
+    string config = "{ " + genIfaceConfig() + "," +
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ ],"
+        "    \"subnet\": \"192.0.2.0/24\","
+        "    \"interface\": \"ethX\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ConstElementPtr json;
+    ASSERT_NO_THROW(json = parseDHCP4(config, true));
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    checkResult(status, 1);
+    EXPECT_TRUE(errorContainsPosition(status, "<string>"));
+}
+
 // The goal of this test is to check whether an option definition
 // that defines an option carrying an IPv4 address can be created.
 TEST_F(Dhcp4ParserTest, optionDefIpv4Address) {

src/bin/dhcp4/tests/d2_unittest.cc

@@ -386,6 +386,125 @@ TEST_F(Dhcp4SrvD2Test, queueMaxError) {
     ASSERT_FALSE(mgr.amSending());
 }
 
+// Tests invalid config with TCP protocol
+TEST_F(Dhcp4SrvD2Test, badTCP) {
+    std::string config =
+        "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        " \"dhcp-ddns\" : {"
+        "     \"enable-updates\" : true, "
+        "     \"server-ip\" : \"127.0.0.1\", "
+        "     \"server-port\" : 53001, "
+        "     \"sender-ip\" : \"0.0.0.0\", "
+        "     \"sender-port\" : 0, "
+        "     \"max-queue-size\" : 1024, "
+        "     \"ncr-protocol\" : \"TCP\", "
+        "     \"ncr-format\" : \"JSON\", "
+        "     \"always-include-fqdn\" : true, "
+        "     \"override-no-update\" : true, "
+        "     \"override-client-update\" : true, "
+        "     \"replace-client-name\" : \"when-present\", "
+        "     \"generated-prefix\" : \"test.prefix\", "
+        "     \"qualifying-suffix\" : \"test.suffix.\" },"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ConstElementPtr status;
+
+    CfgMgr::instance().clear();
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json));
+    ASSERT_TRUE(status);
+    int rcode;
+    ConstElementPtr comment = config::parseAnswer(rcode, status);
+    EXPECT_EQ(1, rcode);
+}
+
+// Tests invalid config with bad sender family
+TEST_F(Dhcp4SrvD2Test, badFamily) {
+    std::string config =
+        "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        " \"dhcp-ddns\" : {"
+        "     \"enable-updates\" : true, "
+        "     \"server-ip\" : \"127.0.0.1\", "
+        "     \"server-port\" : 53001, "
+        "     \"sender-ip\" : \"::\", "
+        "     \"sender-port\" : 0, "
+        "     \"max-queue-size\" : 1024, "
+        "     \"ncr-protocol\" : \"UDP\", "
+        "     \"ncr-format\" : \"JSON\", "
+        "     \"always-include-fqdn\" : true, "
+        "     \"override-no-update\" : true, "
+        "     \"override-client-update\" : true, "
+        "     \"replace-client-name\" : \"when-present\", "
+        "     \"generated-prefix\" : \"test.prefix\", "
+        "     \"qualifying-suffix\" : \"test.suffix.\" },"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ConstElementPtr status;
+
+    CfgMgr::instance().clear();
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json));
+    ASSERT_TRUE(status);
+    int rcode;
+    ConstElementPtr comment = config::parseAnswer(rcode, status);
+    EXPECT_EQ(1, rcode);
+}
+
+// Tests invalid config with server == sender
+TEST_F(Dhcp4SrvD2Test, senderEqServer) {
+    std::string config =
+        "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        " \"dhcp-ddns\" : {"
+        "     \"enable-updates\" : true, "
+        "     \"server-ip\" : \"127.0.0.1\", "
+        "     \"server-port\" : 53001, "
+        "     \"sender-ip\" : \"127.0.0.1\", "
+        "     \"sender-port\" : 53001, "
+        "     \"max-queue-size\" : 1024, "
+        "     \"ncr-protocol\" : \"UDP\", "
+        "     \"ncr-format\" : \"JSON\", "
+        "     \"always-include-fqdn\" : true, "
+        "     \"override-no-update\" : true, "
+        "     \"override-client-update\" : true, "
+        "     \"replace-client-name\" : \"when-present\", "
+        "     \"generated-prefix\" : \"test.prefix\", "
+        "     \"qualifying-suffix\" : \"test.suffix.\" },"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ConstElementPtr status;
+
+    CfgMgr::instance().clear();
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json));
+    ASSERT_TRUE(status);
+    int rcode;
+    ConstElementPtr comment = config::parseAnswer(rcode, status);
+    EXPECT_EQ(1, rcode);
+}
 
 } // namespace test
 } // namespace dhcp

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -112,6 +112,7 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
             try {
                 defs->add(def.first, def.second);
             } catch (const std::exception& ex) {
+                // Sanity check: it should never happen
                 isc_throw(DhcpConfigError, ex.what() << " ("
                           << option_def->getPosition() << ")");
             }

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -72,10 +72,12 @@ MACSourcesListConfigParser::parse(CfgMACSource& mac_sources, ConstElementPtr val
 // ******************** ControlSocketParser *************************
 void ControlSocketParser::parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value) {
     if (!value) {
+        // Sanity check: not supposed to fail.
         isc_throw(DhcpConfigError, "Logic error: specified control-socket is null");
     }
 
     if (value->getType() != Element::map) {
+        // Sanity check: not supposed to fail.
         isc_throw(DhcpConfigError, "Specified control-socket is expected to be a map"
                   ", i.e. a structure defined within { }");
     }
@@ -201,6 +203,7 @@ OptionDefParser::parse(ConstElementPtr option_def) {
 void
 OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_list) {
     if (!option_def_list) {
+        // Sanity check: not supposed to fail.
         isc_throw(DhcpConfigError, "parser error: a pointer to a list of"
                   << " option definitions is NULL ("
                   << option_def_list->getPosition() << ")");
@@ -355,6 +358,7 @@ PoolParser::parse(PoolStoragePtr pools,
     // If there's user-context specified, store it.
     ConstElementPtr user_context = pool_structure->get("user-context");
     if (user_context) {
+        // The grammar accepts only maps but still check it.
         if (user_context->getType() != Element::map) {
             isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map ("
                       << user_context->getPosition() << ")");
@@ -445,6 +449,7 @@ SubnetConfigParser::hrModeFromText(const std::string& txt) {
     } else if (txt.compare("all") == 0) {
         return (Network::HR_ALL);
     } else {
+        // Should never happen...
         isc_throw(BadValue, "Can't convert '" << txt
                   << "' into any valid reservation-mode values");
     }
@@ -502,6 +507,7 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     // If there's user-context specified, store it.
     ConstElementPtr user_context = params->get("user-context");
     if (user_context) {
+        // The grammar accepts only maps but still check it.
         if (user_context->getType() != Element::map) {
             isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map ("
                       << user_context->getPosition() << ")");
@@ -529,6 +535,7 @@ Subnet4ConfigParser::parse(ConstElementPtr subnet) {
     SubnetPtr generic = SubnetConfigParser::parse(subnet);
 
     if (!generic) {
+        // Sanity check: not supposed to fail.
         isc_throw(DhcpConfigError,
                   "Failed to create an IPv4 subnet (" <<
                   subnet->getPosition() << ")");
@@ -901,6 +908,7 @@ Subnet6ConfigParser::parse(ConstElementPtr subnet) {
     SubnetPtr generic = SubnetConfigParser::parse(subnet);
 
     if (!generic) {
+        // Sanity check: not supposed to fail.
         isc_throw(DhcpConfigError,
                   "Failed to create an IPv6 subnet (" <<
                   subnet->getPosition() << ")");
@@ -933,6 +941,7 @@ Subnet6ConfigParser::parse(ConstElementPtr subnet) {
     return (sn6ptr);
 }
 
+// Unused?
 void
 Subnet6ConfigParser::duplicate_option_warning(uint32_t code,
                                               asiolink::IOAddress& addr) {

src/lib/dhcpsrv/parsers/duid_config_parser.cc

@@ -27,6 +27,7 @@ void
 DUIDConfigParser::parse(const CfgDUIDPtr& cfg,
                         isc::data::ConstElementPtr duid_configuration) {
     if (!cfg) {
+        // Sanity check
         isc_throw(DhcpConfigError, "Must provide valid pointer to cfg when parsing duid");
     }
 

src/lib/dhcpsrv/parsers/option_data_parser.cc

@@ -36,6 +36,7 @@ OptionDataParser::parse(isc::data::ConstElementPtr single_option) {
     std::pair<OptionDescriptor, std::string> opt = createOption(single_option);
 
     if (!opt.first.option_) {
+        // Should never happen (@todo: update message)
         isc_throw(isc::InvalidOperation,
             "parser logic error: no option has been configured and"
             " thus there is nothing to commit. Has build() been called?");

src/lib/dhcpsrv/parsers/shared_network_parser.cc

@@ -70,6 +70,9 @@ SharedNetwork4Parser::parse(const data::ConstElementPtr& shared_network_data) {
             }
         }
 
+    } catch (const DhcpConfigError&) {
+	// Position was already added
+	throw;
     } catch (const std::exception& ex) {
         isc_throw(DhcpConfigError, ex.what() << " ("
                   << shared_network_data->getPosition() << ")");

src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc

@@ -768,9 +768,21 @@ TEST_F(ClientClassDefParserTest, nextServerBogus) {
         "        } \n"
         "      ] \n"
         "} \n";
+    std::string bogus_broadcast =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"next-server\": \"255.255.255.255\",\n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
 
     EXPECT_THROW(parseClientClassDef(bogus_v6, AF_INET), DhcpConfigError);
     EXPECT_THROW(parseClientClassDef(bogus_junk, AF_INET), DhcpConfigError);
+    EXPECT_THROW(parseClientClassDef(bogus_broadcast, AF_INET), DhcpConfigError);
 }
 
 // Test verifies that it is possible to define server-hostname field and it

src/lib/dhcpsrv/tests/d2_client_unittest.cc

@@ -370,6 +370,42 @@ TEST(D2ClientMgr, validConfig) {
     EXPECT_NE(*original_config, *updated_config);
 }
 
+/// @brief Checks passing the D2ClientMgr a valid D2 client configuration
+/// using IPv6 service.
+TEST(D2ClientMgr, ipv6Config) {
+    D2ClientMgrPtr d2_client_mgr;
+
+    // Construct the manager and fetch its initial configuration.
+    ASSERT_NO_THROW(d2_client_mgr.reset(new D2ClientMgr()));
+    D2ClientConfigPtr original_config = d2_client_mgr->getD2ClientConfig();
+    ASSERT_TRUE(original_config);
+
+    // Create a new, enabled config.
+    D2ClientConfigPtr new_cfg;
+    ASSERT_NO_THROW(new_cfg.reset(new D2ClientConfig(true,
+                                  isc::asiolink::IOAddress("::1"), 477,
+                                  isc::asiolink::IOAddress("::1"), 478,
+                                  1024,
+                                  dhcp_ddns::NCR_UDP, dhcp_ddns::FMT_JSON,
+                                  true, true, true, D2ClientConfig::RCM_WHEN_PRESENT,
+                                  "pre-fix", "suf-fix")));
+
+    // Verify that we can assign a new, non-empty configuration.
+    ASSERT_NO_THROW(d2_client_mgr->setD2ClientConfig(new_cfg));
+
+    // Verify that we can fetch the newly assigned configuration.
+    D2ClientConfigPtr updated_config = d2_client_mgr->getD2ClientConfig();
+    ASSERT_TRUE(updated_config);
+    EXPECT_TRUE(updated_config->getEnableUpdates());
+
+    // Make sure convenience method agrees with the updated configuration.
+    EXPECT_TRUE(d2_client_mgr->ddnsEnabled());
+
+    // Make sure the configuration we fetched is the one we assigned,
+    // and not the original configuration.
+    EXPECT_EQ(*new_cfg, *updated_config);
+    EXPECT_NE(*original_config, *updated_config);
+}
 
 /// @brief Tests that analyzeFqdn detects invalid combination of both the
 /// client S and N flags set to true.

src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc

@@ -248,6 +248,20 @@ TEST_F(DbAccessParserTest, validTypeMemfile) {
     checkAccessString("Valid memfile", parser.getDbAccessParameters(), config);
 }
 
+// Check that the parser works with a simple configuration for host database.
+TEST_F(DbAccessParserTest, hosts) {
+    const char* config[] = {"type", "memfile",
+                            NULL};
+
+    string json_config = toJson(config);
+    ConstElementPtr json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
+    TestDbAccessParser parser(DbAccessParser::HOSTS_DB);
+    EXPECT_NO_THROW(parser.parse(json_elements));
+    checkAccessString("Valid memfile", parser.getDbAccessParameters(), config);
+}
+
 // Check that the parser works with a simple configuration that
 // includes empty elements.
 TEST_F(DbAccessParserTest, emptyKeyword) {

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -677,6 +677,24 @@ TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) {
     cfg.runCfgOptionsTest(family_, config);
 }
 
+/// @brief Check parsing of option definitions using invalid space fails.
+TEST_F(ParseConfigTest, badSpaceOptionDefTest) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100000,"
+        "      \"type\": \"ipv6-address\","
+        "      \"space\": \"-1\""
+        "  } ]"
+        "}";
+
+    // Verify that the configuration string does not parse.
+    int rcode = parseConfiguration(config, true);
+    ASSERT_NE(0, rcode);
+}
+
 /// @brief Check basic parsing of options.
 ///
 /// Note that this tests basic operation of the OptionDataListParser and
@@ -761,6 +779,40 @@ TEST_F(ParseConfigTest, minimalOptionDataTest) {
     cfg.runCfgOptionsTest(family_, expected);
 }
 
+/// @brief Check parsing of unknown options fails.
+TEST_F(ParseConfigTest, unknownOptionDataTest) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"foo\","
+        "    \"data\": \"01\","
+        "    \"space\": \"bar\""
+        " } ]"
+        "}";
+
+    // Verify that the configuration string does not parse.
+    int rcode = parseConfiguration(config, true);
+    ASSERT_NE(0, rcode);
+}
+
+/// @brief Check parsing of options with invalid space fails.
+TEST_F(ParseConfigTest, badSpaceOptionDataTest) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"code\": 100,"
+        "    \"data\": \"01\","
+        "    \"space\": \"-1\""
+        " } ]"
+        "}";
+
+    // Verify that the configuration string does not parse.
+    int rcode = parseConfiguration(config, true);
+    ASSERT_NE(0, rcode);
+}
+
 /// @brief Check parsing of options with escape characters.
 ///
 /// Note that this tests basic operation of the OptionDataListParser and

src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc

@@ -942,6 +942,30 @@ TEST_F(HostReservationParserTest, dhcp6NullPrefix2) {
 }
 
 // This test verifies that the configuration parser throws an exception
+// when slash is missing for the prefix..
+TEST_F(HostReservationParserTest, dhcp6NullPrefix3) {
+    std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
+        "\"prefixes\": [ \"2001:db8:2000:0101::\" ] }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
+// This test verifies that the configuration parser throws an exception
+// when slash is followed by nothing for the prefix..
+TEST_F(HostReservationParserTest, dhcp6NullPrefix4) {
+    std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
+        "\"prefixes\": [ \"2001:db8:2000:0101::/\" ] }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
+// This test verifies that the configuration parser throws an exception
+// when slash is not followed by a number for the prefix..
+TEST_F(HostReservationParserTest, dhcp6NullPrefix5) {
+    std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
+        "\"prefixes\": [ \"2001:db8:2000:0101::/foo\" ] }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
+// This test verifies that the configuration parser throws an exception
 // when the same address is reserved twice.
 TEST_F(HostReservationParserTest, dhcp6DuplicatedAddress) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","