From fbbb39b0c02822e513282726382b57acb2b955b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Alby?= Date: Mon, 9 Mar 2020 08:36:13 -0500 Subject: [PATCH] Merge pull request #2620 from overleaf/ta-body-parser-errors Convert Errors with Status Code To HTTP Errors GitOrigin-RevId: 4c7abf4f9164c1a907fbf38c6e440409a616e047 --- .../src/infrastructure/BodyParserWrapper.js | 33 +++++++ services/web/app/src/infrastructure/Server.js | 2 +- .../acceptance/src/BodyParserErrorsTest.js | 87 +++++++++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 services/web/app/src/infrastructure/BodyParserWrapper.js create mode 100644 services/web/test/acceptance/src/BodyParserErrorsTest.js diff --git a/services/web/app/src/infrastructure/BodyParserWrapper.js b/services/web/app/src/infrastructure/BodyParserWrapper.js new file mode 100644 index 0000000000..985e96e388 --- /dev/null +++ b/services/web/app/src/infrastructure/BodyParserWrapper.js @@ -0,0 +1,33 @@ +const HttpErrors = require('@overleaf/o-error/http') +const bodyParser = require('body-parser') + +const convertToHTTPError = error => { + if (!error.statusCode || error.statusCode < 400 || error.statusCode >= 600) { + // cannot be converted to a HttpError + return error + } + + return new HttpErrors.HttpError({ + message: error.message, + statusCode: error.statusCode + }).withCause(error) +} + +// Wraps a parser and attempt to wrap its error (if any) into a HTTPError so the +// response code is forwarded to the client +const wrapBodyParser = method => opts => { + const middleware = bodyParser[method](opts) + return (req, res, next) => { + middleware(req, res, nextArg => { + if (nextArg instanceof Error) { + return next(convertToHTTPError(nextArg)) + } + next(nextArg) + }) + } +} + +module.exports = { + urlencoded: wrapBodyParser('urlencoded'), + json: wrapBodyParser('json') +} diff --git a/services/web/app/src/infrastructure/Server.js b/services/web/app/src/infrastructure/Server.js index 239aeddad8..582891b10b 100644 --- a/services/web/app/src/infrastructure/Server.js +++ b/services/web/app/src/infrastructure/Server.js @@ -16,7 +16,7 @@ const SessionAutostartMiddleware = require('./SessionAutostartMiddleware') const SessionStoreManager = require('./SessionStoreManager') const session = require('express-session') const RedisStore = require('connect-redis')(session) -const bodyParser = require('body-parser') +const bodyParser = require('./BodyParserWrapper') const methodOverride = require('method-override') const cookieParser = require('cookie-parser') const bearerToken = require('express-bearer-token') diff --git a/services/web/test/acceptance/src/BodyParserErrorsTest.js b/services/web/test/acceptance/src/BodyParserErrorsTest.js new file mode 100644 index 0000000000..fa55a7ec44 --- /dev/null +++ b/services/web/test/acceptance/src/BodyParserErrorsTest.js @@ -0,0 +1,87 @@ +const Settings = require('settings-sharelatex') +const request = require('./helpers/request') + +// create a string that is longer than the max allowed (as defined in Server.js) +const wayTooLongString = 'a'.repeat(2 * Settings.max_doc_length + 64 * 1024 + 1) + +describe('BodyParserErrors', function() { + describe('when request is too large', function() { + describe('json', function() { + it('return 413', function(done) { + request.post( + { + url: '/login', + body: { password: wayTooLongString }, + json: true + }, + (error, response, body) => { + if (error) { + return done(error) + } + response.statusCode.should.equal(413) + body.should.deep.equal({}) + done() + } + ) + }) + }) + + describe('urlencoded', function() { + it('return 413', function(done) { + request.post( + { + url: '/login', + form: { password: wayTooLongString } + }, + (error, response, body) => { + if (error) { + return done(error) + } + response.statusCode.should.equal(413) + body.should.match(/Something went wrong, sorry/) + done() + } + ) + }) + }) + }) + + describe('when request is not too large', function() { + describe('json', function() { + it('return normal status code', function(done) { + request.post( + { + url: '/login', + body: { password: 'foo' }, + json: true + }, + (error, response, body) => { + if (error) { + return done(error) + } + response.statusCode.should.equal(403) + done() + } + ) + }) + }) + + describe('urlencoded', function() { + it('return normal status code', function(done) { + request.post( + { + url: '/login', + form: { password: 'foo' } + }, + (error, response, body) => { + if (error) { + return done(error) + } + response.statusCode.should.equal(403) + done() + } + ) + }) + }) + }) +})