Skip to content

Strip the port from a bracketed IPv6 host correctly - #817

Open
januththedev wants to merge 1 commit into
gorilla:mainfrom
januththedev:fix/ipv6-host-port
Open

januththedev wants to merge 1 commit into
gorilla:mainfrom
januththedev:fix/ipv6-host-port

Conversation

@januththedev

Copy link
Copy Markdown

Strip the port from a bracketed IPv6 host correctly

Description

routeRegexp.Match and routeRegexpGroup.setMatch both dropped the port by truncating at the first colon:

// Don't be strict on the port match
if i := strings.Index(host, ":"); i != -1 {
    host = host[:i]
}

An IPv6 literal in a Host header is bracketed per RFC 3986 §3.2.2 ([::1]:8080), so the port separator is the colon after the closing bracket — the first colon is inside the address. Truncating at the first colon destroys the host.

Why it's wrong — two distinct symptoms

  1. Corrupt route variable. r.Host("{host}") with Host: [::1]:8080 sets mux.Vars(req)["host"] == "[". The route still matches (the default pattern [^.]+ happily matches "["), so this fails silently with garbage data in the handler rather than 404ing. For a longer literal it is not even one character: [2001:db8::1]:80 yields "[2001".
  2. False non-match on a literal template. r.Host("[{host}]") never matches [::1]. This is made worse by the escaped pattern: wildcardHostPort is computed from patternStr, and for "[{host}]" that string is ^\[(?P<v0>[^.]+)\]$ — no colon — so port-stripping is wrongly enabled for a template that never intended it.

The fix

// stripHostPort returns host without its optional ":port" suffix.
// An IPv6 literal is enclosed in square brackets (RFC 3986, 3.2.2), so its port
// is separated by the colon following the closing bracket, not by the first
// colon of the host.
func stripHostPort(host string) string {
	if strings.HasPrefix(host, "[") {
		if i := strings.LastIndex(host, "]"); i != -1 {
			return host[:i+1]
		}
	}
	if i := strings.Index(host, ":"); i != -1 {
		return host[:i]
	}
	return host
}

The HasPrefix("[") guard keeps malformed input on the original path, so only valid bracketed literals change behaviour.

Tests

  • TestHostVarWithIPv6 — parameterized over [::1], [::1]:8080, and [2001:db8::1]:80, plus two non-IPv6 controls (example:8080, example) that pass both before and after and therefore pin existing behaviour.
  • TestHostMatchIPv6Literal — asserts Host("[{host}]") matches a [::1] request.

Before the fix, all three IPv6 var cases failed ("[", "[", "[2001") and the literal-template case did not match at all. After: all pass.

top-level PASS subtest PASS total FAIL
Baseline (pristine HEAD) 76 221 347 0
After fix 78 226 354 0

Pre-existing failures on unmodified HEAD: zero, measured directly and again from a pristine git archive copy so the working tree was never disturbed. go test -race cannot run here (gcc: fatal error: cannot execute 'cc1', no C compiler), so CI's exact command is unrunnable locally; gofmt flags every file in the repo including untouched ones because the checkout is CRLF, and I preserved CRLF so the diff has no line-ending churn.

I fetched regexp.go from main and both blocks are byte-identical to this HEAD, so this is unfixed upstream. Searches for IPv6 in issues turn up only closed #134 (2018, about ports). I deliberately avoided the areas already claimed: PR #641 (double-escaping with UseEncodedPath) and the semicolon query-separator cluster (#781/#787/#798/#811).

route.go:471 returning a nil *Route from Queries() on an odd arg count (so r.Queries("a").Handler(h) panics instead of surfacing the error) is a real neighbouring defect I left alone to keep this change narrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant