From 0f2aaa6c0c198200a5f71f90a2f2c33b635d1569 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Fri, 2 Aug 2024 09:13:00 +0100 Subject: [PATCH] Merge pull request #19668 from overleaf/jpa-file-not-found-404 [web] entity not found when downloading file yields 404 GitOrigin-RevId: e6a2a7cf60f717fae53a42f39efb65f1624f71bb --- .../app/src/Features/FileStore/FileStoreController.js | 7 +++++-- .../test/unit/src/FileStore/FileStoreControllerTests.js | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/services/web/app/src/Features/FileStore/FileStoreController.js b/services/web/app/src/Features/FileStore/FileStoreController.js index e4dd34bcd7..ad1ec7b597 100644 --- a/services/web/app/src/Features/FileStore/FileStoreController.js +++ b/services/web/app/src/Features/FileStore/FileStoreController.js @@ -15,18 +15,21 @@ module.exports = { { project_id: projectId, element_id: fileId, type: 'file' }, function (err, file) { if (err) { - if (err.name === 'NotFoundError') { + if (err instanceof Errors.NotFoundError) { logger.warn( { err, projectId, fileId, queryString }, 'entity not found when downloading file' ) + // res.sendStatus() sends a description of the status as body. + // Using res.status().end() avoids sending that fake body. + return res.status(404).end() } else { logger.err( { err, projectId, fileId, queryString }, 'error finding element for downloading file' ) + return res.status(500).end() } - return res.sendStatus(500) } FileStoreHandler.getFileStream( projectId, diff --git a/services/web/test/unit/src/FileStore/FileStoreControllerTests.js b/services/web/test/unit/src/FileStore/FileStoreControllerTests.js index 3092061f9a..e38cf8de3f 100644 --- a/services/web/test/unit/src/FileStore/FileStoreControllerTests.js +++ b/services/web/test/unit/src/FileStore/FileStoreControllerTests.js @@ -93,6 +93,15 @@ describe('FileStoreController', function () { this.controller.getFile(this.req, this.res) }) + it('should return a 404 when not found', function (done) { + this.res.callback = () => { + expect(this.res.statusCode).to.equal(404) + done() + } + this.ProjectLocator.findElement.yields(new Errors.NotFoundError()) + this.controller.getFile(this.req, this.res) + }) + // Test behaviour around handling html files ;['.html', '.htm', '.xhtml'].forEach(extension => { describe(`with a '${extension}' file extension`, function () {