From 48205c1ddb813f6b270ff90a5c988af800d5fbde Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 7 Feb 2021 19:56:07 -0500 Subject: [PATCH] import/export: Make sure Express sees async errors Express v4.x does not check to see if a Promise returned from a middleware function will be rejected, so explicitly pass the Promise rejection reason to `next()`. We can revert this change after we upgrade to Express v5.0. See https://expressjs.com/en/guide/error-handling.html for details. --- src/node/hooks/express/importexport.js | 94 ++++++++++++++------------ 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/src/node/hooks/express/importexport.js b/src/node/hooks/express/importexport.js index 4dbe816f6..598629632 100644 --- a/src/node/hooks/express/importexport.js +++ b/src/node/hooks/express/importexport.js @@ -21,58 +21,62 @@ const limiter = rateLimit(settings.importExportRateLimiting); exports.expressCreateServer = (hookName, args, cb) => { // handle export requests args.app.use('/p/:pad/:rev?/export/:type', limiter); - args.app.get('/p/:pad/:rev?/export/:type', async (req, res, next) => { - const types = ['pdf', 'doc', 'txt', 'html', 'odt', 'etherpad']; - // send a 404 if we don't support this filetype - if (types.indexOf(req.params.type) === -1) { - return next(); - } - - // if abiword is disabled, and this is a format we only support with abiword, output a message - if (settings.exportAvailable() === 'no' && - ['odt', 'pdf', 'doc'].indexOf(req.params.type) !== -1) { - console.error(`Impossible to export pad "${req.params.pad}" in ${req.params.type} format.` + - ' There is no converter configured'); - - // ACHTUNG: do not include req.params.type in res.send() because there is - // no HTML escaping and it would lead to an XSS - res.send('This export is not enabled at this Etherpad instance. Set the path to Abiword' + - ' or soffice (LibreOffice) in settings.json to enable this feature'); - return; - } - - res.header('Access-Control-Allow-Origin', '*'); - - if (await hasPadAccess(req, res)) { - let padId = req.params.pad; - - let readOnlyId = null; - if (readOnlyManager.isReadOnlyId(padId)) { - readOnlyId = padId; - padId = await readOnlyManager.getPadId(readOnlyId); - } - - const exists = await padManager.doesPadExists(padId); - if (!exists) { - console.warn(`Someone tried to export a pad that doesn't exist (${padId})`); + args.app.get('/p/:pad/:rev?/export/:type', (req, res, next) => { + (async () => { + const types = ['pdf', 'doc', 'txt', 'html', 'odt', 'etherpad']; + // send a 404 if we don't support this filetype + if (types.indexOf(req.params.type) === -1) { return next(); } - console.log(`Exporting pad "${req.params.pad}" in ${req.params.type} format`); - exportHandler.doExport(req, res, padId, readOnlyId, req.params.type); - } + // if abiword is disabled, and this is a format we only support with abiword, output a message + if (settings.exportAvailable() === 'no' && + ['odt', 'pdf', 'doc'].indexOf(req.params.type) !== -1) { + console.error(`Impossible to export pad "${req.params.pad}" in ${req.params.type} format.` + + ' There is no converter configured'); + + // ACHTUNG: do not include req.params.type in res.send() because there is + // no HTML escaping and it would lead to an XSS + res.send('This export is not enabled at this Etherpad instance. Set the path to Abiword' + + ' or soffice (LibreOffice) in settings.json to enable this feature'); + return; + } + + res.header('Access-Control-Allow-Origin', '*'); + + if (await hasPadAccess(req, res)) { + let padId = req.params.pad; + + let readOnlyId = null; + if (readOnlyManager.isReadOnlyId(padId)) { + readOnlyId = padId; + padId = await readOnlyManager.getPadId(readOnlyId); + } + + const exists = await padManager.doesPadExists(padId); + if (!exists) { + console.warn(`Someone tried to export a pad that doesn't exist (${padId})`); + return next(); + } + + console.log(`Exporting pad "${req.params.pad}" in ${req.params.type} format`); + exportHandler.doExport(req, res, padId, readOnlyId, req.params.type); + } + })().catch((err) => next(err || new Error(err))); }); // handle import requests args.app.use('/p/:pad/import', limiter); - args.app.post('/p/:pad/import', async (req, res, next) => { - const {session: {user} = {}} = req; - const {accessStatus} = await securityManager.checkAccess( - req.params.pad, req.cookies.sessionID, req.cookies.token, user); - if (accessStatus !== 'grant' || !webaccess.userCanModify(req.params.pad, req)) { - return res.status(403).send('Forbidden'); - } - await importHandler.doImport(req, res, req.params.pad); + args.app.post('/p/:pad/import', (req, res, next) => { + (async () => { + const {session: {user} = {}} = req; + const {accessStatus} = await securityManager.checkAccess( + req.params.pad, req.cookies.sessionID, req.cookies.token, user); + if (accessStatus !== 'grant' || !webaccess.userCanModify(req.params.pad, req)) { + return res.status(403).send('Forbidden'); + } + await importHandler.doImport(req, res, req.params.pad); + })().catch((err) => next(err || new Error(err))); }); return cb();