Repository navigation
sshutil: Avoid double brackets on IPv6 hosts - #7164
Conversation
Karthik-Chowdary
left a comment
Karthik-Chowdary
left a comment
There was a problem hiding this comment.
The normalization is correctly limited to the no-port path: SplitHostPort first preserves valid [v6]:port inputs, then existing brackets are removed before JoinHostPort adds the default. This avoids double brackets without changing explicit-port behavior, and the coverage includes bare/bracketed IPv6, zone identifiers, IPv4, and hostnames. I fetched the PR head and independently ran go test -race ./util/sshutil ./util/gitutil; both packages pass.
|
Can you create an issue for this with a reproducer? I highly suspect this is an LLM generated pull request but it's bordering on authentic enough that I could be convinced to take a look if there's an appropriate paper trail. |
i'm a Brazilian currently finishing cs college, my english is not great so I prefer letting a LLM write the PRs for me, I did not make a issue because the bug seemed easily seen and solvable, but since it's a requirement I made a issue here: #7215 |
jsternberg
left a comment
jsternberg
left a comment
There was a problem hiding this comment.
Minor change to add some documentation to the new section of code but otherwise looks fine to me.
| if strings.HasPrefix(hostport, "[") && strings.HasSuffix(hostport, "]") { | ||
| hostport = hostport[1 : len(hostport)-1] | ||
| } |
There was a problem hiding this comment.
Add a comment to this section mentioning that this is to strip the brackets from an IPv6 host address. I'm a bit surprised the Go functions here for SplitHostPort and JoinHostPort don't handle this at all. SplitHostPort seems to fail if there's no port so we can't use that so this does seem to be the correct way to handle this. It might also be useful to include a comment about how SplitHostPort can't be used here.
done |
jsternberg
left a comment
jsternberg
left a comment
There was a problem hiding this comment.
Squash the commits into one and it LGTM.
A Git URL such as `ssh://git@[::1]/repo.git` supplies `[::1]` as its host. Adding the default SSH port currently produces `[[::1]]:22`, which cannot be dialed. Remove existing brackets before calling `net.JoinHostPort` when no port is present. Tests cover bracketed and bare IPv6 addresses, zone identifiers, explicit ports, IPv4, and hostnames. Signed-off-by: Alexandre Rodrigues <alexandre3ylf@gmail.com>
1e91461 to
521a1be
Compare
4 participants
Fixes #7215, which includes the runnable
llb.Gitreproducer and before/after output.A Git URL such as
ssh://git@[::1]/repo.gitsupplies[::1]as its host. Adding the default SSH port currently produces[[::1]]:22, which cannot be dialed.Remove existing brackets before calling
net.JoinHostPortwhen no port is present. Tests cover bracketed and bare IPv6 addresses, zone identifiers, explicit ports, IPv4, and hostnames.Validation: regression tests fail before the fix and pass afterward.
go test -race ./util/sshutil ./util/gitutilThe configured golangci-lint checks pass for the changed package(s).