Răsfoiți Sursa

Merge pull request #19668 from overleaf/jpa-file-not-found-404

[web] entity not found when downloading file yields 404

GitOrigin-RevId: e6a2a7cf60f717fae53a42f39efb65f1624f71bb
Alf Eaton 2 ani în urmă
părinte
comite
0f2aaa6c0c

+ 5 - 2
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,

+ 9 - 0
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 () {