From 82a72c9b7315c405cd3d7954b9748dc836b3702f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 14 Nov 2019 16:32:59 +0000 Subject: [PATCH] fix missing bodyParser limit --- services/document-updater/app.coffee | 4 +- .../coffee/SettingADocumentTests.coffee | 78 +++++++++++++++++++ .../coffee/helpers/MockWebApi.coffee | 3 +- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 645eb8cc63..e3aee88bf7 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -28,7 +28,7 @@ Metrics.event_loop.monitor(logger, 100) app = express() app.configure -> app.use(Metrics.http.monitor(logger)); - app.use express.bodyParser() + app.use express.bodyParser({limit: (Settings.max_doc_length + 64 * 1024)}) app.use app.router Metrics.injectMetricsRoute(app) @@ -125,6 +125,8 @@ app.use (error, req, res, next) -> res.send 404 else if error instanceof Errors.OpRangeNotAvailableError res.send 422 # Unprocessable Entity + else if error.statusCode is 413 + res.send(413, "request entity too large") else logger.error err: error, req: req, "request errored" res.send(500, "Oops, something went wrong") diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index f5ec74cbd8..45fc732033 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -53,6 +53,11 @@ describe "Setting a document", -> , 200 return null + after -> + MockTrackChangesApi.flushDoc.reset() + MockProjectHistoryApi.flushProject.reset() + MockWebApi.setDocument.reset() + it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -89,6 +94,11 @@ describe "Setting a document", -> setTimeout done, 200 return null + after -> + MockTrackChangesApi.flushDoc.reset() + MockProjectHistoryApi.flushProject.reset() + MockWebApi.setDocument.reset() + it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -110,6 +120,64 @@ describe "Setting a document", -> done() return null + describe "when the updated doc is too large for the body parser", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} + @newLines = [] + while JSON.stringify(@newLines).length < Settings.max_doc_length + 64 * 1024 + @newLines.push("(a long line of text)".repeat(10000)) + console.log("newlines size",JSON.stringify(@newLines).length) + DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) => + @statusCode = res.statusCode + setTimeout done, 200 + return null + + after -> + MockTrackChangesApi.flushDoc.reset() + MockProjectHistoryApi.flushProject.reset() + MockWebApi.setDocument.reset() + + it "should return a 413 status code", -> + @statusCode.should.equal 413 + + it "should not send the updated doc lines to the web api", -> + MockWebApi.setDocument.called.should.equal false + + it "should not flush track changes", -> + MockTrackChangesApi.flushDoc.called.should.equal false + + it "should not flush project history", -> + MockProjectHistoryApi.flushProject.called.should.equal false + + describe "when the updated doc is large but under the bodyParser and HTTPController size limit", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} + + @newLines = [] + while JSON.stringify(@newLines).length < 2 * 1024 * 1024 # limit in HTTPController + @newLines.push("(a long line of text)".repeat(10000)) + @newLines.pop() # remove the line which took it over the limit + console.log("newlines size",JSON.stringify(@newLines).length) + DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) => + @statusCode = res.statusCode + setTimeout done, 200 + return null + + after -> + MockTrackChangesApi.flushDoc.reset() + MockProjectHistoryApi.flushProject.reset() + MockWebApi.setDocument.reset() + + it "should return a 204 status code", -> + @statusCode.should.equal 204 + + it "should send the updated doc lines to the web api", -> + MockWebApi.setDocument + .calledWith(@project_id, @doc_id, @newLines) + .should.equal true + describe "with track changes", -> before -> @lines = ["one", "one and a half", "two", "three"] @@ -139,6 +207,11 @@ describe "Setting a document", -> setTimeout done, 200 return null + after -> + MockTrackChangesApi.flushDoc.reset() + MockProjectHistoryApi.flushProject.reset() + MockWebApi.setDocument.reset() + it "should undo the tracked changes", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) => throw error if error? @@ -161,6 +234,11 @@ describe "Setting a document", -> setTimeout done, 200 return null + after -> + MockTrackChangesApi.flushDoc.reset() + MockProjectHistoryApi.flushProject.reset() + MockWebApi.setDocument.reset() + it "should not undo the tracked changes", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) => throw error if error? diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 5ee673ccf7..4f73017a1d 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -1,5 +1,6 @@ express = require("express") app = express() +MAX_REQUEST_SIZE = 2*(2*1024*1024 + 64*1024) module.exports = MockWebApi = docs: {} @@ -35,7 +36,7 @@ module.exports = MockWebApi = else res.send 404 - app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) => + app.post "/project/:project_id/doc/:doc_id", express.bodyParser({limit: MAX_REQUEST_SIZE}), (req, res, next) => MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, req.body.ranges, req.body.lastUpdatedAt, req.body.lastUpdatedBy, (error) -> if error? res.send 500