Bug 5158: AnyP::Uri::host() mishandles [escaped] IPv6 addresses (#920)

When an address is expected to be a string in the form "Host:Port", the
Host MUST be wrapped in square brackets iff it is a numeric IPv6
address. The ":Port" part is usually optional. i.e. "[2001:db8::1]:443"
or "[2001:db8::1]".

There are 2 bugs relating to the handling of numeric IPv6 addresses:

* Bug 1: AnyP::Uri::host(const char *) is supposed to accept the host
  part of a URI, but just copied it into hostAddr_ instead of using the
  fromHost() method to set hostAddr_.  Since the argument is the host
  part of a URI, numeric IPv6 addresses are wrapped in square brackets,
  which would never be stripped and hostIsNumeric was therefore not set.
  We now use the fromHost() method to process the host name correctly.

* Bug 2: Conversely, AnyP::Uri::hostOrIp() is supposed to return the
  host name, suitable for use in a URI or Host header, which means that
  numeric IPv6 addresses need to be wrapped in square brackets.
  However, that wrapping was never done.  We now use the toHostStr()
  method to correctly format the string.

As far as I know, neither of these bugs actually break anything at the
moment, but they have the potential to break future developments, so
applying this fix should have no operational impact.

Also removed wrong IP::Address::toStr() description from .cc file that
(poorly) duplicated correct description of that method in the header.

diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc
index 2c41905473787c75975c5c3a72c5850f19f85da6..7fa98a455f67ee6f2114c2b12065bf00020b7871 100644 (file)
--- a/src/anyp/Uri.cc
+++ b/src/anyp/Uri.cc
@@ -97,8 +97,7 @@ AnyP::Uri::SlashPath()
 void
 AnyP::Uri::host(const char *src)
 {
-    hostAddr_.setEmpty();
-    hostAddr_ = src;
+    hostAddr_.fromHost(src);
     if (hostAddr_.isAnyAddr()) {
         xstrncpy(host_, src, sizeof(host_));
         hostIsNumeric_ = false;
@@ -113,10 +112,11 @@ AnyP::Uri::host(const char *src)
 SBuf
 AnyP::Uri::hostOrIp() const
 {
-    static char ip[MAX_IPSTRLEN];
-    if (hostIsNumeric())
-        return SBuf(hostIP().toStr(ip, sizeof(ip)));
-    else
+    if (hostIsNumeric()) {
+        static char ip[MAX_IPSTRLEN];
+        const auto hostStrLen = hostIP().toHostStr(ip, sizeof(ip));
+        return SBuf(ip, hostStrLen);
+    } else
         return SBuf(host());
 }
 

diff --git a/src/anyp/Uri.h b/src/anyp/Uri.h
index 0ba96ed0d22c65038390ed2e63f503559bc263a9..bb29734d8b20c6f5bba922e0f67942098f871821 100644 (file)
--- a/src/anyp/Uri.h
+++ b/src/anyp/Uri.h
@@ -87,8 +87,8 @@ public:
     Ip::Address const & hostIP(void) const {return hostAddr_;}
 
     /// \returns the host subcomponent of the authority component
-    /// If the host is an IPv6 address, returns that IP address without
-    /// [brackets]! See RFC 3986 Section 3.2.2.
+    /// If the host is an IPv6 address, returns that IP address with
+    /// [brackets]. See RFC 3986 Section 3.2.2.
     SBuf hostOrIp() const;
 
     void port(unsigned short p) {port_=p; touch();}

diff --git a/src/ip/Address.cc b/src/ip/Address.cc
index 95aa7323d5741bb7f18c4883d3c9812c5b83ead2..99293581d0ff6fa6654ade2e1acc9d2664958ec2 100644 (file)
--- a/src/ip/Address.cc
+++ b/src/ip/Address.cc
@@ -788,16 +788,6 @@ Ip::Address::port(unsigned short prt)
     return prt;
 }
 
-/**
- * toStr Given a buffer writes a readable ascii version of the IPA and/or port stored
- *
- * Buffer must be of a size large enough to hold the converted address.
- * This size is provided in the form of a global defined variable MAX_IPSTRLEN
- * Should a buffer shorter be provided the string result will be truncated
- * at the length of the available buffer.
- *
- * A copy of the buffer is also returned for simple immediate display.
- */
 char *
 Ip::Address::toStr(char* buf, const unsigned int blen, int force) const
 {