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() + } + ) + }) + }) + }) +})