Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SshConnection can now bind to a specific local host address and/or local port #128

Conversation

bastiaanb
Copy link
Contributor

Adds ability to bind to a specific local host address and/or local port to SshConnection via options 'bindAddress' and 'bindPort', like OpenSSHs' '-b' parameter.
Binding to a specific local address or port may be necessary for compliance with network policies (e.g. routing or firewall settings).

@buildhive
Copy link

XebiaLabs » overthere #215 SUCCESS
This pull request looks good
(what's this?)

@vpartington
Copy link
Contributor

Hi @bastiaanb, thanks for your pull request. Being able to specify the bind address and bind port sounds like a useful addition to Overthere.

Unfortunately I'm a little busy right now, so I'll review it next week and get back to you!

Regards, Vincent.

@@ -152,7 +163,7 @@ protected void connect() {
client.addHostKeyVerifier(new PromiscuousVerifier());

try {
client.connect(host, port);
client.connect(host, port, InetAddress.getByName(bindAddress), bindPort);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure supplying 0.0.0.0 and 0 here is the same as not supplying anything? Looking at the code, this seems to cause an additional call to Socket.bind, and there does not seem to be any indication that if you do not make that explicit call, it will bind to 0.0.0.0:0.

Instead use something like the following here?

bindAddress = options.get(BIND_ADDRESS);
...
if (bindAddress == null) {
  client.connect(host, port);
} else {
  client.connect(host, port, InetAddress.getByName(bindAddress), bindPort);
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Using a magic value '0.0.0.0' is not nice in any case and may yield different results. E.g. in case of IPv6.
If if-construct alternative does not support the option of specifying a port but not an address. Tha'ts acceptable I guess.

@bastiaanb
Copy link
Contributor Author

I will create a new pull with suggested improvements.
@demobox thanks for the comments!

@bastiaanb bastiaanb closed this Oct 10, 2014
@demobox
Copy link
Contributor

demobox commented Nov 19, 2014

@bastiaanb @vpartington Ping..?

@hierynomus
Copy link
Contributor

Bastiaan created a new PR for this ;)

@demobox
Copy link
Contributor

demobox commented Nov 20, 2014

Bastiaan created a new PR for this ;)

Ah, #131, I see...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants