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

Using a url string with connect() parses full host instead of hostname #189

Closed
seubert opened this issue May 21, 2014 · 6 comments · Fixed by #183 or #190
Closed

Using a url string with connect() parses full host instead of hostname #189

seubert opened this issue May 21, 2014 · 6 comments · Fixed by #183 or #190

Comments

@seubert
Copy link
Contributor

seubert commented May 21, 2014

I am calling connect() with a URL like so: tcp://username:password@mqtt.redacted.com:1883?clientId=1150293638929, which is, subsequently, calling createClient('1883', 'mqtt.redacted.com:1883', opts). I am thinking this should, instead, be createClient('1883', 'mqtt.redacted.com', opts)?

I am happy to open a PR fixing this, but I'm not sure which is the desired behavior. My case seems to require the second one, which currently doesn't work without a patch.

@mcollina
Copy link
Member

Yes it is, thanks! Remember to add a unit test for it!

@mcollina mcollina mentioned this issue May 22, 2014
8 tasks
@mcollina
Copy link
Member

Any updates on this? I'd love to have it in 0.3.9.

@seubert
Copy link
Contributor Author

seubert commented May 23, 2014

@mcollina The patch is easy, but I'm having trouble figuring out how to put a useful test in. Mocha doesn't have spies so I can't easily make sure the right things are being passed to createClient and I can't find anywhere that the host is exposed after connect() is called. Ideas?

@mcollina
Copy link
Member

Got it, add sinon as a devDepenency and you are done :).

@mcollina
Copy link
Member

Any update on this?

@seubert
Copy link
Contributor Author

seubert commented May 26, 2014

Ahhh, so sorry, I will get this in within the next couple hours. Rough weekend personally.

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 a pull request may close this issue.

2 participants