[PATCH] urlmatch: add underscore to URL_HOST_CHARS

On Tue, Oct 12, 2021 at 04:53:48PM -0400, Jeff King wrote:

Absent any other input, I'd propose the patch below.

-- >8 --
Subject: urlmatch: add underscore to URL_HOST_CHARS

When parsing a URL to normalize it, we allow hostnames to contain only dot (".") or dash ("-"), plus brackets and colons for IPv6 literals. This matches the old URL standard in RFC 1738, which says:

  host           = hostname | hostnumber
  hostname       = *[ domainlabel "." ] toplabel
  domainlabel    = alphadigit | alphadigit *[ alphadigit | "-" ] alphadigit

But this was later updated by RFC 3986, which is more liberal:

  host        = IP-literal / IPv4address / reg-name
  reg-name    = *( unreserved / pct-encoded / sub-delims )
  unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"

While names with underscore in them are not common and possibly violate some DNS rules, they do work in practice, and we will happily contact them over http://, git://, or ssh://. It seems odd to ignore them for purposes of URL matching, especially when the URL RFC seems to allow them.

There shouldn't be any downside here. It's not a syntactically significant character in a URL, so we won't be confused about parsing; we'd have simply rejected such a URL previously (the test here checks the url code directly, but the obvious user-visible effect would be failing to match credential.http://foo_bar.example.com.helper, or similar config in http.<url>.*).

Arguably we'd want to allow tilde ("~") here, too. There's likewise probably no downside, but I didn't add it simply because it seems like an even less likely character to appear in a hostname.

Reported-by: Alex Waite <alex@waite.eu>
Signed-off-by: Jeff King <peff@peff.net>
---
I'm on the fence regarding "~". I didn't actually test that things like
curl even allow it (I did for underscore by creating a throwaway DNS
name).

 t/t0110-urlmatch-normalization.sh | 2 +-
 urlmatch.c                        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)