[trac575] Move parseAddresses to libserver_common

It is taken out from the resolver and will be used by both servers.
Added tests, it was tested implicitly before.

doc/Doxyfile

@@ -568,7 +568,7 @@ WARN_LOGFILE           =
 # directories like "/usr/src/myproject". Separate the files or directories
 # with spaces.
 
-INPUT                  = ../src/lib/cc ../src/lib/config ../src/lib/dns ../src/lib/exceptions ../src/lib/datasrc ../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log ../src/lib/asiolink/ ../src/lib/nsas ../src/lib/testutils ../src/lib/cache
+INPUT                  = ../src/lib/cc ../src/lib/config ../src/lib/dns ../src/lib/exceptions ../src/lib/datasrc ../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log ../src/lib/asiolink/ ../src/lib/nsas ../src/lib/testutils ../src/lib/cache ../src/lib/server_common/
 
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is

src/bin/resolver/Makefile.am

@@ -48,6 +48,7 @@ b10_resolver_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o
 b10_resolver_LDFLAGS = -pthread
 

src/bin/resolver/resolver.cc

@@ -39,6 +39,7 @@
 #include <dns/rrttl.h>
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
+#include <server_common/portconfig.h>
 
 #include <log/dummylog.h>
 
@@ -52,8 +53,7 @@ using namespace isc::data;
 using namespace isc::config;
 using isc::log::dlog;
 using namespace asiolink;
-
-typedef pair<string, uint16_t> addr_t;
+using namespace isc::server_common::portconfig;
 
 class ResolverImpl {
 private:
@@ -96,14 +96,14 @@ public:
         }
     }
 
-    void setForwardAddresses(const vector<addr_t>& upstream,
+    void setForwardAddresses(const vector<AddressPair>& upstream,
         DNSService *dnss)
     {
         upstream_ = upstream;
         if (dnss) {
             if (!upstream_.empty()) {
                 dlog("Setting forward addresses:");
-                BOOST_FOREACH(const addr_t& address, upstream) {
+                BOOST_FOREACH(const AddressPair& address, upstream) {
                     dlog(" " + address.first + ":" +
                         boost::lexical_cast<string>(address.second));
                 }
@@ -113,14 +113,14 @@ public:
         }
     }
 
-    void setRootAddresses(const vector<addr_t>& upstream_root,
+    void setRootAddresses(const vector<AddressPair>& upstream_root,
                           DNSService *dnss)
     {
         upstream_root_ = upstream_root;
         if (dnss) {
             if (!upstream_root_.empty()) {
                 dlog("Setting root addresses:");
-                BOOST_FOREACH(const addr_t& address, upstream_root) {
+                BOOST_FOREACH(const AddressPair& address, upstream_root) {
                     dlog(" " + address.first + ":" +
                         boost::lexical_cast<string>(address.second));
                 }
@@ -144,11 +144,11 @@ public:
     /// These members are public because Resolver accesses them directly.
     ModuleCCSession* config_session_;
     /// Addresses of the root nameserver(s)
-    vector<addr_t> upstream_root_;
+    vector<AddressPair> upstream_root_;
     /// Addresses of the forward nameserver
-    vector<addr_t> upstream_;
+    vector<AddressPair> upstream_;
     /// Addresses we listen on
-    vector<addr_t> listen_;
+    vector<AddressPair> listen_;
 
     /// Timeout for outgoing queries in milliseconds
     int query_timeout_;
@@ -460,46 +460,6 @@ ResolverImpl::processNormalQuery(const Question& question,
     rec_query_->resolve(question, answer_message, buffer, server);
 }
 
-namespace {
-
-vector<addr_t>
-parseAddresses(ConstElementPtr addresses) {
-    vector<addr_t> result;
-    if (addresses) {
-        if (addresses->getType() == Element::list) {
-            for (size_t i(0); i < addresses->size(); ++ i) {
-                ConstElementPtr addrPair(addresses->get(i));
-                ConstElementPtr addr(addrPair->get("address"));
-                ConstElementPtr port(addrPair->get("port"));
-                if (!addr || ! port) {
-                    isc_throw(BadValue, "Address must contain both the IP"
-                        "address and port");
-                }
-                try {
-                    IOAddress(addr->stringValue());
-                    if (port->intValue() < 0 ||
-                        port->intValue() > 0xffff) {
-                        isc_throw(BadValue, "Bad port value (" <<
-                            port->intValue() << ")");
-                    }
-                    result.push_back(addr_t(addr->stringValue(),
-                        port->intValue()));
-                }
-                catch (const TypeError &e) { // Better error message
-                    isc_throw(TypeError,
-                        "Address must be a string and port an integer");
-                }
-            }
-        } else if (addresses->getType() != Element::null) {
-            isc_throw(TypeError,
-                "root_addresses, forward_addresses, and listen_on config element must be a list");
-        }
-    }
-    return (result);
-}
-
-}
-
 ConstElementPtr
 Resolver::updateConfig(ConstElementPtr config) {
     dlog("New config comes: " + config->toWire());
@@ -507,11 +467,14 @@ Resolver::updateConfig(ConstElementPtr config) {
     try {
         // Parse forward_addresses
         ConstElementPtr rootAddressesE(config->get("root_addresses"));
-        vector<addr_t> rootAddresses(parseAddresses(rootAddressesE));
+        vector<AddressPair> rootAddresses(parseAddresses(rootAddressesE,
+                                                    "root_addresses"));
         ConstElementPtr forwardAddressesE(config->get("forward_addresses"));
-        vector<addr_t> forwardAddresses(parseAddresses(forwardAddressesE));
+        vector<AddressPair> forwardAddresses(parseAddresses(forwardAddressesE,
+                                                       "forward_addresse"));
         ConstElementPtr listenAddressesE(config->get("listen_on"));
-        vector<addr_t> listenAddresses(parseAddresses(listenAddressesE));
+        vector<AddressPair> listenAddresses(parseAddresses(listenAddressesE,
+                                                      "listen_on"));
         bool set_timeouts(false);
         int qtimeout = impl_->query_timeout_;
         int ctimeout = impl_->client_timeout_;
@@ -584,13 +547,13 @@ Resolver::updateConfig(ConstElementPtr config) {
 }
 
 void
-Resolver::setForwardAddresses(const vector<addr_t>& addresses)
+Resolver::setForwardAddresses(const vector<AddressPair>& addresses)
 {
     impl_->setForwardAddresses(addresses, dnss_);
 }
 
 void
-Resolver::setRootAddresses(const vector<addr_t>& addresses)
+Resolver::setRootAddresses(const vector<AddressPair>& addresses)
 {
     impl_->setRootAddresses(addresses, dnss_);
 }
@@ -600,12 +563,12 @@ Resolver::isForwarding() const {
     return (!impl_->upstream_.empty());
 }
 
-vector<addr_t>
+vector<AddressPair>
 Resolver::getForwardAddresses() const {
     return (impl_->upstream_);
 }
 
-vector<addr_t>
+vector<AddressPair>
 Resolver::getRootAddresses() const {
     return (impl_->upstream_root_);
 }
@@ -613,9 +576,9 @@ Resolver::getRootAddresses() const {
 namespace {
 
 void
-setAddresses(DNSService *service, const vector<addr_t>& addresses) {
+setAddresses(DNSService *service, const vector<AddressPair>& addresses) {
     service->clearServers();
-    BOOST_FOREACH(const addr_t &address, addresses) {
+    BOOST_FOREACH(const AddressPair &address, addresses) {
         service->addServer(address.second, address.first);
     }
 }
@@ -623,10 +586,10 @@ setAddresses(DNSService *service, const vector<addr_t>& addresses) {
 }
 
 void
-Resolver::setListenAddresses(const vector<addr_t>& addresses) {
+Resolver::setListenAddresses(const vector<AddressPair>& addresses) {
     try {
         dlog("Setting listen addresses:");
-        BOOST_FOREACH(const addr_t& addr, addresses) {
+        BOOST_FOREACH(const AddressPair& addr, addresses) {
             dlog(" " + addr.first + ":" +
                         boost::lexical_cast<string>(addr.second));
         }
@@ -687,7 +650,7 @@ Resolver::getRetries() const {
     return impl_->retries_;
 }
 
-vector<addr_t>
+vector<AddressPair>
 Resolver::getListenAddresses() const {
     return (impl_->listen_);
 }

src/bin/resolver/tests/Makefile.am

@@ -37,6 +37,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
+run_unittests_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 
 # Note the ordering matters: -Wno-... must follow -Wextra (defined in
 # B10_CXXFLAGS

src/lib/server_common/Makefile.am

@@ -18,5 +18,8 @@ endif
 
 lib_LTLIBRARIES = libserver_common.la
 libserver_common_la_SOURCES = portconfig.h portconfig.cc
+libserver_common_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+libserver_common_la_LIBADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
+libserver_common_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la
 
 CLEANFILES = *.gcno *.gcda

src/lib/server_common/portconfig.cc

@@ -0,0 +1,66 @@
+// Copyright (C) 2011  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 "portconfig.h"
+
+#include <asiolink/io_address.h>
+
+using namespace std;
+using namespace isc::data;
+using namespace asiolink;
+
+namespace isc {
+namespace server_common {
+namespace portconfig {
+
+AddressList
+parseAddresses(isc::data::ConstElementPtr addresses,
+               const std::string& elemName)
+{
+    AddressList result;
+    if (addresses) {
+        if (addresses->getType() == Element::list) {
+            for (size_t i(0); i < addresses->size(); ++ i) {
+                ConstElementPtr addrPair(addresses->get(i));
+                ConstElementPtr addr(addrPair->get("address"));
+                ConstElementPtr port(addrPair->get("port"));
+                if (!addr || ! port) {
+                    isc_throw(BadValue, "Address must contain both the IP"
+                        "address and port");
+                }
+                try {
+                    IOAddress(addr->stringValue());
+                    if (port->intValue() < 0 ||
+                        port->intValue() > 0xffff) {
+                        isc_throw(BadValue, "Bad port value (" <<
+                            port->intValue() << ")");
+                    }
+                    result.push_back(AddressPair(addr->stringValue(),
+                        port->intValue()));
+                }
+                catch (const TypeError &e) { // Better error message
+                    isc_throw(TypeError,
+                        "Address must be a string and port an integer");
+                }
+            }
+        } else if (addresses->getType() != Element::null) {
+            isc_throw(TypeError, elemName + " config element must be a list");
+        }
+    }
+    return (result);
+}
+
+}
+}
+}

src/lib/server_common/portconfig.h

@@ -0,0 +1,86 @@
+// Copyright (C) 2011  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 ISC_SERVER_COMMON_PORTCONFIG_H
+#define ISC_SERVER_COMMON_PORTCONFIG_H
+
+#include <utility>
+#include <string>
+#include <stdint.h>
+#include <vector>
+
+#include <cc/data.h>
+
+namespace isc {
+namespace server_common {
+/**
+ * \brief Utilities to configure ports and addresses.
+ *
+ * Here are some utilities to help a server to parse configuration of addresses
+ * and ports and install the configuration.
+ */
+namespace portconfig {
+
+/**
+ * \brief An address-port pair.
+ *
+ * It is just a pair of string for an address and unsigned integer for port
+ * number. Anything more fancy would be an overkill, it is used only to pass
+ * the addresses and ports around as intermediate results.
+ */
+typedef std::pair<std::string, uint16_t> AddressPair;
+
+/// \brief Bunch of address pairs
+typedef std::vector<AddressPair> AddressList;
+
+/**
+ * \brief
+ *
+ * This parses a list of address-port configurations and returns them. The
+ * configuration looks like this:
+ *
+ * \verbatim
+[
+  {
+    "address": "192.0.2.1",
+    "port": 13
+  },
+  {
+    "address": "::",
+    "port": 80
+  }
+]
+ * \endverbatim
+ * \param addresses The configuration element to parse (the list). Empty list,
+ *     null element and null pointer all mean empty list of addresses.
+ * \param elemName The name of the element, used to create descriptions for
+ *     exceptions.
+ * \return Vector of parsed address-port pairs found in the configuration.
+ * \throw isc::data::TypeError if something in the configuration is of a wrong
+ *     type (string passed to a port, element in the list that isn't hash,
+ *     etc).
+ * \throw asiolink::IOError if the provided address string can't be parsed.
+ * \throw BadValue for other invalid configurations (missing port or address
+ *     element in the hash, port number out of range).
+ * \throw std::bad_alloc when allocation fails.
+ */
+AddressList
+parseAddresses(isc::data::ConstElementPtr addresses,
+               const std::string& elemName);
+
+}
+}
+}
+
+#endif

src/lib/server_common/tests/Makefile.am

@@ -26,6 +26,7 @@ TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
+run_unittests_SOURCES += portconfig_unittest.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

src/lib/server_common/tests/portconfig_unittest.cc

@@ -0,0 +1,106 @@
+// Copyright (C) 2011  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 "../portconfig.h"
+
+#include <cc/data.h>
+#include <exceptions/exceptions.h>
+#include <asiolink/asiolink.h>
+
+#include <gtest/gtest.h>
+#include <string>
+
+using namespace isc::server_common::portconfig;
+using namespace isc::data;
+using namespace isc;
+using namespace std;
+using namespace asiolink;
+
+namespace {
+
+/// Testcase for parseAddresses call (struct, nobody cares about private here)
+struct ParseAddresses : public ::testing::Test {
+    AddressList result_;
+    void empty(ElementPtr config, const string& name) {
+        SCOPED_TRACE(name);
+        EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+        EXPECT_TRUE(result_.empty());
+    }
+    template<class Exception>
+    void invalidTest(const string& json, const string& name) {
+        SCOPED_TRACE(name);
+        ElementPtr config(Element::fromJSON(json));
+        EXPECT_THROW(parseAddresses(config, "test"), Exception) <<
+            "Should throw " << typeid(Exception).name();
+    }
+};
+
+// Parse valid IPv4 address
+TEST_F(ParseAddresses, ipv4) {
+    ElementPtr config(Element::fromJSON("["
+                                        "   {"
+                                        "       \"address\": \"192.0.2.1\","
+                                        "       \"port\": 53"
+                                        "   }"
+                                        "]"));
+    EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+    ASSERT_EQ(1, result_.size());
+    EXPECT_EQ("192.0.2.1", result_[0].first);
+    EXPECT_EQ(53, result_[0].second);
+}
+
+// Parse valid IPv6 address
+TEST_F(ParseAddresses, ipv6) {
+    ElementPtr config(Element::fromJSON("["
+                                        "   {"
+                                        "       \"address\": \"2001:db8::1\","
+                                        "       \"port\": 53"
+                                        "   }"
+                                        "]"));
+    EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+    ASSERT_EQ(1, result_.size());
+    EXPECT_EQ("2001:db8::1", result_[0].first);
+    EXPECT_EQ(53, result_[0].second);
+}
+
+// Parse various versions of empty list
+TEST_F(ParseAddresses, empty) {
+    empty(Element::fromJSON("[]"), "Empty list");
+    empty(ElementPtr(new NullElement), "Null element");
+    empty(ElementPtr(), "Null pointer");
+}
+
+// Reject invalid configs
+TEST_F(ParseAddresses, invalid) {
+    invalidTest<TypeError>("{}", "Not a list");
+    invalidTest<BadValue>("[{}]", "Empty element");
+    invalidTest<TypeError>("[{"
+                           "   \"port\": 1.5,"
+                           "   \"address\": \"192.0.2.1\""
+                           "}]", "Float port");
+    invalidTest<BadValue>("[{"
+                "   \"port\": -5,"
+                "   \"address\": \"192.0.2.1\""
+                "}]", "Negative port");
+    invalidTest<BadValue>("[{"
+                "   \"port\": 1000000,"
+                "   \"address\": \"192.0.2.1\""
+                "}]", "Port too big");
+    invalidTest<IOError>("[{"
+                                   "   \"port\": 53,"
+                                   "   \"address\": \"bad_address\""
+                                   "}]", "Bad address");
+}
+
+}