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

ws does not support multiple paths #381

Closed
ibc opened this issue Oct 18, 2014 · 9 comments
Closed

ws does not support multiple paths #381

ibc opened this issue Oct 18, 2014 · 9 comments

Comments

@ibc
Copy link
Contributor

ibc commented Oct 18, 2014

var ws = require('ws');
var http = require('http');

var httpServer = http.createServer();
httpServer.listen(10080, '127.0.0.1');

var ws1 = new ws.Server({server:httpServer, path:"/qwe"});
ws1.on('connection', function(){
  console.log("connection on /qwe");
});

var ws2 = new ws.Server({server:httpServer, path:"/asd"});
ws2.on('connection', function(){
  console.log("connection on /asd");
});

OK, now go to a browser (Chrome for example) and open a ws connection to both paths:

CHROME: new WebSocket("ws://local.protoo.org:10080/qwe");
NODE: > connection on /qwe

CHROME: new WebSocket("ws://local.protoo.org:10080/asd");
NODE: > connection on /qwe  <--- same!

And even worst, it behaves wrong after that and does not react on new events, etc. For example try to connect to path "/nonexist" and later again to "/qwe", nothing happens then.

So, is this feature really tested?

@ibc
Copy link
Contributor Author

ibc commented Oct 18, 2014

Easier: create a http server and a single ws Server with some specific path, and add a log on the 'connection' event.

  • Connect from a browser to the correct path. You'll see the log.
  • Connect from a browser to any other invalid path. You'll see nothing (because ws does nothing in this case).
  • Connect from a browser to the correct path. You'll see nothing!

So, this means that if you set "path" and any client attempts to connect to a different path, then ws stops working forever. Wow. Not, this feature cannot be tested at all.

@ibc
Copy link
Contributor Author

ibc commented Oct 18, 2014

This is "fixed" (I mean, removed) in my pull request #379.

@ibc
Copy link
Contributor Author

ibc commented Oct 19, 2014

To be clear. Sharing the same http.Server on two different instances of ws.Server means that the "upgrade" event is called on both, That is a no-go (by testing I've seen even errors in Chrome due to wrong frame received, probably because both ws.Server instances are replying 101, etc.

So, my pull request removes all the path stuff from ws. Instead, the user can inspect the HTTP request in the reqUpgrade field of the ws.WebSocket generated instance, exactly the same he must do when dealing with just http.Server.

@yarax
Copy link

yarax commented Apr 9, 2015

Hi! I proposed a pull request #481 for using regexp both with string paths
I hope it would be useful for the issue

@sushantdhiman
Copy link

I am having same issue when sharing the WS servers with same express server.

@lpinca
Copy link
Member

lpinca commented Nov 9, 2016

I agree with @ibc. The feature is kinda broken when using 2+ server that share the same HTTP server.
I've also proposed a PR (#885) to remove the broken feature.

@3rd-Eden 3rd-Eden closed this as completed Nov 9, 2016
@lpinca
Copy link
Member

lpinca commented Nov 10, 2016

@ibc I think it has been closed because #885 has been merged.

@ibc
Copy link
Contributor Author

ibc commented Nov 10, 2016

Nice to know, thanks. So sorry for my comment.

@lpinca
Copy link
Member

lpinca commented Nov 10, 2016

No problem.

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

No branches or pull requests

5 participants