Skip to content

Commit

Permalink
Ban brackets in CONNECT targets sooner when !Ip::EnableIpv6
Browse files Browse the repository at this point in the history
This change spills branch changes to CONNECT handling code path but
avoids making GET and CONNECT checks more different.

AFAICT, the added parseHost() check does not change the end result of
parseHost() because our fromHost() check already rejects IPv6 addresses
when IPv6 support is disabled. However, it is probably a good idea to
reject all bracketed addresses early when IPv6 support is disabled. This
clarifies the intent and protects us in case something goes wrong with
fromHost() implementation again. In the spirit of this logic, I added an
assertion to double check that fromHost() is returning an IPv6 address
when our parseHost() check expects one.

Context:
#1421 (comment)
  • Loading branch information
rousskov committed Jul 30, 2023
1 parent e9e21ba commit 95fc071
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/anyp/Uri.cc
Original file line number Diff line number Diff line change
Expand Up @@ -425,17 +425,19 @@ AnyP::Uri::parse(const HttpRequestMethod& method, const SBuf &rawUrl)
}

if (validateAsIpv6) {
// TODO: Add this check to parseHost().
// TODO: Remove these parseHost() code snippets after converting to call parseHost().

// We say "IPv6-like" because we do not know whether foundHost
// is IPv6. We only know that it came from URI context where we
// expect to find an IPv6 address.
if (!Ip::EnableIpv6)
throw TextException("unsupported IPv6-like address in uri-host", Here());

// TODO: Remove this parseHost() code snippet after converting to call parseHost().
Ip::Address ipv6check;
if (!ipv6check.fromHost(foundHost))
throw TextException("malformed IPv6 address in uri-host", Here());

Assure(ipv6check.isIPv6());
}
}

Expand Down Expand Up @@ -587,6 +589,10 @@ AnyP::Uri::parseHost(Parser::Tokenizer &tok) const

// IP-literal = "[" ( IPv6address / IPvFuture ) "]"
if (tok.skip('[')) {
// Do not say "IPv6" here: We do not yet know what follows the bracket.
if (!Ip::EnableIpv6)
throw TextException("unsupported bracketed IP address in uri-host", Here());

// Add "." because IPv6address in RFC 3986 includes ls32, which includes
// IPv4address: ls32 = ( h16 ":" h16 ) / IPv4address
// This set rejects IPvFuture that needs a "v" character.
Expand All @@ -609,6 +615,7 @@ AnyP::Uri::parseHost(Parser::Tokenizer &tok) const
if (!ipv6check.fromHost(ipv6ish.c_str()))
throw TextException("malformed bracketed IPv6 address in uri-host", Here());

Assure(ipv6check.isIPv6());
return ipv6ish;
}

Expand All @@ -619,7 +626,8 @@ AnyP::Uri::parseHost(Parser::Tokenizer &tok) const
// non-CONNECT uri-host parsing code to use us.

SBuf otherHost; // IPv4address-ish or reg-name-ish;
// ":" is not in TCHAR so we will stop before any port specification
// Since ":" is not in TCHAR, we will stop before any port specification
// (and will never match an IPv6 address -- no Ip::EnableIpv6 check needed).
if (tok.prefix(otherHost, CharacterSet::TCHAR))
return otherHost;

Expand Down

0 comments on commit 95fc071

Please sign in to comment.