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 5880e5e8495c8217881585313c637e892d3196ed..1f7c3da76e3657c941764584af3dd1e0f7c22546 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 bf5964bea50345048a3dcd31b24e2eddaa1114ee..b706ff896897332e440f5c586bb2b02eb90b6fdf 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 a7730b223f8eb76812d24e553b826a50c9b1e871..b01474f0bf0a2a36535d4619a4d6ffb3b2fc7f8e 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
 {