From 0830760f0fdf5c183d69e83b7ff7a1d225fb84c9 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 8 Feb 2023 08:27:18 -0500 Subject: [PATCH] Merge pull request #11702 from overleaf/em-headers-sent Don't send an error response when headers have already been sent GitOrigin-RevId: a9832022a4f47562d31134d34bfd8c52e1eb4d7e --- .../src/Features/Errors/ErrorController.js | 48 ++++++++++++++----- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/services/web/app/src/Features/Errors/ErrorController.js b/services/web/app/src/Features/Errors/ErrorController.js index 5cb56ee60d..10a4ff2a7b 100644 --- a/services/web/app/src/Features/Errors/ErrorController.js +++ b/services/web/app/src/Features/Errors/ErrorController.js @@ -23,6 +23,7 @@ module.exports = ErrorController = { }, handleError(error, req, res, next) { + const shouldSendErrorResponse = !res.headersSent const user = SessionManager.getSessionUser(req.session) // log errors related to SAML flow if (req.session && req.session.saml) { @@ -33,43 +34,66 @@ module.exports = ErrorController = { { err: error, url: req.url, method: req.method, user }, 'invalid csrf' ) - res.sendStatus(403) + if (shouldSendErrorResponse) { + res.sendStatus(403) + } } else if (error instanceof Errors.NotFoundError) { logger.warn({ err: error, url: req.url }, 'not found error') - ErrorController.notFound(req, res) + if (shouldSendErrorResponse) { + ErrorController.notFound(req, res) + } } else if ( error instanceof URIError && error.message.match(/^Failed to decode param/) ) { logger.warn({ err: error, url: req.url }, 'Express URIError') - res.status(400) - res.render('general/500', { title: 'Invalid Error' }) + if (shouldSendErrorResponse) { + res.status(400) + res.render('general/500', { title: 'Invalid Error' }) + } } else if (error instanceof Errors.ForbiddenError) { logger.error({ err: error }, 'forbidden error') - ErrorController.forbidden(req, res) + if (shouldSendErrorResponse) { + ErrorController.forbidden(req, res) + } } else if (error instanceof Errors.TooManyRequestsError) { logger.warn({ err: error, url: req.url }, 'too many requests error') - res.sendStatus(429) + if (shouldSendErrorResponse) { + res.sendStatus(429) + } } else if (error instanceof Errors.InvalidError) { logger.warn({ err: error, url: req.url }, 'invalid error') - res.status(400) - plainTextResponse(res, error.message) + if (shouldSendErrorResponse) { + res.status(400) + plainTextResponse(res, error.message) + } } else if (error instanceof Errors.InvalidNameError) { logger.warn({ err: error, url: req.url }, 'invalid name error') - res.status(400) - plainTextResponse(res, error.message) + if (shouldSendErrorResponse) { + res.status(400) + plainTextResponse(res, error.message) + } } else if (error instanceof Errors.SAMLSessionDataMissing) { logger.warn( { err: error, url: req.url }, 'missing SAML session data error' ) - HttpErrorHandler.badRequest(req, res, error.message) + if (shouldSendErrorResponse) { + HttpErrorHandler.badRequest(req, res, error.message) + } } else { logger.error( { err: error, url: req.url, method: req.method, user }, 'error passed to top level next middleware' ) - ErrorController.serverError(req, res) + if (shouldSendErrorResponse) { + ErrorController.serverError(req, res) + } + } + if (!shouldSendErrorResponse) { + // Pass the error to the default Express error handler, which will close + // the connection. + next(error) } },