[1086] Replace addr#port with addr:port

So that output of addresses is now consistent

examples/host/host.cc

@@ -95,7 +95,7 @@ host_lookup(const char* const name, const char* const dns_class,
         cout << "Name: " << server << "\n";
         // TODO: I guess I have to do a lookup to get that address and aliases
         // too
-        //cout << "Address: " << address << "\n" ; // "#" << port << "\n";
+        //cout << "Address: " << address << "\n" ;
         //cout << "Aliases: " << server << "\n";
     }
 

src/bin/bind10/init.py.in

@@ -38,6 +38,7 @@ __main__.
 
 import sys; sys.path.append ('@@PYTHONPATH@@')
 import os
+from isc.util.address_formatter import AddressFormatter
 
 # If B10_FROM_SOURCE is set in the environment, we use data files
 # from a directory relative to that, otherwise we use the ones
@@ -428,7 +429,7 @@ class Init:
                         port)
         else:
             logger.info(BIND10_STARTING_PROCESS_PORT_ADDRESS,
-                        self.curproc, address, port)
+                        self.curproc, AddressFormatter(address, port))
 
     def log_started(self, pid = None):
         """

src/bin/bind10/init_messages.mes

@@ -285,9 +285,9 @@ The b10-init module is starting the given process.
 The b10-init module is starting the given process, which will listen on the
 given port number.
 
-% BIND10_STARTING_PROCESS_PORT_ADDRESS starting process %1 (to listen on %2#%3)
+% BIND10_STARTING_PROCESS_PORT_ADDRESS starting process %1 (to listen on %2)
 The b10-init module is starting the given process, which will listen on the
-given address and port number (written as <address>#<port>).
+given address and port number (written as <address>:<port>).
 
 % BIND10_STARTUP_COMPLETE BIND 10 started
 All modules have been successfully started, and BIND 10 is now running.

src/bin/stats/stats_httpd.py.in

@@ -35,6 +35,7 @@ import re
 import isc.cc
 import isc.config
 import isc.util.process
+from isc.util.address_formatter import AddressFormatter
 
 import isc.log
 from isc.log_messages.stats_httpd_messages import *
@@ -325,8 +326,8 @@ class StatsHttpd:
                 server_address, HttpHandler,
                 self.xml_handler, self.xsd_handler, self.xsl_handler,
                 self.write_log)
-            logger.info(STATSHTTPD_STARTED, server_address[0],
+            logger.info(STATSHTTPD_STARTED,
-                        server_address[1])
+                        AddressFormatter(server_address, address_family))
             return httpd
         except (socket.gaierror, socket.error,
                 OverflowError, TypeError) as err:
@@ -341,8 +342,8 @@ class StatsHttpd:
         """Closes sockets for HTTP"""
         while len(self.httpd)>0:
             ht = self.httpd.pop()
-            logger.info(STATSHTTPD_CLOSING, ht.server_address[0],
+            logger.info(STATSHTTPD_CLOSING,
-                        ht.server_address[1])
+                        AddressFormatter(ht.server_address))
             ht.server_close()
 
     def start(self):

src/bin/stats/stats_httpd_messages.mes

@@ -24,7 +24,7 @@ The stats-httpd module was unable to connect to the BIND 10 command
 and control bus. A likely problem is that the message bus daemon
 (b10-msgq) is not running. The stats-httpd module will now shut down.
 
-% STATSHTTPD_CLOSING closing %1#%2
+% STATSHTTPD_CLOSING closing %1
 The stats-httpd daemon will stop listening for requests on the given
 address and port number.
 
@@ -80,7 +80,7 @@ and an error is sent back.
 % STATSHTTPD_SHUTDOWN shutting down
 The stats-httpd daemon is shutting down.
 
-% STATSHTTPD_STARTED listening on %1#%2
+% STATSHTTPD_STARTED listening on %1
 The stats-httpd daemon will now start listening for requests on the
 given address and port number.
 

src/lib/python/isc/notify/notify_out.py

@@ -26,6 +26,7 @@ from isc.net import addr
 import isc
 from isc.log_messages.notify_out_messages import *
 from isc.statistics import Counters
+from isc.util.address_formatter import AddressFormatter
 
 logger = isc.log.Logger("notify_out")
 
@@ -433,13 +434,13 @@ class NotifyOut:
                     self._notify_next_target(zone_notify_info)
 
         elif event_type == _EVENT_TIMEOUT and zone_notify_info.notify_try_num > 0:
-            logger.info(NOTIFY_OUT_TIMEOUT, tgt[0], tgt[1])
+            logger.info(NOTIFY_OUT_TIMEOUT, AddressFormatter(tgt))
 
         tgt = zone_notify_info.get_current_notify_target()
         if tgt:
             zone_notify_info.notify_try_num += 1
             if zone_notify_info.notify_try_num > _MAX_NOTIFY_TRY_NUM:
-                logger.warn(NOTIFY_OUT_RETRY_EXCEEDED, tgt[0], tgt[1],
+                logger.warn(NOTIFY_OUT_RETRY_EXCEEDED, AddressFormatter(tgt),
                             _MAX_NOTIFY_TRY_NUM)
                 self._notify_next_target(zone_notify_info)
             else:
@@ -487,15 +488,14 @@ class NotifyOut:
             elif zone_notify_info.get_socket().family == socket.AF_INET6:
                 self._counters.inc('zones', zone_notify_info.zone_name,
                                   'notifyoutv6')
-            logger.info(NOTIFY_OUT_SENDING_NOTIFY, addrinfo[0],
+            logger.info(NOTIFY_OUT_SENDING_NOTIFY, AddressFormatter(addrinfo))
-                        addrinfo[1])
         except (socket.error, addr.InvalidAddress) as err:
-            logger.error(NOTIFY_OUT_SOCKET_ERROR, addrinfo[0],
+            logger.error(NOTIFY_OUT_SOCKET_ERROR, AddressFormatter(addrinfo),
-                         addrinfo[1], err)
+                         err)
             return False
         except addr.InvalidAddress as iae:
-            logger.error(NOTIFY_OUT_INVALID_ADDRESS, addrinfo[0],
+            logger.error(NOTIFY_OUT_INVALID_ADDRESS,
-                         addrinfo[1], iae)
+                         AddressFormatter(addrinfo), iae)
             return False
 
         return True
@@ -544,26 +544,28 @@ class NotifyOut:
         try:
             msg.from_wire(msg_data)
             if not msg.get_header_flag(Message.HEADERFLAG_QR):
-                logger.warn(NOTIFY_OUT_REPLY_QR_NOT_SET, from_addr[0],
+                logger.warn(NOTIFY_OUT_REPLY_QR_NOT_SET,
-                            from_addr[1])
+                            AddressFormatter(from_addr))
                 return _BAD_QR
 
             if msg.get_qid() != zone_notify_info.notify_msg_id:
-                logger.warn(NOTIFY_OUT_REPLY_BAD_QID, from_addr[0],
+                logger.warn(NOTIFY_OUT_REPLY_BAD_QID,
-                            from_addr[1], msg.get_qid(),
+                            AddressFormatter(from_addr), msg.get_qid(),
                             zone_notify_info.notify_msg_id)
                 return _BAD_QUERY_ID
 
             question = msg.get_question()[0]
             if question.get_name() != Name(zone_notify_info.zone_name):
-                logger.warn(NOTIFY_OUT_REPLY_BAD_QUERY_NAME, from_addr[0],
+                logger.warn(NOTIFY_OUT_REPLY_BAD_QUERY_NAME,
-                            from_addr[1], question.get_name().to_text(),
+                            AddressFormatter(from_addr),
+                            question.get_name().to_text(),
                             Name(zone_notify_info.zone_name).to_text())
                 return _BAD_QUERY_NAME
 
             if msg.get_opcode() != Opcode.NOTIFY:
-                logger.warn(NOTIFY_OUT_REPLY_BAD_OPCODE, from_addr[0],
+                logger.warn(NOTIFY_OUT_REPLY_BAD_OPCODE,
-                            from_addr[1], msg.get_opcode().to_text())
+                            AddressFormatter(from_addr),
+                            msg.get_opcode().to_text())
                 return _BAD_OPCODE
         except Exception as err:
             # We don't care what exception, just report it?
@@ -576,8 +578,8 @@ class NotifyOut:
         try:
             msg, addr = sock.recvfrom(512)
         except socket.error as err:
-            logger.error(NOTIFY_OUT_SOCKET_RECV_ERROR, tgt_addr[0],
+            logger.error(NOTIFY_OUT_SOCKET_RECV_ERROR,
-                         tgt_addr[1], err)
+                         AddressFormatter(tgt_addr), err)
             return None
 
         return msg

src/lib/python/isc/notify/notify_out_messages.mes

@@ -27,7 +27,7 @@ because notify_out first identifies a list of available zones before
 this process.  So this means some critical inconsistency in the data
 source or software bug.
 
-% NOTIFY_OUT_INVALID_ADDRESS invalid address %1#%2: %3
+% NOTIFY_OUT_INVALID_ADDRESS invalid address %1: %2
 The notify_out library tried to send a notify message to the given
 address, but it appears to be an invalid address. The configuration
 for secondary nameservers might contain a typographic error, or a
@@ -35,26 +35,26 @@ different BIND 10 module has forgotten to validate its data before
 sending this module a notify command. As such, this should normally
 not happen, and points to an oversight in a different module.
 
-% NOTIFY_OUT_REPLY_BAD_OPCODE bad opcode in notify reply from %1#%2: %3
+% NOTIFY_OUT_REPLY_BAD_OPCODE bad opcode in notify reply from %1: %2
 The notify_out library sent a notify message to the nameserver at
 the given address, but the response did not have the opcode set to
 NOTIFY. The opcode in the response is printed. Since there was a
 response, no more notifies will be sent to this server for this
 notification event.
 
-% NOTIFY_OUT_REPLY_BAD_QID bad QID in notify reply from %1#%2: got %3, should be %4
+% NOTIFY_OUT_REPLY_BAD_QID bad QID in notify reply from %1: got %2, should be %3
 The notify_out library sent a notify message to the nameserver at
 the given address, but the query id in the response does not match
 the one we sent. Since there was a response, no more notifies will
 be sent to this server for this notification event.
 
-% NOTIFY_OUT_REPLY_BAD_QUERY_NAME bad query name in notify reply from %1#%2: got %3, should be %4
+% NOTIFY_OUT_REPLY_BAD_QUERY_NAME bad query name in notify reply from %1: got %2, should be %3
 The notify_out library sent a notify message to the nameserver at
 the given address, but the query name in the response does not match
 the one we sent. Since there was a response, no more notifies will
 be sent to this server for this notification event.
 
-% NOTIFY_OUT_REPLY_QR_NOT_SET QR flags set to 0 in reply to notify from %1#%2
+% NOTIFY_OUT_REPLY_QR_NOT_SET QR flags set to 0 in reply to notify from %1
 The notify_out library sent a notify message to the namesever at the
 given address, but the reply did not have the QR bit set to one.
 Since there was a response, no more notifies will be sent to this
@@ -70,27 +70,27 @@ explicitly. Please file a bug report. Since there was a response,
 no more notifies will be sent to this server for this notification
 event.
 
-% NOTIFY_OUT_RETRY_EXCEEDED notify to %1#%2: number of retries (%3) exceeded
+% NOTIFY_OUT_RETRY_EXCEEDED notify to %1: number of retries (%2) exceeded
 The maximum number of retries for the notify target has been exceeded.
 Either the address of the secondary nameserver is wrong, or it is not
 responding.
 
-% NOTIFY_OUT_SENDING_NOTIFY sending notify to %1#%2
+% NOTIFY_OUT_SENDING_NOTIFY sending notify to %1
 A notify message is sent to the secondary nameserver at the given
 address.
 
-% NOTIFY_OUT_SOCKET_ERROR socket error sending notify to %1#%2: %3
+% NOTIFY_OUT_SOCKET_ERROR socket error sending notify to %1: %2
 There was a network error while trying to send a notify message to
 the given address. The address might be unreachable. The socket
 error is printed and should provide more information.
 
-% NOTIFY_OUT_SOCKET_RECV_ERROR socket error reading notify reply from %1#%2: %3
+% NOTIFY_OUT_SOCKET_RECV_ERROR socket error reading notify reply from %1: %2
 There was a network error while trying to read a notify reply
 message from the given address. The socket error is printed and should
 provide more information.
 
-% NOTIFY_OUT_TIMEOUT retry notify to %1#%2
+% NOTIFY_OUT_TIMEOUT retry notify to %1
-The notify message to the given address (noted as address#port) has
+The notify message to the given address (noted as address:port) has
 timed out, and the message will be resent until the max retry limit
 is reached.
 

src/lib/python/isc/util/Makefile.am

@@ -1,6 +1,7 @@
 SUBDIRS = . cio tests
 
-python_PYTHON = __init__.py process.py socketserver_mixin.py file.py
+python_PYTHON =  __init__.py process.py socketserver_mixin.py file.py
+python_PYTHON += address_formatter.py
 
 pythondir = $(pyexecdir)/isc/util
 

src/lib/python/isc/util/address_formatter.py

@@ -0,0 +1,82 @@
+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+import socket
+
+class AddressFormatter:
+    """
+    A utility class to convert an IP address with a port number to a
+    string.
+
+    It takes a tuple (or list) containing and address string and a port
+    number, and optionally a family.
+
+    If the family is IPv4, the __str__ output will be
+    <address>:<port>
+    If the family is IPv6, the __str__ output will be
+    [<address>]:<port>
+
+    If family is not given, the __str__ method will try to figure it out
+    itself, by checking for the ':' character in the address string.
+
+    This class is designed to delay the conversion until it's explicitly
+    requested, so the conversion doesn't happen if the corresponding log
+    message is suppressed because of its log level (which is often the case
+    for debug messages).
+
+    Note: this optimization comes with the cost of instantiating the
+    formatter object itself.  It's not really clear which overhead is
+    heavier, and we may conclude it's actually better to just generate
+    the strings unconditionally.  Alternatively, we can make the stored
+    address of this object replaceable so that this object can be reused.
+    Right now this is an open issue.
+
+    See also ClientFormatter in the ddns.logger code, which does something
+    similar but based on other criteria (and optional extra value).
+    """
+    def __init__(self, addr, family=None):
+        self.__addr = addr
+        self.__family = family
+
+    def __addr_v4(self):
+        return self.__addr[0] + ':' + str(self.__addr[1])
+
+    def __addr_v6(self):
+        return '[' + self.__addr[0] + ']:' + str(self.__addr[1])
+
+    def __format_addr(self):
+        # Some basic sanity checks, should we leave this out for efficiency?
+        # (especially strings produce unexpected results)
+        if isinstance(self.__addr, str) or\
+           not hasattr(self.__addr, "__getitem__"):
+            raise ValueError("Address must be a list or tuple")
+
+        if self.__family is not None:
+            if self.__family == socket.AF_INET6:
+                return self.__addr_v6()
+            elif self.__family == socket.AF_INET:
+                return self.__addr_v4()
+            else:
+                raise ValueError("Unknown address family: " +
+                                 str(self.__family))
+        else:
+            if self.__addr[0].find(':') >= 0:
+                return self.__addr_v6()
+            else:
+                return self.__addr_v4()
+
+    def __str__(self):
+        return self.__format_addr()
+

src/lib/python/isc/util/tests/Makefile.am

@@ -1,5 +1,6 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 PYTESTS = process_test.py socketserver_mixin_test.py file_test.py
+PYTESTS += address_formatter_test.py
 EXTRA_DIST = $(PYTESTS)
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
@@ -12,7 +13,7 @@ endif
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
-	touch $(abs_top_srcdir)/.coverage 
+	touch $(abs_top_srcdir)/.coverage
 	rm -f .coverage
 	${LN_S} $(abs_top_srcdir)/.coverage .coverage
 endif

src/lib/python/isc/util/tests/address_formatter_test.py

@@ -0,0 +1,68 @@
+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+import socket
+import unittest
+from isc.util.address_formatter import AddressFormatter
+
+class AddressFormatterTest(unittest.TestCase):
+    def test_v4(self):
+        self.assertEqual("127.0.0.1:123",
+                         str(AddressFormatter(("127.0.0.1", 123))))
+        self.assertEqual("127.0.0.1:123",
+                         str(AddressFormatter(("127.0.0.1", 123), None)))
+        self.assertEqual("192.0.2.1:1",
+                         str(AddressFormatter(("192.0.2.1", 1))))
+
+    def test_v6(self):
+        self.assertEqual("[::1]:123",
+                         str(AddressFormatter(("::1", 123))));
+        self.assertEqual("[::1]:123",
+                         str(AddressFormatter(("::1", 123), None)))
+        self.assertEqual("[2001:db8::]:1",
+                         str(AddressFormatter(("2001:db8::", 1))))
+
+    def test_force_family_good(self):
+        self.assertEqual("127.0.0.1:123",
+                         str(AddressFormatter(("127.0.0.1", 123),
+                                              socket.AF_INET)))
+        self.assertEqual("[::1]:123",
+                         str(AddressFormatter(("::1", 123),
+                                              socket.AF_INET6)))
+
+    def test_force_family_bad(self):
+        """
+        These results are 'bad' as in they don't return the value as
+        specified by our guidelines, since the internal check is skipped if
+        the family is given
+        """
+        self.assertEqual("[127.0.0.1]:123",
+                         str(AddressFormatter(("127.0.0.1", 123),
+                                              socket.AF_INET6)))
+        self.assertEqual("::1:123",
+                         str(AddressFormatter(("::1", 123),
+                                              socket.AF_INET)))
+
+    def test_bad_values(self):
+        self.assertRaises(ValueError, str, AddressFormatter("string"))
+        self.assertRaises(ValueError, str, AddressFormatter(None))
+        self.assertRaises(ValueError, str, AddressFormatter(1))
+        self.assertRaises(ValueError, str, AddressFormatter(("::1", 123), 1))
+        self.assertRaises(ValueError, str, AddressFormatter(("::1", 123), 1))
+
+
+
+if __name__ == "__main__":
+    unittest.main()

src/lib/server_common/client.cc

@@ -57,8 +57,14 @@ Client::getRequestSourceIPAddress() const {
 std::string
 Client::toText() const {
     std::stringstream ss;
-    ss << impl_->request_.getRemoteEndpoint().getAddress().toText()
+    const asiolink::IOAddress& addr =
-       << '#' << impl_->request_.getRemoteEndpoint().getPort();
+        impl_->request_.getRemoteEndpoint().getAddress();
+    if (addr.isV6()) {
+        ss << '[' << addr.toText() << ']';
+    } else {
+        ss << addr.toText();
+    }
+    ss << ':' << impl_->request_.getRemoteEndpoint().getPort();
     return (ss.str());
 }
 

src/lib/server_common/client.h

@@ -119,7 +119,7 @@ public:
     ///
     /// (In the initial implementation) the format of the resulting string
     /// is as follows:
-    /// \code <IP address>#<port>
+    /// \code <IP address>:<port>
     /// \endcode
     /// The IP address is the textual representation of the client's IP
     /// address, which is the source address of the request the client has

src/lib/server_common/portconfig.cc

@@ -117,8 +117,14 @@ installListenAddresses(const AddressList& new_addresses,
     try {
         LOG_DEBUG(logger, DBG_TRACE_BASIC, SRVCOMM_SET_LISTEN);
         BOOST_FOREACH(const AddressPair& addr, new_addresses) {
+            string addr_str;
+            if (addr.first.find(':') != string::npos) {
+                addr_str = "[" + addr.first + "]";
+            } else {
+                addr_str = addr.first;
+            }
             LOG_DEBUG(logger, DBG_TRACE_VALUES, SRVCOMM_ADDRESS_VALUE).
-                arg(addr.first).arg(addr.second);
+                arg(addr_str).arg(addr.second);
         }
         setAddresses(service, new_addresses, server_options);
         address_store = new_addresses;

src/lib/server_common/server_common_messages.mes

@@ -72,7 +72,7 @@ which it is running.  The server will continue running to allow
 reconfiguration, but will not be listening on any address or port until
 an administrator does so.
 
-% SRVCOMM_ADDRESS_VALUE address to set: %1#%2
+% SRVCOMM_ADDRESS_VALUE address to set: %1:%2
 Debug message. This lists one address and port value of the set of
 addresses we are going to listen on (eg. there will be one log message
 per pair). This appears only after SRVCOMM_SET_LISTEN, but might

src/lib/server_common/tests/client_unittest.cc

@@ -92,8 +92,8 @@ TEST_F(ClientTest, constructIPv6) {
 }
 
 TEST_F(ClientTest, toText) {
-    EXPECT_EQ("192.0.2.1#53214", client4->toText());
+    EXPECT_EQ("192.0.2.1:53214", client4->toText());
-    EXPECT_EQ("2001:db8::1#53216", client6->toText());
+    EXPECT_EQ("[2001:db8::1]:53216", client6->toText());
 }
 
 // test operator<<.  We simply confirm it appends the result of toText().