Skip to content

Commit

Permalink
[fix] Abort the request if close is called while connecting (#956)
Browse files Browse the repository at this point in the history 
Fixes #388
  • Loading branch information
@lpinca
lpinca committed Jan 10, 2017
1 parent 443d0ba commit 772a814
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 104 deletions.
66 changes: 28 additions & 38 deletions lib/WebSocket.js
View file
Expand Up @@ -262,10 +262,10 @@ class WebSocket extends EventEmitter {
*/
close (code, data) {
if (this.readyState === WebSocket.CLOSED) return;

if (this.readyState === WebSocket.CONNECTING) {
this.readyState = WebSocket.CLOSED;
return;
this._req.abort();
this.emit('error', new Error('closed before the connection is established'));
return this.finalize(true);
}

if (this.readyState === WebSocket.CLOSING) {
Expand Down Expand Up @@ -374,6 +374,11 @@ class WebSocket extends EventEmitter {
*/
terminate () {
if (this.readyState === WebSocket.CLOSED) return;
if (this.readyState === WebSocket.CONNECTING) {
this._req.abort();
this.emit('error', new Error('closed before the connection is established'));
return this.finalize(true);
}

if (this._socket) {
this.readyState = WebSocket.CLOSING;
Expand All @@ -385,8 +390,6 @@ class WebSocket extends EventEmitter {
//
clearTimeout(this._closeTimer);
this._closeTimer = setTimeout(this._finalize, closeTimeout, true);
} else if (this.readyState === WebSocket.CONNECTING) {
this.finalize(true);
}
}
}
Expand Down Expand Up @@ -618,43 +621,32 @@ function initAsClient (address, protocols, options) {

if (agent) requestOptions.agent = agent;

const req = httpObj.get(requestOptions);
this._req = httpObj.get(requestOptions);

this._req.on('error', (error) => {
if (this._req.aborted) return;

req.on('error', (error) => {
this.emit('error', error);
this.finalize(error);
this.finalize(true);
});

req.on('response', (res) => {
var error;

if (!this.emit('unexpected-response', req, res)) {
error = new Error(`unexpected server response (${res.statusCode})`);
req.abort();
this.emit('error', error);
this._req.on('response', (res) => {
if (!this.emit('unexpected-response', this._req, res)) {
this._req.abort();
this.emit('error', new Error(`unexpected server response (${res.statusCode})`));
this.finalize(true);
}

this.finalize(error);
});

req.on('upgrade', (res, socket, head) => {
if (this.readyState === WebSocket.CLOSED) {
// client closed before server accepted connection
this.emit('close');
this.removeAllListeners();
socket.end();
return;
}

this._req.on('upgrade', (res, socket, head) => {
const digest = crypto.createHash('sha1')
.update(key + GUID, 'binary')
.digest('base64');

if (res.headers['sec-websocket-accept'] !== digest) {
socket.destroy();
this.emit('error', new Error('invalid server key'));
this.removeAllListeners();
socket.end();
return;
return this.finalize(true);
}

const serverProt = res.headers['sec-websocket-protocol'];
Expand All @@ -670,30 +662,28 @@ function initAsClient (address, protocols, options) {
}

if (protError) {
socket.destroy();
this.emit('error', new Error(protError));
this.removeAllListeners();
socket.end();
return;
return this.finalize(true);
}

if (serverProt) this.protocol = serverProt;

const serverExtensions = Extensions.parse(res.headers['sec-websocket-extensions']);

if (perMessageDeflate && serverExtensions[PerMessageDeflate.extensionName]) {
try {
perMessageDeflate.accept(serverExtensions[PerMessageDeflate.extensionName]);
} catch (err) {
socket.destroy();
this.emit('error', new Error('invalid extension parameter'));
this.removeAllListeners();
socket.end();
return;
return this.finalize(true);
}

this.extensions[PerMessageDeflate.extensionName] = perMessageDeflate;
}

this._req = null;
this.setSocket(socket, head);

req.removeAllListeners();
agent = null;
});
}
122 changes: 76 additions & 46 deletions test/WebSocket.test.js
View file
Expand Up @@ -95,39 +95,28 @@ describe('WebSocket', function () {
wss.on('connection', (ws) => ws.send('foobar'));
});

it('#url exposes the server url', function (done) {
server.createServer(++port, (srv) => {
const url = `ws://localhost:${port}`;
const ws = new WebSocket(url);

assert.strictEqual(ws.url, url);
it('#url exposes the server url', function () {
const url = `ws://localhost:${port}`;
const ws = new WebSocket(url, { agent: new CustomAgent() });

ws.on('close', () => srv.close(done));
ws.close();
});
assert.strictEqual(ws.url, url);
});

it('#protocolVersion exposes the protocol version', function (done) {
server.createServer(++port, (srv) => {
const ws = new WebSocket(`ws://localhost:${port}`);

assert.strictEqual(ws.protocolVersion, 13);

ws.on('close', () => srv.close(done));
ws.close();
it('#protocolVersion exposes the protocol version', function () {
const ws = new WebSocket(`ws://localhost:${port}`, {
agent: new CustomAgent()
});

assert.strictEqual(ws.protocolVersion, 13);
});

describe('#bufferedAmount', function () {
it('defaults to zero', function (done) {
server.createServer(++port, (srv) => {
const ws = new WebSocket(`ws://localhost:${port}`);

assert.strictEqual(ws.bufferedAmount, 0);

ws.on('close', () => srv.close(done));
ws.close();
it('defaults to zero', function () {
const ws = new WebSocket(`ws://localhost:${port}`, {
agent: new CustomAgent()
});

assert.strictEqual(ws.bufferedAmount, 0);
});

it('defaults to zero upon "open"', function (done) {
Expand Down Expand Up @@ -203,15 +192,12 @@ describe('WebSocket', function () {
});

describe('#readyState', function () {
it('defaults to connecting', function (done) {
server.createServer(++port, (srv) => {
const ws = new WebSocket(`ws://localhost:${port}`);

assert.strictEqual(ws.readyState, WebSocket.CONNECTING);

ws.on('close', () => srv.close(done));
ws.close();
it('defaults to connecting', function () {
const ws = new WebSocket(`ws://localhost:${port}`, {
agent: new CustomAgent()
});

assert.strictEqual(ws.readyState, WebSocket.CONNECTING);
});

it('set to open once connection is established', function (done) {
Expand All @@ -236,7 +222,7 @@ describe('WebSocket', function () {
srv.close(done);
});

ws.close(1001);
ws.on('open', () => ws.close(1001));
});
});

Expand All @@ -249,7 +235,7 @@ describe('WebSocket', function () {
srv.close(done);
});

ws.terminate();
ws.on('open', () => ws.terminate());
});
});
});
Expand Down Expand Up @@ -308,26 +294,68 @@ describe('WebSocket', function () {
});

describe('connection establishing', function () {
it('can disconnect before connection is established', function (done) {
server.createServer(++port, (srv) => {
it('can terminate before connection is established (1/2)', function (done) {
const wss = new WebSocketServer({ port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
ws.on('close', () => srv.close(done));
ws.on('error', (err) => {
assert.ok(err instanceof Error);
assert.strictEqual(err.message, 'closed before the connection is established');
ws.on('close', () => wss.close(done));
});
ws.terminate();
});
});

it('can close before connection is established', function (done) {
server.createServer(++port, (srv) => {
it('can terminate before connection is established (2/2)', function (done) {
const wss = new WebSocketServer({
verifyClient: (info, cb) => setTimeout(cb, 300, true),
port: ++port
}, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
ws.on('close', () => srv.close(done));
ws.on('error', (err) => {
assert.ok(err instanceof Error);
assert.strictEqual(err.message, 'closed before the connection is established');
ws.on('close', () => wss.close(done));
});
setTimeout(() => ws.terminate(), 150);
});
});

it('can close before connection is established (1/2)', function (done) {
const wss = new WebSocketServer({ port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
ws.on('error', (err) => {
assert.ok(err instanceof Error);
assert.strictEqual(err.message, 'closed before the connection is established');
ws.on('close', () => wss.close(done));
});
ws.close(1001);
});
});

it('can close before connection is established (2/2)', function (done) {
const wss = new WebSocketServer({
verifyClient: (info, cb) => setTimeout(cb, 300, true),
port: ++port
}, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
ws.on('error', (err) => {
assert.ok(err instanceof Error);
assert.strictEqual(err.message, 'closed before the connection is established');
ws.on('close', () => wss.close(done));
});
setTimeout(() => ws.close(1001), 150);
});
});

it('can handle error before request is upgraded', function (done) {
// Here, we don't create a server, to guarantee that the connection will
// fail before the request is upgraded
Expand Down Expand Up @@ -803,15 +831,17 @@ describe('WebSocket', function () {
});

it('before connect should pass error through callback, if present', function (done) {
server.createServer(++port, (srv) => {
const wss = new WebSocketServer({ port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

ws.send('hi', (error) => {
assert.ok(error instanceof Error);
srv.close(done);
ws.terminate();
ws.send('hi', (err) => {
assert.ok(err instanceof Error);
assert.strictEqual(err.message, 'not opened');
ws.on('close', () => wss.close(done));
});
});

wss.on('connection', (ws) => ws.close());
});

it('without data should be successful', function (done) {
Expand Down
38 changes: 18 additions & 20 deletions test/WebSocketServer.test.js
View file
Expand Up @@ -202,38 +202,37 @@ describe('WebSocketServer', function () {
const ws = new WebSocket(`ws://localhost:${port}`);
});

wss.on('connection', (client) => {
wss.on('connection', (ws) => {
assert.strictEqual(wss.clients.size, 1);
wss.close();
done();
wss.close(done);
});
});

it('can be disabled', function (done) {
const wss = new WebSocketServer({ port: ++port, clientTracking: false }, () => {
assert.strictEqual(wss.clients, undefined);
const ws = new WebSocket(`ws://localhost:${port}`);

ws.on('open', () => ws.close());
});

wss.on('connection', (client) => {
wss.on('connection', (ws) => {
assert.strictEqual(wss.clients, undefined);
wss.close();
done();
ws.on('close', () => wss.close(done));
});
});

it('is updated when client terminates the connection', function (done) {
const wss = new WebSocketServer({ port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

wss.on('connection', (client) => {
client.on('close', () => {
assert.strictEqual(wss.clients.size, 0);
wss.close();
done();
});
ws.on('open', () => ws.terminate());
});

ws.close();
wss.on('connection', (ws) => {
ws.on('close', () => {
assert.strictEqual(wss.clients.size, 0);
wss.close(done);
});
});
});
Expand All @@ -242,14 +241,13 @@ describe('WebSocketServer', function () {
const wss = new WebSocketServer({ port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`);

wss.on('connection', (client) => {
client.on('close', () => {
assert.strictEqual(wss.clients.size, 0);
wss.close();
done();
});
ws.on('open', () => ws.close());
});

ws.close();
wss.on('connection', (ws) => {
ws.on('close', () => {
assert.strictEqual(wss.clients.size, 0);
wss.close(done);
});
});
});
Expand Down

0 comments on commit 772a814

Please sign in to comment.