Uploading an empty zipfile throws specific error (#1909)

Added a counter for the total files extracted from a zip. If no files are extracted a specific error is thrown.

GitOrigin-RevId: 391bb669500c86e2e7fecebd90e2201248a8afd3

services/web/app/src/Features/Uploads/ArchiveManager.js

@@ -61,7 +61,7 @@ const ArchiveManager = {
             { source, totalSizeInBytes },
             'error getting bytes of zip'
           )
-          return callback(new Error('error getting bytes of zip'))
+          return callback(new Errors.InvalidError('invalid_zip_file'))
         }
         const isTooLarge = totalSizeInBytes > ONE_MEG * 300
         return callback(null, isTooLarge)
@@ -139,6 +139,8 @@ const ArchiveManager = {
       zipfile.on('error', callback)
       // read all the entries
       zipfile.readEntry()
+
+      let entryFileCount = 0
       zipfile.on('entry', function(entry) {
         logger.log(
           { source, fileName: entry.fileName },
@@ -168,6 +170,7 @@ const ArchiveManager = {
                   zipfile.close() // bail out, stop reading file entries
                   return callback(err)
                 } else {
+                  entryFileCount++
                   return zipfile.readEntry()
                 }
               }
@@ -179,7 +182,13 @@ const ArchiveManager = {
         })
       })
       // no more entries to read
-      return zipfile.on('end', callback)
+      return zipfile.on('end', () => {
+        if (entryFileCount > 0) {
+          callback()
+        } else {
+          callback(new Errors.InvalidError('empty_zip_file'))
+        }
+      })
     })
   },
 

services/web/test/acceptance/src/ProjectStructureTests.js

@@ -342,6 +342,66 @@ describe('ProjectStructureChanges', function() {
     })
   })
 
+  describe('uploading an empty zipfile', function() {
+    let zipFile = null
+
+    before(function(done) {
+      MockDocUpdaterApi.clearProjectStructureUpdates()
+      zipFile = fs.createReadStream(
+        Path.resolve(__dirname + '/../files/test_project_empty.zip')
+      )
+      done()
+    })
+
+    it('should fail with 422 error', function(done) {
+      this.owner.request.post(
+        {
+          uri: 'project/new/upload',
+          formData: {
+            qqfile: zipFile
+          }
+        },
+        (error, res) => {
+          if (error != null) {
+            throw error
+          }
+          expect(res.statusCode).to.equal(422)
+          done()
+        }
+      )
+    })
+  })
+
+  describe('uploading a zipfile containing only empty directories', function() {
+    let zipFile = null
+
+    before(function(done) {
+      MockDocUpdaterApi.clearProjectStructureUpdates()
+      zipFile = fs.createReadStream(
+        Path.resolve(__dirname + '/../files/test_project_with_empty_folder.zip')
+      )
+      done()
+    })
+
+    it('should fail with 422 error', function(done) {
+      this.owner.request.post(
+        {
+          uri: 'project/new/upload',
+          formData: {
+            qqfile: zipFile
+          }
+        },
+        (error, res) => {
+          if (error != null) {
+            throw error
+          }
+          expect(res.statusCode).to.equal(422)
+          done()
+        }
+      )
+    })
+  })
+
   describe('uploading a project with a shared top-level folder', function() {
     before(function(done) {
       MockDocUpdaterApi.clearProjectStructureUpdates()

services/web/test/unit/src/Uploads/ArchiveManagerTests.js

@@ -70,11 +70,23 @@ describe('ArchiveManager', function() {
 
     describe('successfully', function() {
       beforeEach(function(done) {
+        this.readStream = new events.EventEmitter()
+        this.readStream.pipe = sinon.stub()
+        this.zipfile.openReadStream = sinon
+          .stub()
+          .callsArgWith(1, null, this.readStream)
+        this.writeStream = new events.EventEmitter()
+        this.fs.createWriteStream = sinon.stub().returns(this.writeStream)
+        this.fse.ensureDir = sinon.stub().callsArg(1)
         this.ArchiveManager.extractZipArchive(
           this.source,
           this.destination,
           done
         )
+
+        // entry contains a single file
+        this.zipfile.emit('entry', { fileName: 'testfile.txt' })
+        this.readStream.emit('end')
         return this.zipfile.emit('end')
       })
 
@@ -93,6 +105,60 @@ describe('ArchiveManager', function() {
       })
     })
 
+    describe('with a zipfile containing an empty directory', function() {
+      beforeEach(function(done) {
+        this.readStream = new events.EventEmitter()
+        this.readStream.pipe = sinon.stub()
+        this.zipfile.openReadStream = sinon
+          .stub()
+          .callsArgWith(1, null, this.readStream)
+        this.writeStream = new events.EventEmitter()
+        this.fs.createWriteStream = sinon.stub().returns(this.writeStream)
+        this.fse.ensureDir = sinon.stub().callsArg(1)
+        this.ArchiveManager.extractZipArchive(
+          this.source,
+          this.destination,
+          error => {
+            this.callback(error)
+            done()
+          }
+        )
+
+        // entry contains a single, empty directory
+        this.zipfile.emit('entry', { fileName: 'testdir/' })
+        this.readStream.emit('end')
+        return this.zipfile.emit('end')
+      })
+
+      it('should return the callback with an error', function() {
+        return sinon.assert.calledWithExactly(
+          this.callback,
+          new Errors.InvalidError('empty_zip_file')
+        )
+      })
+    })
+
+    describe('with an empty zipfile', function() {
+      beforeEach(function(done) {
+        this.ArchiveManager.extractZipArchive(
+          this.source,
+          this.destination,
+          error => {
+            this.callback(error)
+            return done()
+          }
+        )
+        return this.zipfile.emit('end')
+      })
+
+      it('should return the callback with an error', function() {
+        return sinon.assert.calledWithExactly(
+          this.callback,
+          new Errors.InvalidError('empty_zip_file')
+        )
+      })
+    })
+
     describe('with an error in the zip file header', function() {
       beforeEach(function(done) {
         this.yauzl.open = sinon
@@ -286,10 +352,6 @@ describe('ArchiveManager', function() {
       it('should not try to read the entry', function() {
         return this.zipfile.openReadStream.called.should.equal(false)
       })
-
-      it('should not log out a warning', function() {
-        return this.logger.warn.called.should.equal(false)
-      })
     })
 
     describe('with an error opening the file read stream', function() {