From 1560ae8247845484ee5eba0926183626489a4bb5 Mon Sep 17 00:00:00 2001 From: Opendium Date: Wed, 27 Oct 2021 14:20:16 +0000 Subject: [PATCH] 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. --- src/anyp/Uri.cc | 12 ++++++------ src/anyp/Uri.h | 4 ++-- src/ip/Address.cc | 10 ---------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc index 2c41905473..7fa98a455f 100644 --- 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 0ba96ed0d2..bb29734d8b 100644 --- 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 95aa7323d5..99293581d0 100644 --- 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 { -- 2.47.2