From 4416210471e1964d7c30babc354e0e03d1cc00ca Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 21 Sep 2012 17:12:22 +0200 Subject: [PATCH 1/7] Differentiate between http server and express app --- doc/api/hooks_server-side.md | 3 ++- src/node/hooks/express.js | 12 +++++++----- src/node/hooks/express/socketio.js | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 518e12130..0bcd85a3b 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -54,7 +54,8 @@ Called from: src/node/server.js Things in context: -1. app - the main application object (helpful for adding new paths and such) +1. app - the main express application object (helpful for adding new paths and such) +1. server - the http server object This hook gets called after the application object has been created, but before it starts listening. This is similar to the expressConfigure hook, but it's not guaranteed that the application object will have all relevant configuration variables. diff --git a/src/node/hooks/express.js b/src/node/hooks/express.js index e4ff40d96..eb3f6188a 100644 --- a/src/node/hooks/express.js +++ b/src/node/hooks/express.js @@ -1,4 +1,5 @@ var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks"); +var http = require('http'); var express = require('express'); var settings = require('../utils/Settings'); var fs = require('fs'); @@ -48,17 +49,18 @@ exports.restartServer = function () { server.close(); } - server = express(); // New syntax for express v3 + var app = express(); // New syntax for express v3 + server = http.createServer(app); - server.use(function (req, res, next) { + app.use(function (req, res, next) { res.header("Server", serverName); next(); }); - server.configure(function() { - hooks.callAll("expressConfigure", {"app": server}); + app.configure(function() { + hooks.callAll("expressConfigure", {"app": app}); }); - hooks.callAll("expressCreateServer", {"app": server}); + hooks.callAll("expressCreateServer", {"app": app, "server": server}); server.listen(settings.port, settings.ip); } diff --git a/src/node/hooks/express/socketio.js b/src/node/hooks/express/socketio.js index 4f780cb0b..9e1a010fc 100644 --- a/src/node/hooks/express/socketio.js +++ b/src/node/hooks/express/socketio.js @@ -10,7 +10,7 @@ var connect = require('connect'); exports.expressCreateServer = function (hook_name, args, cb) { //init socket.io and redirect all requests to the MessageHandler - var io = socketio.listen(args.app); + var io = socketio.listen(args.server); /* Require an express session cookie to be present, and load the * session. See http://www.danielbaulig.de/socket-ioexpress for more @@ -62,5 +62,5 @@ exports.expressCreateServer = function (hook_name, args, cb) { socketIORouter.setSocketIO(io); socketIORouter.addComponent("pad", padMessageHandler); - hooks.callAll("socketio", {"app": args.app, "io": io}); + hooks.callAll("socketio", {"app": args.app, "io": io, "server": args.server}); } From ff7cf991c9f6bb6b7d1c9f9d1bd0963050170bb8 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 21 Sep 2012 21:39:08 +0200 Subject: [PATCH 2/7] Upgrade log4js to v0.5 --- src/node/hooks/express/webaccess.js | 1 + src/node/server.js | 10 +++++++--- src/package.json | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index ffced0476..a6a270c02 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -95,6 +95,7 @@ exports.expressConfigure = function (hook_name, args, cb) { // Not installing the log4js connect logger when the log level has a higher severity than INFO since it would not log at that level anyway. if (!(settings.loglevel === "WARN" || settings.loglevel == "ERROR")) args.app.use(log4js.connectLogger(httpLogger, { level: log4js.levels.INFO, format: ':status, :method :url'})); + args.app.use(express.cookieParser()); /* Do not let express create the session, so that we can retain a diff --git a/src/node/server.js b/src/node/server.js index cca76c1f9..b91b0e17c 100755 --- a/src/node/server.js +++ b/src/node/server.js @@ -21,8 +21,15 @@ * limitations under the License. */ +// set up logger var log4js = require('log4js'); +log4js.replaceConsole(); + var settings = require('./utils/Settings'); + +//set loglevel +log4js.setGlobalLogLevel(settings.loglevel); + var db = require('./db/DB'); var async = require('async'); var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins"); @@ -31,9 +38,6 @@ var npm = require("npm/lib/npm.js"); hooks.plugins = plugins; -//set loglevel -log4js.setGlobalLogLevel(settings.loglevel); - async.waterfall([ //initalize the database function (callback) diff --git a/src/package.json b/src/package.json index a7550faed..bee732058 100644 --- a/src/package.json +++ b/src/package.json @@ -22,7 +22,7 @@ "clean-css" : "0.3.2", "uglify-js" : "1.2.5", "formidable" : "1.0.9", - "log4js" : "0.4.1", + "log4js" : "0.5.x", "jsdom-nocontextifiy" : "0.2.10", "async-stacktrace" : "0.0.2", "npm" : "1.1.24", From 71579d14783aa0f020664353f6596142dddfb069 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 22 Sep 2012 13:51:39 +0200 Subject: [PATCH 3/7] Fix res.send (migrate to express v3) --- src/node/hooks/express/adminplugins.js | 8 ++------ src/node/hooks/express/padreadonly.js | 2 +- src/node/hooks/express/padurlsanitize.js | 4 ++-- src/node/hooks/express/webaccess.js | 4 ++-- src/node/padaccess.js | 2 +- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/node/hooks/express/adminplugins.js b/src/node/hooks/express/adminplugins.js index fc274a075..97a0d602f 100644 --- a/src/node/hooks/express/adminplugins.js +++ b/src/node/hooks/express/adminplugins.js @@ -12,14 +12,10 @@ exports.expressCreateServer = function (hook_name, args, cb) { errors: [], }; - res.send(eejs.require( - "ep_etherpad-lite/templates/admin/plugins.html", - render_args), {}); + res.send( eejs.require("ep_etherpad-lite/templates/admin/plugins.html", render_args) ); }); args.app.get('/admin/plugins/info', function(req, res) { - res.send(eejs.require( - "ep_etherpad-lite/templates/admin/plugins-info.html", - {}), {}); + res.send( eejs.require("ep_etherpad-lite/templates/admin/plugins-info.html", {}) ); }); } diff --git a/src/node/hooks/express/padreadonly.js b/src/node/hooks/express/padreadonly.js index 60ece0add..af5cbed39 100644 --- a/src/node/hooks/express/padreadonly.js +++ b/src/node/hooks/express/padreadonly.js @@ -56,7 +56,7 @@ exports.expressCreateServer = function (hook_name, args, cb) { ERR(err); if(err == "notfound") - res.send('404 - Not Found', 404); + res.send(404, '404 - Not Found'); else res.send(html); }); diff --git a/src/node/hooks/express/padurlsanitize.js b/src/node/hooks/express/padurlsanitize.js index 24ec2c3d0..29782b692 100644 --- a/src/node/hooks/express/padurlsanitize.js +++ b/src/node/hooks/express/padurlsanitize.js @@ -7,7 +7,7 @@ exports.expressCreateServer = function (hook_name, args, cb) { //ensure the padname is valid and the url doesn't end with a / if(!padManager.isValidPadId(padId) || /\/$/.test(req.url)) { - res.send('Such a padname is forbidden', 404); + res.send(404, 'Such a padname is forbidden'); } else { @@ -19,7 +19,7 @@ exports.expressCreateServer = function (hook_name, args, cb) { var query = url.parse(req.url).query; if ( query ) real_url += '?' + query; res.header('Location', real_url); - res.send('You should be redirected to ' + real_url + '', 302); + res.send(302, 'You should be redirected to ' + real_url + ''); } //the pad id was fine, so just render it else diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index a6a270c02..99971206e 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -56,10 +56,10 @@ exports.basicAuth = function (req, res, next) { res.header('WWW-Authenticate', 'Basic realm="Protected Area"'); if (req.headers.authorization) { setTimeout(function () { - res.send('Authentication required', 401); + res.send(401, 'Authentication required'); }, 1000); } else { - res.send('Authentication required', 401); + res.send(401, 'Authentication required'); } })); } diff --git a/src/node/padaccess.js b/src/node/padaccess.js index a3d1df332..4388ab946 100644 --- a/src/node/padaccess.js +++ b/src/node/padaccess.js @@ -15,7 +15,7 @@ module.exports = function (req, res, callback) { callback(); //no access } else { - res.send("403 - Can't touch this", 403); + res.send(403, "403 - Can't touch this"); } }); } From 794c3d1afe9616fdd611fc8cc8f1c0131998a6a2 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 22 Sep 2012 14:05:41 +0200 Subject: [PATCH 4/7] Set secret on cookieParser (migrate to express v3) --- src/node/hooks/express/webaccess.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index 99971206e..28cb649e4 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -95,8 +95,6 @@ exports.expressConfigure = function (hook_name, args, cb) { // Not installing the log4js connect logger when the log level has a higher severity than INFO since it would not log at that level anyway. if (!(settings.loglevel === "WARN" || settings.loglevel == "ERROR")) args.app.use(log4js.connectLogger(httpLogger, { level: log4js.levels.INFO, format: ':status, :method :url'})); - - args.app.use(express.cookieParser()); /* Do not let express create the session, so that we can retain a * reference to it for socket.io to use. Also, set the key (cookie @@ -107,11 +105,12 @@ exports.expressConfigure = function (hook_name, args, cb) { exports.sessionStore = new express.session.MemoryStore(); secret = randomString(32); } + + args.app.use(express.cookieParser(secret)); args.app.sessionStore = exports.sessionStore; args.app.use(express.session({store: args.app.sessionStore, - key: 'express_sid', - secret: secret})); + key: 'express_sid' })); args.app.use(exports.basicAuth); } From 0f436d5916807cde879617c85a5aea18b98ae1d4 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 22 Sep 2012 15:22:15 +0200 Subject: [PATCH 5/7] Migrate error handling middleware to express v3 --- src/node/hooks/express/errorhandling.js | 26 +++++-------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/node/hooks/express/errorhandling.js b/src/node/hooks/express/errorhandling.js index 3c5727ed4..749b04273 100644 --- a/src/node/hooks/express/errorhandling.js +++ b/src/node/hooks/express/errorhandling.js @@ -32,31 +32,15 @@ exports.gracefulShutdown = function(err) { exports.expressCreateServer = function (hook_name, args, cb) { exports.app = args.app; - - -/* - Below breaks Express 3, commented out to allow express 3o to run. For mroe details see: - https://github.com/visionmedia/express/wiki/Migrating-from-2.x-to-3.x -/* -/* - args.app.error(function(err, req, res, next){ - res.send(500); - console.error(err.stack ? err.stack : err.toString()); - exports.gracefulShutdown(); - }); -*/ - - - -// args.app.on('close', function(){ -// console.log("Exited in a sloppy fashion"); -// }) - + // Handle errors args.app.use(function(err, req, res, next){ // if an error occurs Connect will pass it down // through these "error-handling" middleware // allowing you to respond however you like - res.send(500, { error: 'Sorry something bad happened!' }); + res.send(500, { error: 'Sorry, something bad happened!' }); + console.error(err.stack? err.stack : err.toString()); + exports.gracefulShutdown(); + next(); }) From 0c9c1f514fba98b4333d92ce6a331818ec2ebe97 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 22 Sep 2012 16:03:40 +0200 Subject: [PATCH 6/7] Fix socket.io auth: Use connect to parse signed cookies (migrate to express v3) --- src/node/hooks/express/socketio.js | 22 ++++++++++++++++------ src/node/hooks/express/webaccess.js | 6 +++--- src/package.json | 2 +- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/node/hooks/express/socketio.js b/src/node/hooks/express/socketio.js index 9e1a010fc..546ba2af6 100644 --- a/src/node/hooks/express/socketio.js +++ b/src/node/hooks/express/socketio.js @@ -3,6 +3,7 @@ var socketio = require('socket.io'); var settings = require('../../utils/Settings'); var socketIORouter = require("../../handler/SocketIORouter"); var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks"); +var webaccess = require("ep_etherpad-lite/node/hooks/express/webaccess"); var padMessageHandler = require("../../handler/PadMessageHandler"); @@ -17,12 +18,21 @@ exports.expressCreateServer = function (hook_name, args, cb) { * info */ io.set('authorization', function (data, accept) { if (!data.headers.cookie) return accept('No session cookie transmitted.', false); - data.cookie = connect.utils.parseCookie(data.headers.cookie); - data.sessionID = data.cookie.express_sid; - args.app.sessionStore.get(data.sessionID, function (err, session) { - if (err || !session) return accept('Bad session / session has expired', false); - data.session = new connect.middleware.session.Session(data, session); - accept(null, true); + + // Use connect's cookie parser, because it knows how to parse signed cookies + connect.cookieParser(webaccess.secret)(data, {}, function(err){ + if(err) { + console.error(err); + accept("Couldn't parse request cookies. ", false); + return; + } + + data.sessionID = data.signedCookies.express_sid; + args.app.sessionStore.get(data.sessionID, function (err, session) { + if (err || !session) return accept('Bad session / session has expired', false); + data.session = new connect.middleware.session.Session(data, session); + accept(null, true); + }); }); }); diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index 28cb649e4..41bf38805 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -88,7 +88,7 @@ exports.basicAuth = function (req, res, next) { }); } -var secret = null; +exports.secret = null; exports.expressConfigure = function (hook_name, args, cb) { // If the log level specified in the config file is WARN or ERROR the application server never starts listening to requests as reported in issue #158. @@ -103,10 +103,10 @@ exports.expressConfigure = function (hook_name, args, cb) { if (!exports.sessionStore) { exports.sessionStore = new express.session.MemoryStore(); - secret = randomString(32); + exports.secret = randomString(32); } - args.app.use(express.cookieParser(secret)); + args.app.use(express.cookieParser(exports.secret)); args.app.sessionStore = exports.sessionStore; args.app.use(express.session({store: args.app.sessionStore, diff --git a/src/package.json b/src/package.json index bee732058..67aa1037c 100644 --- a/src/package.json +++ b/src/package.json @@ -18,7 +18,7 @@ "ueberDB" : "0.1.7", "async" : "0.1.x", "express" : "3.x", - "connect" : "1.x", + "connect" : "2.4.x", "clean-css" : "0.3.2", "uglify-js" : "1.2.5", "formidable" : "1.0.9", From 1c38f5bab9a2c78655c9230e0fca0cfb80a7b8f3 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 22 Sep 2012 16:04:30 +0200 Subject: [PATCH 7/7] Update docs --- doc/api/hooks_server-side.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 0bcd85a3b..c60bbe733 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -55,7 +55,7 @@ Called from: src/node/server.js Things in context: 1. app - the main express application object (helpful for adding new paths and such) -1. server - the http server object +2. server - the http server object This hook gets called after the application object has been created, but before it starts listening. This is similar to the expressConfigure hook, but it's not guaranteed that the application object will have all relevant configuration variables. @@ -77,6 +77,7 @@ Things in context: 1. app - the application object 2. io - the socketio object +3. server - the http server object I have no idea what this is useful for, someone else will have to add this description.