fix: auto-bracket bare IPv6 literals in serpent.URL.Set - #33
fix: auto-bracket bare IPv6 literals in serpent.URL.Set#33rowansmithau wants to merge 4 commits into
Conversation
net/url.Parse cannot disambiguate an unbracketed IPv6 host from its trailing port (e.g. "http://2600:2222::22:8080" fails with "invalid port ... after host"). This breaks CODER_DERP_SERVER_RELAY_URL when set from a Kubernetes IPv6 pod IP (coder/coder#27266). Detect this shape in the authority component and wrap the IPv6 literal in brackets before delegating to url.Parse. IPv4 hosts, hostnames, and already-bracketed IPv6 hosts are left untouched, so this is a strict superset of previously-accepted input.
Improves the URL parsing heuristic added in the previous commit:
- Split userinfo ("user[:pass]@") off the authority before host
detection so credentials containing colons are not mistaken for
part of an IPv6 literal.
- Terminate the authority at the first of "/", "?", or "#" (not
just "/") so bare IPv6 hosts followed by a query or fragment are
handled correctly.
- Use netip.ParseAddr + Is6() instead of net.ParseIP so IPv4 hosts
that would parse successfully (e.g. "::ffff:1.2.3.4" forms) are
not misclassified as bracket-worthy IPv6.
- Strip optional IPv6 zone identifiers ("%eth0") before parsing.
Adds test coverage for the new cases and for an invalid-but-
IPv6-shaped host, which must fall through to url.Parse and error
rather than being silently accepted.
The userinfo case (http://user:pass@ipv6:port) is not exercised by any real Coder configuration; the reported bug is exclusively about bare IPv6 literals from Kubernetes pod IPs, which never carry credentials. Keeping the change minimal reduces review surface and avoids maintaining a heuristic for a shape that already produces a reasonable url.Parse error today.
strconv.Atoi already rejects empty strings, signs, and non-digits, matching net/url's own port validation. Removes 12 lines of unnecessary helper code.
|
@rowansmithau Those commits that you see from Blink - That is me and blink trying to come up with what we can to fix this. |
|
I am going to run this against a live deployment and will report back. |
|
Okay. So I had to do the following in order to test this out: Resolve the branch to a SHA: Then use the short SHA from the above command's output: After Ran a tidy: Ran 3 tests with different values for
All 3 cases passed and seemed to have worked correctly. Output of 1 & 2: Output of 3: |
|
I have removed the |
2 participants
An attempt to solve 27266 in coder/coder.
net/url.Parse cannot disambiguate an unbracketed IPv6 host from its trailing port (e.g. "http://2600:2222::22:8080" fails with "invalid port ... after host"). This breaks CODER_DERP_SERVER_RELAY_URL when set from a Kubernetes IPv6 pod IP.
This adds detection of IPv6 address add brackets them accordingly.
Tested on a local build of coder with a replace directive.
I ran
go mod edit -replace github.com/coder/serpent=github.com/coder/serpent@rowan/serpent_ipv6_auto_bracket && go mod tidyfollowed bymake teston coder's main branch and those tests still passed.