From f31186d20b6e18455271e1986f48a70a4eae57d0 Mon Sep 17 00:00:00 2001 From: cadorn Date: Tue, 14 Aug 2012 14:34:59 -0700 Subject: [PATCH 1/8] regular expression support for checking routes --- lib/engine.io.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/engine.io.js b/lib/engine.io.js index 888b80700..da32a8d60 100644 --- a/lib/engine.io.js +++ b/lib/engine.io.js @@ -96,14 +96,25 @@ exports.listen = function (port, options, fn) { exports.attach = function (server, options) { var engine = new exports.Server(options) , options = options || {} - , path = (options.path || '/engine.io').replace(/\/$/, '') + , path = '/engine.io' , resource = options.resource || 'default' + if (typeof options.path === 'string') { + path = options.path.replace(/\/$/, ''); + // normalize path path += '/' + resource + '/'; + } else + if (typeof options.path === 'object' && options.path.test) { + path = options.path; + } function check (req) { + if (typeof path === 'string') { return path == req.url.substr(0, path.length); + } else { + return path.test(req.url); + } } // cache and clean up listeners From 007c574c97cdad7bede8ce3dbc47a6b2ed4dea51 Mon Sep 17 00:00:00 2001 From: cadorn Date: Wed, 5 Sep 2012 11:27:29 -0700 Subject: [PATCH 2/8] regexp and check function for path/routes with tests --- lib/engine.io.js | 26 +++++++++++------------ test/engine.io.js | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/lib/engine.io.js b/lib/engine.io.js index da32a8d60..dff945b81 100644 --- a/lib/engine.io.js +++ b/lib/engine.io.js @@ -99,22 +99,22 @@ exports.attach = function (server, options) { , path = '/engine.io' , resource = options.resource || 'default' - if (typeof options.path === 'string') { - path = options.path.replace(/\/$/, ''); + var check = function (req) { + return path == req.url.substr(0, path.length); + } - // normalize path - path += '/' + resource + '/'; + if (typeof options.path === 'string') { + path = options.path.replace(/\/$/, ''); + // normalize path + path += '/' + resource + '/'; + } else + if (typeof options.path === 'function') { + check = options.path; } else if (typeof options.path === 'object' && options.path.test) { - path = options.path; - } - - function check (req) { - if (typeof path === 'string') { - return path == req.url.substr(0, path.length); - } else { - return path.test(req.url); - } + check = function (req) { + return options.path.test(req.url); + }; } // cache and clean up listeners diff --git a/test/engine.io.js b/test/engine.io.js index 0c8d1df13..09c57196d 100644 --- a/test/engine.io.js +++ b/test/engine.io.js @@ -193,4 +193,57 @@ describe('engine', function () { }); }); + describe('attach() at path', function () { + it('should attach at string path', function (done) { + var server = http.createServer() + , engine = eio.attach(server, { + path: "/custom-engine.io-path" + }); + + server.listen(function () { + var uri = 'http://localhost:%d/custom-engine.io-path/default/'.s(server.address().port); + request.get(uri, function (res) { + expect(res.status).to.be(500); + server.once('close', done); + server.close(); + }); + }); + }); + + it('should attach at regexp path', function (done) { + var server = http.createServer() + , engine = eio.attach(server, { + path: /^\/custom-engine.io-[^\/]+/ + }); + + server.listen(function () { + var uri = 'http://localhost:%d/custom-engine.io-path/default/'.s(server.address().port); + request.get(uri, function (res) { + expect(res.status).to.be(500); + server.once('close', done); + server.close(); + }); + }); + }); + + it('should attach at check function path', function (done) { + var server = http.createServer() + , engine = eio.attach(server, { + path: function(req) { + var path = "/custom-engine.io-path"; + return path == req.url.substr(0, path.length); + } + }); + + server.listen(function () { + var uri = 'http://localhost:%d/custom-engine.io-path/default/'.s(server.address().port); + request.get(uri, function (res) { + expect(res.status).to.be(500); + server.once('close', done); + server.close(); + }); + }); + }); + }); + }); From c0fd8e5fcec14368f38905f5931286de9e0dc43c Mon Sep 17 00:00:00 2001 From: cadorn Date: Wed, 5 Sep 2012 12:40:58 -0700 Subject: [PATCH 3/8] many paths test --- test/engine.io.js | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/test/engine.io.js b/test/engine.io.js index 09c57196d..0603614ec 100644 --- a/test/engine.io.js +++ b/test/engine.io.js @@ -243,7 +243,40 @@ describe('engine', function () { server.close(); }); }); - }); + }); + + it('should work with many instances', function (done) { + var server = http.createServer(); + var tests = []; + function attach(i) { + var engine = eio.attach(server, { + path: "/custom-engine.io-path-" + i + }); + tests.push(function(done) { + var uri = ('http://localhost:%d/custom-engine.io-path-' + i + '/default/').s(server.address().port); + request.get(uri, function (res) { + expect(res.status).to.be(500); + done(); + }); + }); + } + for (var i=0; i<20; i++) { + attach(i); + } + server.listen(function () { + var counter = 0; + tests.forEach(function(test) { + counter += 1; + test(function() { + counter -= 1; + if (counter === 0) { + server.once('close', done); + server.close(); + } + }); + }); + }); + }); }); }); From c57caed0c34dd6cc7651c3c8326feb82ee6c4b9b Mon Sep 17 00:00:00 2001 From: cadorn Date: Wed, 5 Sep 2012 13:25:27 -0700 Subject: [PATCH 4/8] many routes per server & refactored path check function --- lib/engine.io.js | 134 ++++++++++++++++++++++++++++++++-------------- test/engine.io.js | 3 +- 2 files changed, 96 insertions(+), 41 deletions(-) diff --git a/lib/engine.io.js b/lib/engine.io.js index dff945b81..53d0b5a99 100644 --- a/lib/engine.io.js +++ b/lib/engine.io.js @@ -5,6 +5,15 @@ var http = require('http') , debug = require('debug')('engine:core') + , events = require('events') + +/** + * Server handler cache. + * + * @api private + */ + +var serverHandlers = []; /** * Protocol revision number. @@ -96,70 +105,115 @@ exports.listen = function (port, options, fn) { exports.attach = function (server, options) { var engine = new exports.Server(options) , options = options || {} - , path = '/engine.io' - , resource = options.resource || 'default' - var check = function (req) { - return path == req.url.substr(0, path.length); - } + function serverHandler() { + for (var i=0; i=0.7 + for (var i = 0, l = listeners.length; i < l; i++) { + oldListeners[i] = listeners[i]; + } + + server.removeAllListeners('request'); + + server.on('close', function () { + handler.emit('close'); + }); + + // add request handler + server.on('request', function (req, res) { + handler.emit('request', req, res); + }); + handler.next = function(req, res) { + for (var i = 0, l = oldListeners.length; i < l; i++) { + oldListeners[i].call(server, req, res); + } + } + + server.on('upgrade', function (req, socket, head) { + handler.emit('upgrade', req, socket, head); + }); + + if (~engine.transports.indexOf('flashsocket') + && false !== options.policyFile) { + server.on('connection', function (socket) { + // NOTE: This only needs to be attached once to any `engine` instance. + engine.handleSocket(socket); + }); + } - // cache and clean up listeners - var listeners = server.listeners('request') - , oldListeners = [] + serverHandlers.push([ + server, + handler + ]); - // copy the references onto a new array for node >=0.7 - for (var i = 0, l = listeners.length; i < l; i++) { - oldListeners[i] = listeners[i]; + return handler; } - server.removeAllListeners('request'); + var handler = serverHandler(); - server.on('close', function () { + handler.on('close', function() { engine.close(); }); + var check = makePathCheck(options.path, options.resource || 'default'); + // add request handler - server.on('request', function (req, res) { + handler.on('request', function (req, res) { if (check(req)) { - debug('intercepting request for path "%s"', path); engine.handleRequest(req, res); } else { - for (var i = 0, l = oldListeners.length; i < l; i++) { - oldListeners[i].call(server, req, res); - } + handler.next(req, res); } }); if(~engine.transports.indexOf('websocket')) { - server.on('upgrade', function (req, socket, head) { + handler.on('upgrade', function (req, socket, head) { if (check(req)) { engine.handleUpgrade(req, socket, head); } else if (false !== options.destroyUpgrade) { socket.end(); } }); - } - - if (~engine.transports.indexOf('flashsocket') - && false !== options.policyFile) { - server.on('connection', function (socket) { - engine.handleSocket(socket); - }); - } + } return engine; }; + +/** + * Make function to check path/route. + * + * @param {Mixed} path + * @param {String} resource + * @return {Function} check function + * @api private + */ + +function makePathCheck(path, resource) { + if (typeof path === 'function') { + return path; + } else + if (typeof path === 'object' && path.test) { + return function (req) { + return path.test(req.url); + }; + } else { + var route = (path || '/engine.io').replace(/\/$/, ''); + // normalize path + route += '/' + resource + '/'; + return function (req) { + return route == req.url.substr(0, route.length); + } + } +} diff --git a/test/engine.io.js b/test/engine.io.js index 0603614ec..066fad8ad 100644 --- a/test/engine.io.js +++ b/test/engine.io.js @@ -246,6 +246,7 @@ describe('engine', function () { }); it('should work with many instances', function (done) { + // TODO: Capture `warning: possible EventEmitter memory leak detected. 11 listeners added.` and fail test. var server = http.createServer(); var tests = []; function attach(i) { @@ -260,7 +261,7 @@ describe('engine', function () { }); }); } - for (var i=0; i<20; i++) { + for (var i=0; i<200; i++) { attach(i); } server.listen(function () { From 337f430a9139bda6e365abefa5ba162e89fce1e9 Mon Sep 17 00:00:00 2001 From: cadorn Date: Wed, 5 Sep 2012 15:51:40 -0700 Subject: [PATCH 5/8] dynamic resource instead of path; immutable URI prefix --- lib/engine.io.js | 25 ++++++++++++++++--------- test/engine.io.js | 28 ++++++++++++++-------------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/lib/engine.io.js b/lib/engine.io.js index 53d0b5a99..0e46f3049 100644 --- a/lib/engine.io.js +++ b/lib/engine.io.js @@ -7,6 +7,14 @@ var http = require('http') , debug = require('debug')('engine:core') , events = require('events') +/** + * Prefix for all URIs. + * + * @api public + */ + +exports.URI_PREFIX = '/engine.io'; + /** * Server handler cache. * @@ -114,7 +122,7 @@ exports.attach = function (server, options) { } var handler = new events.EventEmitter(); - handler.setMaxListeners(10000); + handler.setMaxListeners(100); // cache and clean up listeners var listeners = server.listeners('request') @@ -167,7 +175,7 @@ exports.attach = function (server, options) { engine.close(); }); - var check = makePathCheck(options.path, options.resource || 'default'); + var check = makePathCheck(options.resource); // add request handler handler.on('request', function (req, res) { @@ -200,18 +208,17 @@ exports.attach = function (server, options) { * @api private */ -function makePathCheck(path, resource) { - if (typeof path === 'function') { - return path; +function makePathCheck(resource) { + if (typeof resource === 'function') { + return resource; } else - if (typeof path === 'object' && path.test) { + if (typeof resource === 'object' && resource.test) { return function (req) { - return path.test(req.url); + return resource.test(req.url.substring(exports.URI_PREFIX.length)); }; } else { - var route = (path || '/engine.io').replace(/\/$/, ''); // normalize path - route += '/' + resource + '/'; + var route = exports.URI_PREFIX + '/' + (resource || 'default').replace(/^\/|\/$/g, '') + '/'; return function (req) { return route == req.url.substr(0, route.length); } diff --git a/test/engine.io.js b/test/engine.io.js index 066fad8ad..0f3f11b9c 100644 --- a/test/engine.io.js +++ b/test/engine.io.js @@ -193,15 +193,15 @@ describe('engine', function () { }); }); - describe('attach() at path', function () { - it('should attach at string path', function (done) { + describe('attach() at resource', function () { + it('should attach at string resource', function (done) { var server = http.createServer() , engine = eio.attach(server, { - path: "/custom-engine.io-path" + resource: "/custom/path" }); server.listen(function () { - var uri = 'http://localhost:%d/custom-engine.io-path/default/'.s(server.address().port); + var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); server.once('close', done); @@ -210,14 +210,14 @@ describe('engine', function () { }); }); - it('should attach at regexp path', function (done) { + it('should attach at regexp resource', function (done) { var server = http.createServer() , engine = eio.attach(server, { - path: /^\/custom-engine.io-[^\/]+/ + resource: /^\/custom\/[^\/]+/ }); server.listen(function () { - var uri = 'http://localhost:%d/custom-engine.io-path/default/'.s(server.address().port); + var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); server.once('close', done); @@ -226,17 +226,17 @@ describe('engine', function () { }); }); - it('should attach at check function path', function (done) { + it('should attach at check function resource', function (done) { var server = http.createServer() , engine = eio.attach(server, { - path: function(req) { - var path = "/custom-engine.io-path"; + resource: function(req) { + var path = eio.URI_PREFIX + '/custom/path'; return path == req.url.substr(0, path.length); } }); server.listen(function () { - var uri = 'http://localhost:%d/custom-engine.io-path/default/'.s(server.address().port); + var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); server.once('close', done); @@ -251,17 +251,17 @@ describe('engine', function () { var tests = []; function attach(i) { var engine = eio.attach(server, { - path: "/custom-engine.io-path-" + i + resource: "/custom/path-" + i }); tests.push(function(done) { - var uri = ('http://localhost:%d/custom-engine.io-path-' + i + '/default/').s(server.address().port); + var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path-' + i + '/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); done(); }); }); } - for (var i=0; i<200; i++) { + for (var i=0; i<100; i++) { attach(i); } server.listen(function () { From 253265e5cbcca990f9bb291237ecd1b508cbc5dc Mon Sep 17 00:00:00 2001 From: cadorn Date: Wed, 5 Sep 2012 15:52:29 -0700 Subject: [PATCH 6/8] added contributor --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 3e34e9baf..1e3475d35 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ , "author": "Guillermo Rauch " , "contributors": [ { "name": "Eugen Dueck", "web": "https://github.com/EugenDueck" }, - { "name": "Afshin Mehrabani", "web": "https://github.com/afshinm" } + { "name": "Afshin Mehrabani", "web": "https://github.com/afshinm" }, + { "name": "Christoph Dorn", "web": "https://github.com/cadorn" } ] , "dependencies": { "debug": "0.6.0" From bf25a242b7b5894443e1067c736223f96eab4c37 Mon Sep 17 00:00:00 2001 From: cadorn Date: Thu, 6 Sep 2012 11:00:26 -0700 Subject: [PATCH 7/8] don't use EventEmitter; fix extra next callbacks --- lib/engine.io.js | 75 +++++++++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 27 deletions(-) diff --git a/lib/engine.io.js b/lib/engine.io.js index 0e46f3049..8f6f354a2 100644 --- a/lib/engine.io.js +++ b/lib/engine.io.js @@ -5,7 +5,6 @@ var http = require('http') , debug = require('debug')('engine:core') - , events = require('events') /** * Prefix for all URIs. @@ -16,12 +15,12 @@ var http = require('http') exports.URI_PREFIX = '/engine.io'; /** - * Server handler cache. + * Server listeners. * * @api private */ -var serverHandlers = []; +var serverListeners = []; /** * Protocol revision number. @@ -114,15 +113,18 @@ exports.attach = function (server, options) { var engine = new exports.Server(options) , options = options || {} - function serverHandler() { - for (var i=0; i Date: Thu, 13 Sep 2012 14:18:09 -0700 Subject: [PATCH 8/8] make path configurable again --- lib/engine.io.js | 20 ++++++++------------ test/engine.io.js | 10 +++++----- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/engine.io.js b/lib/engine.io.js index 8f6f354a2..9b547c9fd 100644 --- a/lib/engine.io.js +++ b/lib/engine.io.js @@ -6,14 +6,6 @@ var http = require('http') , debug = require('debug')('engine:core') -/** - * Prefix for all URIs. - * - * @api public - */ - -exports.URI_PREFIX = '/engine.io'; - /** * Server listeners. * @@ -196,7 +188,11 @@ exports.attach = function (server, options) { engine.close(); }); - var check = makePathCheck(options.resource); + var path = '/engine.io'; + if (typeof options.path === 'string') { + path = options.path; + } + var check = makePathCheck(path, options.resource); // add request handler listener.request.push(function (req, res) { @@ -229,17 +225,17 @@ exports.attach = function (server, options) { * @api private */ -function makePathCheck(resource) { +function makePathCheck(path, resource) { if (typeof resource === 'function') { return resource; } else if (typeof resource === 'object' && resource.test) { return function (req) { - return resource.test(req.url.substring(exports.URI_PREFIX.length)); + return resource.test(req.url.substring(path.length)); }; } else { // normalize path - var route = exports.URI_PREFIX + '/' + (resource || 'default').replace(/^\/|\/$/g, '') + '/'; + var route = path + '/' + (resource || 'default').replace(/^\/|\/$/g, '') + '/'; return function (req) { return route == req.url.substr(0, route.length); } diff --git a/test/engine.io.js b/test/engine.io.js index 0f3f11b9c..75cf2eafb 100644 --- a/test/engine.io.js +++ b/test/engine.io.js @@ -201,7 +201,7 @@ describe('engine', function () { }); server.listen(function () { - var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path/').s(server.address().port); + var uri = ('http://localhost:%d/engine.io/custom/path/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); server.once('close', done); @@ -217,7 +217,7 @@ describe('engine', function () { }); server.listen(function () { - var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path/').s(server.address().port); + var uri = ('http://localhost:%d/engine.io/custom/path/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); server.once('close', done); @@ -230,13 +230,13 @@ describe('engine', function () { var server = http.createServer() , engine = eio.attach(server, { resource: function(req) { - var path = eio.URI_PREFIX + '/custom/path'; + var path = '/engine.io/custom/path'; return path == req.url.substr(0, path.length); } }); server.listen(function () { - var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path/').s(server.address().port); + var uri = ('http://localhost:%d/engine.io/custom/path/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); server.once('close', done); @@ -254,7 +254,7 @@ describe('engine', function () { resource: "/custom/path-" + i }); tests.push(function(done) { - var uri = ('http://localhost:%d' + eio.URI_PREFIX + '/custom/path-' + i + '/').s(server.address().port); + var uri = ('http://localhost:%d/engine.io/custom/path-' + i + '/').s(server.address().port); request.get(uri, function (res) { expect(res.status).to.be(500); done();