From ca3cb6487d57a59af02043832a0c6b3ae5c8339f Mon Sep 17 00:00:00 2001 From: Alex Kocharin Date: Thu, 13 Nov 2014 21:32:31 +0300 Subject: [PATCH] refactor log and etagify middlewares --- lib/index-api.js | 36 ++++++------ lib/index-web.js | 14 ++--- lib/index.js | 11 ++-- lib/local-storage.js | 1 - lib/middleware.js | 127 ++++++++++++++++++++++--------------------- 5 files changed, 95 insertions(+), 94 deletions(-) diff --git a/lib/index-api.js b/lib/index-api.js index f9ea92fcc..707663536 100644 --- a/lib/index-api.js +++ b/lib/index-api.js @@ -37,17 +37,16 @@ module.exports = function(config, auth, storage) { info = Utils.filter_tarball_urls(info, req, config) var version = req.params.version - if (!version) return res.send(info) + if (!version) return next(info) var t = Utils.get_version(info, version) - if (t != null) return res.send(t) + if (t != null) return next(t) if (info['dist-tags'] != null) { if (info['dist-tags'][version] != null) { version = info['dist-tags'][version] - if ((t = Utils.get_version(info, version)) != null) { - return res.send(t) - } + t = Utils.get_version(info, version) + if (t != null) return next(t) } } @@ -76,7 +75,7 @@ module.exports = function(config, auth, storage) { delete result[pkg] } } - return res.send(result) + return next(result) }) }) @@ -91,12 +90,12 @@ module.exports = function(config, auth, storage) { // npmjs.org sets 10h expire expires: new Date(Date.now() + 10*60*60*1000) }) - res.send({ ok: true, name: 'somebody', roles: [] }) + next({ ok: true, name: 'somebody', roles: [] }) }) app.get('/-/user/:org_couchdb_user', function(req, res, next) { res.status(200) - return res.send({ + next({ ok: 'you are authenticated as "' + req.remote_user.name + '"', }) }) @@ -104,13 +103,13 @@ module.exports = function(config, auth, storage) { app.put('/-/user/:org_couchdb_user/:_rev?/:revision?', function(req, res, next) { if (req.remote_user.name != null) { res.status(201) - return res.send({ + return next({ ok: 'you are authenticated as "' + req.remote_user.name + '"', }) } else { if (typeof(req.body.name) !== 'string' || typeof(req.body.password) !== 'string') { if (typeof(req.body.password_sha)) { - return next( Error[422]("your npm version is outdated\nPlease update to npm@1.4.5 or greater.\nSee https://github.com/rlidwka/sinopia/issues/93 for details.") ) + return next( Error[422]('your npm version is outdated\nPlease update to npm@1.4.5 or greater.\nSee https://github.com/rlidwka/sinopia/issues/93 for details.') ) } else { return next( Error[422]('user/password is not found in request (npm issue?)') ) } @@ -126,7 +125,7 @@ module.exports = function(config, auth, storage) { } res.status(201) - return res.send({ ok: 'user "' + req.body.name + '" created' }) + return next({ ok: 'user "' + req.body.name + '" created' }) }) } }) @@ -140,7 +139,7 @@ module.exports = function(config, auth, storage) { storage.add_tags(req.params.package, tags, function(err) { if (err) return next(err) res.status(201) - return res.send({ ok: 'package tagged' }) + return next({ ok: 'package tagged' }) }) }) @@ -153,7 +152,6 @@ module.exports = function(config, auth, storage) { return next( Error[404]('npm star|unstar calls are not implemented') ) } -debugger try { var metadata = Utils.validate_metadata(req.body, name) } catch(err) { @@ -175,7 +173,7 @@ debugger if (metadata._attachments == null) { if (err) return next(err) res.status(201) - return res.send({ ok: ok_message }) + return next({ ok: ok_message }) } // npm-registry-client 0.3+ embeds tarball into the json upload @@ -208,7 +206,7 @@ debugger if (err) return next(err) res.status(201) - return res.send({ ok: ok_message }) + return next({ ok: ok_message }) }) }) }) @@ -242,7 +240,7 @@ debugger storage.remove_package(req.params.package, function(err) { if (err) return next(err) res.status(201) - return res.send({ ok: 'package removed' }) + return next({ ok: 'package removed' }) }) }) @@ -251,7 +249,7 @@ debugger storage.remove_tarball(req.params.package, req.params.filename, req.params.revision, function(err) { if (err) return next(err) res.status(201) - return res.send({ ok: 'tarball removed' }) + return next({ ok: 'tarball removed' }) }) }) @@ -279,7 +277,7 @@ debugger }) stream.on('success', function() { res.status(201) - return res.send({ + return next({ ok: 'tarball uploaded successfully' }) }) @@ -294,7 +292,7 @@ debugger storage.add_version(name, version, req.body, tag, function(err) { if (err) return next(err) res.status(201) - return res.send({ ok: 'package published' }) + return next({ ok: 'package published' }) }) }) diff --git a/lib/index-web.js b/lib/index-web.js index 5256371aa..f8013e3c6 100644 --- a/lib/index-web.js +++ b/lib/index-web.js @@ -41,7 +41,7 @@ module.exports = function(config, auth, storage) { storage.get_local(function(err, packages) { if (err) throw err // that function shouldn't produce any - res.send(template({ + next(template({ name: config.web.title || 'Sinopia', packages: packages.filter(allow), baseUrl: base, @@ -58,7 +58,7 @@ module.exports = function(config, auth, storage) { app.get('/-/static/:filename', function(req, res, next) { var file = __dirname + '/static/' + req.params.filename res.sendFile(file, function(err) { - if (!err) return; + if (!err) return if (err.status === 404) { next() } else { @@ -68,11 +68,11 @@ module.exports = function(config, auth, storage) { }) app.get('/-/logo', function(req, res, next) { - res.sendfile(config.web.logo ? config.web.logo : __dirname + '/static/logo.png') + res.sendFile(config.web.logo ? config.web.logo : __dirname + '/static/logo.png') }) app.get('/-/logo-sm', function(req, res, next) { - res.sendfile(config.web.logosm ? config.web.logosm : __dirname + '/static/logo-sm.png') + res.sendFile(config.web.logosm ? config.web.logosm : __dirname + '/static/logo-sm.png') }) app.post('/-/login', function(req, res, next) { @@ -99,7 +99,7 @@ module.exports = function(config, auth, storage) { } if (i >= results.length - 1) { - res.send(packages) + next(packages) } else { getData(i + 1) } @@ -109,7 +109,7 @@ module.exports = function(config, auth, storage) { if (results.length) { getData(0) } else { - res.send([]) + next([]) } }) @@ -123,7 +123,7 @@ module.exports = function(config, auth, storage) { app.get('/-/readme/:package/:version?', can('access'), function(req, res, next) { storage.get_package(req.params.package, {req: req}, function(err, info) { if (err) return next(err) - res.send( marked(info.readme || 'ERROR: No README data found!') ) + next( marked(info.readme || 'ERROR: No README data found!') ) }) }) return app diff --git a/lib/index.js b/lib/index.js index 73a1eeadd..af5a2d31b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -25,7 +25,7 @@ module.exports = function(config_hash) { if (err.status && err.status >= 400 && err.status < 600) { if (!res.headersSent) { res.status(err.status) - res.send({ error: err.message || 'unknown error' }) + next({ error: err.message || 'unknown error' }) } } else { Logger.logger.error( { err: err } @@ -35,7 +35,7 @@ module.exports = function(config_hash) { res.destroy() } else if (!res.headersSent) { res.status(500) - res.send({ error: 'internal server error' }) + next({ error: 'internal server error' }) } else { // socket should be already closed } @@ -44,8 +44,8 @@ module.exports = function(config_hash) { next() } + app.use(Middleware.log) app.use(error_reporting_middleware) - app.use(Middleware.log_and_etagify) app.use(function(req, res, next) { res.setHeader('X-Powered-By', config.user_agent) next() @@ -75,7 +75,7 @@ module.exports = function(config_hash) { app.get('/-/_debug', function(req, res) { var do_gc = typeof(global.gc) !== 'undefined' if (do_gc) global.gc() - res.send({ + next({ pid : process.pid, main : process.mainModule.filename, conf : config.self_path, @@ -100,6 +100,7 @@ module.exports = function(config_hash) { }) app.use(function(err, req, res, next) { + if (Object.prototype.toString.call(err) !== '[object Error]') return next(err) if (err.code === 'ECONNABORT' && res.statusCode === 304) return next() if (typeof(res.report_error) !== 'function') { // in case of very early error this middleware may not be loaded before error is generated @@ -109,6 +110,8 @@ module.exports = function(config_hash) { res.report_error(err) }) + app.use(Middleware.final) + return app } diff --git a/lib/local-storage.js b/lib/local-storage.js index 0641aa133..7002c0f38 100644 --- a/lib/local-storage.js +++ b/lib/local-storage.js @@ -309,7 +309,6 @@ Storage.prototype.remove_tarball = function(name, filename, revision, callback) } Storage.prototype.add_tarball = function(name, filename) { -debugger assert(Utils.validate_name(filename)) var stream = MyStreams.UploadTarballStream() diff --git a/lib/middleware.js b/lib/middleware.js index bcc4fc3c9..6180f2773 100644 --- a/lib/middleware.js +++ b/lib/middleware.js @@ -65,12 +65,72 @@ function md5sum(data) { return crypto.createHash('md5').update(data).digest('hex') } -module.exports.log_and_etagify = function(req, res, next) { +module.exports.allow = function(config) { + return function(action) { + return function(req, res, next) { + if (config['allow_'+action](req.params.package, req.remote_user)) { + next() + } else { + if (!req.remote_user.name) { + if (req.remote_user.error) { + var message = "can't "+action+' restricted package, ' + req.remote_user.error + } else { + var message = "can't "+action+" restricted package without auth, did you forget 'npm set always-auth true'?" + } + next( Error[403](message) ) + } else { + next( Error[403]('user ' + req.remote_user.name + + ' not allowed to ' + action + ' it') ) + } + } + } + } +} + +module.exports.final = function(body, req, res, next) { + try { + if (typeof(body) === 'string' || typeof(body) === 'object') { + if (!res.getHeader('Content-type')) { + res.header('Content-type', 'application/json') + } + + if (typeof(body) === 'object' && body != null) { + if (typeof(body.error) === 'string') { + res._sinopia_error = body.error + } + body = JSON.stringify(body, undefined, ' ') + '\n' + } + + // don't send etags with errors + if (!res.statusCode || (res.statusCode >= 200 && res.statusCode < 300)) { + res.header('ETag', '"' + md5sum(body) + '"') + } + } else { + // send(null), send(204), etc. + } + } catch(err) { + // if sinopia sends headers first, and then calls res.send() + // as an error handler, we can't report error properly, + // and should just close socket + if (err.message.match(/set headers after they are sent/)) { + if (res.socket != null) res.socket.destroy() + return + } else { + throw err + } + } + + res.send(body) +} + +module.exports.log = function(req, res, next) { // logger req.log = Logger.logger.child({ sub: 'in' }) var _auth = req.headers.authorization - if (_auth) req.headers.authorization = '' + if (_auth != null) req.headers.authorization = '' + var _cookie = req.headers.cookie + if (_cookie != null) req.headers.cookie = '' var _url = req.url req.url = req.originalUrl @@ -78,51 +138,14 @@ module.exports.log_and_etagify = function(req, res, next) { , '@{ip} requested \'@{req.method} @{req.url}\'' ) req.originalUrl = req.url - if (_auth) req.headers.authorization = _auth + if (_auth != null) req.headers.authorization = _auth + if (_cookie != null) req.headers.cookie = _cookie var bytesin = 0 req.on('data', function(chunk) { bytesin += chunk.length }) - var _send = res.send - res.send = function(body) { - try { - if (typeof(body) === 'string' || typeof(body) === 'object') { - if (!res.getHeader('Content-type')) { - res.header('Content-type', 'application/json') - } - - if (typeof(body) === 'object' && body != null) { - if (typeof(body.error) === 'string') { - res._sinopia_error = body.error - } - body = JSON.stringify(body, undefined, '\t') + '\n' - } - - // don't send etags with errors - if (!res.statusCode || (res.statusCode >= 200 && res.statusCode < 300)) { - res.header('ETag', '"' + md5sum(body) + '"') - } - } else { - // send(null), send(204), etc. - } - } catch(err) { - // if sinopia sends headers first, and then calls res.send() - // as an error handler, we can't report error properly, - // and should just close socket - if (err.message.match(/set headers after they are sent/)) { - if (res.socket != null) res.socket.destroy() - return - } else { - throw err - } - } - - res.send = _send - res.send(body) - } - var bytesout = 0 var _write = res.write res.write = function(buf) { @@ -167,25 +190,3 @@ module.exports.log_and_etagify = function(req, res, next) { next() } -module.exports.allow = function(config) { - return function(action) { - return function(req, res, next) { - if (config['allow_'+action](req.params.package, req.remote_user)) { - next() - } else { - if (!req.remote_user.name) { - if (req.remote_user.error) { - var message = "can't "+action+' restricted package, ' + req.remote_user.error - } else { - var message = "can't "+action+" restricted package without auth, did you forget 'npm set always-auth true'?" - } - next( Error[403](message) ) - } else { - next( Error[403]('user ' + req.remote_user.name - + ' not allowed to ' + action + ' it') ) - } - } - } - } -} -