Skip to content

Commit

Permalink
[fix] Fix timing issues with middleware (#2948)
Browse files Browse the repository at this point in the history
Using a middleware could previously lead to a connecting client
receiving a connect event from the server before the server triggers
its own connect event.
  • Loading branch information
darrachequesne authored May 22, 2017
1 parent 832b8fc commit 2b21690
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
2 changes: 2 additions & 0 deletions lib/namespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ Namespace.prototype.initAdapter = function(){
*/

Namespace.prototype.use = function(fn){
debug('removing initial packet');
delete this.server.eio.initialPacket;
this.fns.push(fn);
return this;
};
Expand Down
7 changes: 4 additions & 3 deletions lib/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,10 @@ Socket.prototype.onconnect = function(){
debug('socket connected - writing packet');
this.nsp.connected[this.id] = this;
this.join(this.id);
// the CONNECT packet for the default namespace
// has already been sent as an `initialPacket`
if (this.nsp.name !== '/') {
var skip = this.nsp.name === '/' && this.nsp.fns.length === 0;
if (skip) {
debug('packet already sent in initial handshake');
} else {
this.packet({ type: parser.CONNECT });
}
};
Expand Down
29 changes: 29 additions & 0 deletions test/socket.io.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ describe('socket.io', function(){
srv.set('authorization', function(o, f) { f(null, false); });

var socket = client(httpSrv);
socket.on('connect', function(){
expect().fail();
});
socket.on('error', function(err) {
expect(err).to.be('Not authorized');
done();
Expand Down Expand Up @@ -2145,6 +2148,9 @@ describe('socket.io', function(){
});
srv.listen(function(){
var socket = client(srv);
socket.on('connect', function(){
done(new Error('nope'));
});
socket.on('error', function(err){
expect(err).to.be('Authentication error');
done();
Expand All @@ -2163,6 +2169,9 @@ describe('socket.io', function(){
});
srv.listen(function(){
var socket = client(srv);
socket.on('connect', function(){
done(new Error('nope'));
});
socket.on('error', function(err){
expect(err).to.eql({ a: 'b', c: 3 });
done();
Expand All @@ -2186,6 +2195,26 @@ describe('socket.io', function(){
});
});

it('should only call connection after (lengthy) fns', function(done){
var srv = http();
var sio = io(srv);
var authenticated = false;

sio.use(function(socket, next){
setTimeout(function () {
authenticated = true;
next();
}, 300);
});
srv.listen(function(){
var socket = client(srv);
socket.on('connect', function(){
expect(authenticated).to.be(true);
done();
});
});
});

it('should be ignored if socket gets closed', function(done){
var srv = http();
var sio = io(srv);
Expand Down

3 comments on commit 2b21690

@adrai
Copy link

@adrai adrai commented on 2b21690 Jun 1, 2017

Choose a reason for hiding this comment

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

Any ETA when this will be released in a new version? (v2.0.2)

@darrachequesne
Copy link
Member Author

Choose a reason for hiding this comment

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

2.0.2 is out 🔥

@adrai
Copy link

@adrai adrai commented on 2b21690 Jun 1, 2017

Choose a reason for hiding this comment

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

thx a lot

Please sign in to comment.