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

Add option to specify bind address when connecting #146

Merged
6 commits merged into from
Jan 8, 2015

Conversation

revmischa
Copy link
Contributor

For machines with multiple IPs or people who want to use IPv6

@revmischa
Copy link
Contributor Author

Eagerly awaiting acceptance of this into upstream.

@martynsmith
Copy link
Owner

It seems to me this would work with ssl? It looks like you're copying the secure stuff in to connectOpts and using that?

@revmischa
Copy link
Contributor Author

I believe that fixes it so the other options get passed through to tls, which may have fixed a bug. This was a long time ago so I don't really remember.

@revmischa
Copy link
Contributor Author

Sup, need anything else done?

@ghost
Copy link

ghost commented Jan 6, 2015

Can you update this so it merges cleanly, and then we can revisit it?

@Trinitas
Copy link
Contributor

Trinitas commented Jan 7, 2015

#241 possible with Webirc

creds = {};
if (typeof self.opt.secure == 'object') {
// copy "secure" opts to options passed to connect()
for (var f in self.opt.secure) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll want to add a hasOwnProperty guard here:

for (var f in creds) {
  if (creds.hasOwnProperty(f)) {
    connectionOpts[f] = creds[f]
  }
}

@jirwin
Copy link
Collaborator

jirwin commented Jan 7, 2015

Looks pretty good to me! add that guard and a rebase and we'll get this merged.

@revmischa
Copy link
Contributor Author

Okay, standby

@@ -39,6 +40,9 @@ Client
If you set `selfSigned` to true SSL accepts certificates from a non trusted CA.
If you set `certExpired` to true, the bot connects even if the ssl cert has expired.

`localAddress` is the address to bind to when connecting. Not
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like you got this working with the secure option. Probably should update this doc.

@jirwin
Copy link
Collaborator

jirwin commented Jan 7, 2015

Actually, this is based against the wrong branch now. I think you may need to re-open this PR against the 0.3.x branch

@revmischa
Copy link
Contributor Author

Commit 5c02ec4 makes rebasing highly problematic

@jirwin
Copy link
Collaborator

jirwin commented Jan 7, 2015

Yeah, this is why I think it will probably be easier to just open a new PR against the 0.3.x branch and add the changes there. Rebasing against that will be a huge pain.

@jirwin
Copy link
Collaborator

jirwin commented Jan 7, 2015

I like this PR a lot.

@ghost
Copy link

ghost commented Jan 7, 2015

+1. I have a box with a bunch of IPs so I can test this pretty easily. Will look at doing that today and will merge assuming all goes well.

ghost pushed a commit that referenced this pull request Jan 8, 2015
Add option to specify bind address when connecting
@ghost ghost merged commit dc7d262 into martynsmith:master Jan 8, 2015
This pull request was closed.
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.

4 participants