From f9ab04e4f9b14bbb9eb421298f946d0e6c359ed9 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Sat, 15 Aug 2015 19:50:09 +0200 Subject: [PATCH] upgrade to express 4.x cleanup package.json put errorHandler middleware as the last one use accepts module instead of req.acceptedLanguages use normal object notation as it is supported is es5 remove deprecations from auth controller remove deprecations from groups controller Revert "remove deprecations from groups controller" This reverts commit 7cf36295ebbc50c1f2b210a8ab447d7f2c41dba9. Revert "remove deprecations from auth controller" This reverts commit deecd701e0551862ba53ea894f2d87a746384fed. request limit upped to 1mb and 10k chars for query parameters Upgrade to Express 4.x --- package.json | 14 ++-- website/src/controllers/challenges.js | 2 +- website/src/controllers/groups.js | 3 +- website/src/controllers/user.js | 2 +- website/src/i18n.js | 19 +++--- website/src/routes/apiv1.js | 2 +- website/src/routes/apiv2.coffee | 4 +- website/src/server.js | 94 +++++++++++++++------------ 8 files changed, 80 insertions(+), 60 deletions(-) diff --git a/package.json b/package.json index 57b8a4cbc90..f59ab6d71c1 100644 --- a/package.json +++ b/package.json @@ -4,20 +4,25 @@ "version": "0.0.0-152", "main": "./website/src/server.js", "dependencies": { + "accepts": "^1.2.12", "amazon-payments": "0.0.4", "amplitude": "^2.0.1", "async": "~0.9.0", "aws-sdk": "^2.0.25", "babel": "^5.5.4", "gulp-babel": "^5.2.1", + "body-parser": "^1.13.3", "bower": "~1.3.12", "browserify": "~3.30.2", "coffee-script": "1.6.x", "coffeeify": "0.6.0", + "compression": "^1.5.2", "connect-ratelimit": "0.0.7", + "cookie-parser": "^1.3.5", + "cookie-session": "^1.2.0", "coupon-code": "~0.3.0", "domain-middleware": "~0.1.0", - "express": "~3.17.5", + "express": "~4.13.3", "express-csv": "~0.6.0", "firebase": "^2.2.9", "firebase-token-generator": "^2.0.0", @@ -55,6 +60,7 @@ "moment": "~2.8.3", "mongoose": "~3.8.23", "mongoose-id-autoinc": "~2013.7.14-4", + "morgan": "^1.6.1", "nconf": "~0.6.9", "newrelic": "~1.11.2", "nib": "~1.0.1", @@ -71,6 +77,7 @@ "qs": "^2.3.2", "request": "~2.44.0", "s3-upload-stream": "^1.0.6", + "serve-favicon": "^2.3.0", "stripe": "*", "superagent": "~1.4.0", "swagger-node-express": "lefnire/swagger-node-express#habitrpg", @@ -81,11 +88,6 @@ "winston-newrelic": "~0.1.4" }, "private": true, - "subdomain": "habitrpg", - "domains": [ - "habitrpg.com", - "www.habitrpg.com" - ], "engines": { "node": "0.10.x" }, diff --git a/website/src/controllers/challenges.js b/website/src/controllers/challenges.js index a8bc4dc2442..52195035372 100644 --- a/website/src/controllers/challenges.js +++ b/website/src/controllers/challenges.js @@ -297,7 +297,7 @@ function closeChal(cid, broken, cb) { /** * Delete & close */ -api['delete'] = function(req, res, next){ +api.delete = function(req, res, next){ var user = res.locals.user; var cid = req.params.cid; diff --git a/website/src/controllers/groups.js b/website/src/controllers/groups.js index c96a6420b17..800aa0eabf9 100644 --- a/website/src/controllers/groups.js +++ b/website/src/controllers/groups.js @@ -521,6 +521,7 @@ api.leave = function(req, res, next) { group.leave(user, keep, function(err){ if (err) return next(err); user = group = keep = null; + return res.send(204); }); }; @@ -683,7 +684,7 @@ api.invite = function(req, res, next){ } else if (req.body.emails) { inviteByEmails(req.body.emails, group, req, res, next) } else { - return res.json(400,{err: "Can invite only by email or uuid"}); + return res.json(400, {err: "Can only invite by email or uuid"}); } } diff --git a/website/src/controllers/user.js b/website/src/controllers/user.js index 71cd4c54505..f33c65970e4 100644 --- a/website/src/controllers/user.js +++ b/website/src/controllers/user.js @@ -364,7 +364,7 @@ api.cron = function(req, res, next) { // api.reroll // Shared.ops // api.reset // Shared.ops -api['delete'] = function(req, res, next) { +api.delete = function(req, res, next) { var user = res.locals.user; var plan = user.purchased.plan; diff --git a/website/src/i18n.js b/website/src/i18n.js index 2f0f14eeb63..19444f89f04 100644 --- a/website/src/i18n.js +++ b/website/src/i18n.js @@ -1,9 +1,10 @@ -var fs = require('fs'), - path = require('path'), - _ = require('lodash'), - User = require('./models/user').model, - shared = require('../../common'), - translations = {}; +var fs = require('fs'); +var path = require('path'); +var _ = require('lodash'); +var accepts = require('accepts'); +var User = require('./models/user').model; +var shared = require('../../common'); +var translations = {}; var localePath = path.join(__dirname, "/../../common/locales/") @@ -74,7 +75,9 @@ var chineseVersions = ['zh-tw']; var getUserLanguage = function(req, res, next){ var getFromBrowser = function(){ - var acceptable = _(req.acceptedLanguages).map(function(lang){ + var acceptedLanguages = accepts(req).languages(); + + var acceptable = _(acceptedLanguages).map(function(lang){ return lang.slice(0, 2); }).uniq().value(); @@ -83,7 +86,7 @@ var getUserLanguage = function(req, res, next){ var iAcceptedCompleteLang = (matches.length > 0) ? multipleVersionsLanguages.indexOf(matches[0].toLowerCase()) : -1; if(iAcceptedCompleteLang !== -1){ - var acceptedCompleteLang = _.find(req.acceptedLanguages, function(accepted){ + var acceptedCompleteLang = _.find(acceptedLanguages, function(accepted){ return accepted.slice(0, 2) == multipleVersionsLanguages[iAcceptedCompleteLang]; }); diff --git a/website/src/routes/apiv1.js b/website/src/routes/apiv1.js index 5a28ef8f3b3..f2876fedc48 100644 --- a/website/src/routes/apiv1.js +++ b/website/src/routes/apiv1.js @@ -155,7 +155,7 @@ router.post('/user/tasks/:id/:direction', auth.auth, i18n.getUserLanguage, cron, // Tasks router.get('/user/tasks', auth.auth, i18n.getUserLanguage, cron, api.getTasks); router.get('/user/task/:id', auth.auth, i18n.getUserLanguage, cron, api.getTask); -router["delete"]('/user/task/:id', auth.auth, i18n.getUserLanguage, cron, api.deleteTask); +router.delete('/user/task/:id', auth.auth, i18n.getUserLanguage, cron, api.deleteTask); router.post('/user/task', auth.auth, i18n.getUserLanguage, cron, api.addTask); // User diff --git a/website/src/routes/apiv2.coffee b/website/src/routes/apiv2.coffee index 8edc2277aa5..746423684fc 100644 --- a/website/src/routes/apiv2.coffee +++ b/website/src/routes/apiv2.coffee @@ -265,7 +265,7 @@ module.exports = (swagger, v2) -> method: 'DELETE' description: "Delete a user object entirely, USE WITH CAUTION!" middleware: [auth.auth, i18n.getUserLanguage] - action: user["delete"] + action: user.delete "/user/revive": spec: @@ -774,7 +774,7 @@ module.exports = (swagger, v2) -> description: "Delete a challenge" parameters: [path('cid','Challenge id','string')] middleware: [auth.auth, i18n.getUserLanguage] - action: challenges["delete"] + action: challenges.delete "/challenges/{cid}/close": spec: diff --git a/website/src/server.js b/website/src/server.js index 52919d523c0..a211216cc2b 100644 --- a/website/src/server.js +++ b/website/src/server.js @@ -1,5 +1,5 @@ // Only do the minimal amount of work before forking just in case of a dyno restart -var cluster = require("cluster"); +var cluster = require('cluster'); var _ = require('lodash'); var nconf = require('nconf'); var utils = require('./utils'); @@ -8,7 +8,7 @@ var logging = require('./logging'); var isProd = nconf.get('NODE_ENV') === 'production'; var isDev = nconf.get('NODE_ENV') === 'development'; var DISABLE_LOGGING = nconf.get('DISABLE_REQUEST_LOGGING'); -var cores = +nconf.get("WEB_CONCURRENCY") || 0; +var cores = +nconf.get('WEB_CONCURRENCY') || 0; if (cores!==0 && cluster.isMaster && (isDev || isProd)) { // Fork workers. If config.json has CORES=x, use that - otherwise, use all cpus-1 (production) @@ -21,10 +21,10 @@ if (cores!==0 && cluster.isMaster && (isDev || isProd)) { } else { require('coffee-script'); // remove this once we've fully converted over - var express = require("express"); - var http = require("http"); - var path = require("path"); - var swagger = require("swagger-node-express"); + var express = require('express'); + var http = require('http'); + var path = require('path'); + var swagger = require('swagger-node-express'); var autoinc = require('mongoose-id-autoinc'); var shared = require('../../common'); @@ -77,8 +77,8 @@ if (cores!==0 && cluster.isMaster && (isDev || isProd)) { // This auth strategy is no longer used. It's just kept around for auth.js#loginFacebook() (passport._strategies.facebook.userProfile) // The proper fix would be to move to a general OAuth module simply to verify accessTokens passport.use(new FacebookStrategy({ - clientID: nconf.get("FACEBOOK_KEY"), - clientSecret: nconf.get("FACEBOOK_SECRET"), + clientID: nconf.get('FACEBOOK_KEY'), + clientSecret: nconf.get('FACEBOOK_SECRET'), //callbackURL: nconf.get("BASE_URL") + "/auth/facebook/callback" }, function(accessToken, refreshToken, profile, done) { @@ -87,61 +87,75 @@ if (cores!==0 && cluster.isMaster && (isDev || isProd)) { )); // ------------ Server Configuration ------------ - var publicDir = path.join(__dirname, "/../public"); + var publicDir = path.join(__dirname, '/../public'); - app.set("port", nconf.get('PORT')); + app.set('port', nconf.get('PORT')); require('./middlewares/apiThrottle')(app); app.use(require('./middlewares/domain')(server,mongoose)); - if (!isProd && !DISABLE_LOGGING) app.use(express.logger("dev")); - app.use(express.compress()); - app.set("views", __dirname + "/../views"); - app.set("view engine", "jade"); - app.use(express.favicon(publicDir + '/favicon.ico')); + if (!isProd && !DISABLE_LOGGING) app.use(require('morgan')('dev')); + app.use(require('compression')()); + app.set('views', __dirname + '/../views'); + app.set('view engine', 'jade'); + app.use(require('serve-favicon')(publicDir + '/favicon.ico')); app.use(require('./middlewares/cors')); var redirects = require('./middlewares/redirects'); app.use(redirects.forceHabitica); app.use(redirects.forceSSL); - app.use(express.urlencoded()); - app.use(express.json()); + + var bodyParser = require('body-parser'); + // Default limit is 100kb, need that because we actually send whole groups to the server + // FIXME as soon as possible (need to move on the client from $resource -> $http) + app.use(bodyParser.urlencoded({ + limit: '1mb', + parameterLimit: 10000, // Upped for safety from 1k, FIXME as above + extended: true // Uses 'qs' library as old connect middleware + })); + app.use(bodyParser.json({ + limit: '1mb' + })); + app.use(require('method-override')()); //app.use(express.cookieParser(nconf.get('SESSION_SECRET'))); - app.use(express.cookieParser()); - app.use(express.cookieSession({ secret: nconf.get('SESSION_SECRET'), httpOnly: false, cookie: { maxAge: TWO_WEEKS }})); - //app.use(express.session()); + app.use(require('cookie-parser')()); + app.use(require('cookie-session')({ + name: 'connect:sess', // Used to keep backward compatibility with Express 3 cookies + secret: nconf.get('SESSION_SECRET'), + httpOnly: false, + maxAge: TWO_WEEKS + })); // Initialize Passport! Also use passport.session() middleware, to support // persistent login sessions (recommended). app.use(passport.initialize()); app.use(passport.session()); - app.use(app.router); - - var maxAge = isProd ? 31536000000 : 0; - // Cache emojis without copying them to build, they are too many - app.use(express['static'](path.join(__dirname, "/../build"), { maxAge: maxAge })); - app.use('/common/dist', express['static'](publicDir + "/../../common/dist", { maxAge: maxAge })); - app.use('/common/audio', express['static'](publicDir + "/../../common/audio", { maxAge: maxAge })); - app.use('/common/script/public', express['static'](publicDir + "/../../common/script/public", { maxAge: maxAge })); - app.use('/common/img', express['static'](publicDir + "/../../common/img", { maxAge: maxAge })); - app.use(express['static'](publicDir)); - // Custom Directives - app.use(require('./routes/pages').middleware); - app.use(require('./routes/payments').middleware); - app.use(require('./routes/auth').middleware); - app.use(require('./routes/coupon').middleware); - app.use(require('./routes/unsubscription').middleware); + app.use(require('./routes/pages')); + app.use(require('./routes/payments')); + app.use(require('./routes/auth')); + app.use(require('./routes/coupon')); + app.use(require('./routes/unsubscription')); var v2 = express(); app.use('/api/v2', v2); - app.use('/api/v1', require('./routes/apiv1').middleware); - app.use('/export', require('./routes/dataexport').middleware); + app.use('/api/v1', require('./routes/apiv1')); + app.use('/export', require('./routes/dataexport')); require('./routes/apiv2.coffee')(swagger, v2); + + var maxAge = isProd ? 31536000000 : 0; + // Cache emojis without copying them to build, they are too many + app.use(express.static(path.join(__dirname, '/../build'), { maxAge: maxAge })); + app.use('/common/dist', express.static(publicDir + '/../../common/dist', { maxAge: maxAge })); + app.use('/common/audio', express.static(publicDir + '/../../common/audio', { maxAge: maxAge })); + app.use('/common/script/public', express.static(publicDir + '/../../common/script/public', { maxAge: maxAge })); + app.use('/common/img', express.static(publicDir + '/../../common/img', { maxAge: maxAge })); + app.use(express.static(publicDir)); + app.use(require('./middlewares/errorHandler')); server.on('request', app); - server.listen(app.get("port"), function() { - return logging.info("Express server listening on port " + app.get("port")); + server.listen(app.get('port'), function() { + return logging.info('Express server listening on port ' + app.get('port')); }); module.exports = server;