ssh: Omit port option from ssh command unless specified in remote url#6845
Conversation
Yeah, you're right. Looking at git, they specify I added #6851 so that we could discern that as well. Does that make sense in this regard and can you check whether the port was specifically provided using that? |
That is what I wanted. I'll rebase this PR on #6851 and using that to check whether we should add |
jayong93
force-pushed
the
no-default-ssh-port
branch
3 times, most recently
from
35dccc3 to
ce1d2c0
Compare
ethomson
left a comment
ethomson
left a comment
There was a problem hiding this comment.
Sorry; I didn't notice that you'd pushed an update. One thing I notice is that I think that we can avoid an allocation of the port_str
| git_str ssh_cmd = GIT_STR_INIT; | ||
| const char *default_ssh_cmd = "ssh"; | ||
| int error; | ||
| git_str port_str = GIT_STR_INIT; |
There was a problem hiding this comment.
| git_str port_str = GIT_STR_INIT; |
| url->path); | ||
|
|
||
| done: | ||
| git_str_dispose(&port_str); |
There was a problem hiding this comment.
| git_str_dispose(&port_str); |
| if (url->port_specified && (error = git_str_printf(&port_str, "-p %s", url->port)) < 0) | ||
| goto done; | ||
|
|
There was a problem hiding this comment.
| if (url->port_specified && (error = git_str_printf(&port_str, "-p %s", url->port)) < 0) | |
| goto done; |
| error = git_str_printf(out, "%s %s \"%s%s%s\" \"%s\" \"%s\"", | ||
| ssh_cmd.size > 0 ? ssh_cmd.ptr : default_ssh_cmd, | ||
| url->port, | ||
| port_str.ptr, |
There was a problem hiding this comment.
| port_str.ptr, | |
| url->port_specified ? "-p " : "", | |
| url->port_specified ? url->port : "", |
Omit `-p 22` option from ssh command by default. Adding `-p 22` option when a port is not in a remote url causes that `Port` option in a ssh config is ignored.
jayong93
force-pushed
the
no-default-ssh-port
branch
from
ce1d2c0 to
d2389b2
Compare
Right. I've removed the useless allocation. |
|
Thanks for the fix! Great improvement. |
2 participants
Currently, when using
GIT_SSH_EXECfeature,libgit2doen't respect aPortconfig in ssh config because it always insert-p 22option to a ssh command if there is no port value in a git remote url (e.g.git@my-custom-domain.org:user/path). I've experienced this for my company's GHE.One caveat of this implementation is that if you specify port22in a remote url despite a different port in ssh config, it will be ignored.For example, in~/.ssh/config:and if you try to fetch or push with a remote urlssh://git@my-custom-domain.org:22/user/path, the port1234will be used.It would be unlikely someone tries to do this though, I guess.EDITED:
Now
-p <port>option is added only when the port is specified in a remote url.