Просмотр исходного кода

Improve stream error safety

GitOrigin-RevId: de4c512a62d304b3eeb2a1521aac173fa93d8411
andrew rumble 2 лет назад
Родитель
Сommit
487d9125a2

+ 2 - 1
services/clsi/app/js/OutputController.js

@@ -1,5 +1,6 @@
 const OutputFileArchiveManager = require('./OutputFileArchiveManager')
 const OutputFileArchiveManager = require('./OutputFileArchiveManager')
 const { expressify } = require('@overleaf/promise-utils')
 const { expressify } = require('@overleaf/promise-utils')
+const { pipeline } = require('node:stream/promises')
 
 
 async function createOutputZip(req, res) {
 async function createOutputZip(req, res) {
   const {
   const {
@@ -16,7 +17,7 @@ async function createOutputZip(req, res) {
 
 
   res.attachment('output.zip')
   res.attachment('output.zip')
   res.setHeader('X-Content-Type-Options', 'nosniff')
   res.setHeader('X-Content-Type-Options', 'nosniff')
-  archive.pipe(res)
+  await pipeline(archive, res)
 }
 }
 
 
 module.exports = { createOutputZip: expressify(createOutputZip) }
 module.exports = { createOutputZip: expressify(createOutputZip) }

+ 12 - 3
services/clsi/app/js/OutputFileArchiveManager.js

@@ -34,13 +34,22 @@ module.exports = {
     const missingFiles = []
     const missingFiles = []
 
 
     for (const file of validFiles) {
     for (const file of validFiles) {
+      let fileHandle
+
       try {
       try {
-        const fileHandle = await open(file, 'r')
-        const fileStream = fileHandle.createReadStream()
-        archive.append(fileStream, { name: path.basename(file) })
+        fileHandle = await open(file, 'r')
       } catch (error) {
       } catch (error) {
+        logger.warn(
+          { file, error },
+          'error opening file to add to output files archive'
+        )
         missingFiles.push(file)
         missingFiles.push(file)
+        continue
       }
       }
+      const fileStream = fileHandle.createReadStream()
+      archive.append(fileStream, {
+        name: path.basename(file),
+      })
     }
     }
 
 
     if (missingFiles.length > 0) {
     if (missingFiles.length > 0) {

+ 12 - 7
services/clsi/test/unit/js/OutputControllerTests.js

@@ -8,10 +8,9 @@ const MODULE_PATH = require('path').join(
 describe('OutputController', function () {
 describe('OutputController', function () {
   describe('createOutputZip', function () {
   describe('createOutputZip', function () {
     beforeEach(function () {
     beforeEach(function () {
-      this.archive = {
-        on: sinon.stub(),
-        pipe: sinon.stub(),
-      }
+      this.archive = {}
+
+      this.pipeline = sinon.stub().resolves()
 
 
       this.archiveFilesForBuild = sinon.stub().resolves(this.archive)
       this.archiveFilesForBuild = sinon.stub().resolves(this.archive)
 
 
@@ -20,6 +19,9 @@ describe('OutputController', function () {
           './OutputFileArchiveManager': {
           './OutputFileArchiveManager': {
             archiveFilesForBuild: this.archiveFilesForBuild,
             archiveFilesForBuild: this.archiveFilesForBuild,
           },
           },
+          'node:stream/promises': {
+            pipeline: this.pipeline,
+          },
         },
         },
       })
       })
     })
     })
@@ -40,12 +42,15 @@ describe('OutputController', function () {
             files: ['output.tex'],
             files: ['output.tex'],
           },
           },
         }
         }
-        this.archive.pipe.callsFake(() => done())
+        this.pipeline.callsFake(() => {
+          done()
+          return Promise.resolve()
+        })
         this.OutputController.createOutputZip(this.req, this.res)
         this.OutputController.createOutputZip(this.req, this.res)
       })
       })
 
 
-      it('pipes the archive to the response', function () {
-        sinon.assert.calledWith(this.archive.pipe, this.res)
+      it('creates a pipeline from the archive to the response', function () {
+        sinon.assert.calledWith(this.pipeline, this.archive, this.res)
       })
       })
 
 
       it('calls the express convenience method to set attachment headers', function () {
       it('calls the express convenience method to set attachment headers', function () {