Merge pull request #13728 from backstage/rugvip/zipfix

backend-common: fix for zip entries being skipped

.changeset/twelve-items-jump.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Fix for entries being skipped or incomplete when reading large zip archives.

packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts

@@ -16,6 +16,8 @@
 
 import fs from 'fs-extra';
 import mockFs from 'mock-fs';
+import { Readable } from 'stream';
+import { create as createArchive } from 'archiver';
 import { resolve as resolvePath } from 'path';
 import { ZipArchiveResponse } from './ZipArchiveResponse';
 
@@ -161,6 +163,48 @@ describe('ZipArchiveResponse', () => {
     ).resolves.toBe(false);
   });
 
+  it('should extract a large archive', async () => {
+    const fileCount = 10;
+    const fileSize = 1000 * 1000;
+    const filePath = await new Promise<string>((resolve, reject) => {
+      const outFile = '/large-archive.zip';
+      const archive = createArchive('zip');
+
+      archive.on('error', reject);
+      archive.on('end', () => resolve(outFile));
+      archive.pipe(fs.createWriteStream(outFile));
+      archive.on('warning', w => console.warn('WARN', w));
+
+      for (let i = 0; i < fileCount; i++) {
+        // Workaround for https://github.com/archiverjs/node-archiver/issues/542
+        // TODO(Rugvip): Without this workaround the archive entries end up with an uncompressed size of 0.
+        //               That in turn causes yauzl to hang on extraction, because the internal transform error is ignored.
+        const stream = new Readable();
+        stream.push(Buffer.alloc(fileSize, i));
+        stream.push(null);
+        archive.append(stream, { name: `file-${i}.data` });
+      }
+
+      archive.finalize();
+    });
+
+    const stream = fs.createReadStream(filePath);
+
+    const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag');
+    const dir = await res.dir({
+      targetDir: '/out',
+    });
+
+    expect(dir).toBe('/out');
+    const files = await fs.readdir(dir);
+    expect(files).toHaveLength(fileCount);
+
+    for (const file of files) {
+      const stat = await fs.stat(resolvePath(dir, file));
+      expect(stat.size).toBe(fileSize);
+    }
+  });
+
   it('should throw on invalid archive', async () => {
     const stream = fs.createReadStream('/test-archive-corrupted.zip');
 

packages/backend-common/src/reading/tree/ZipArchiveResponse.ts

@@ -124,9 +124,11 @@ export class ZipArchiveResponse implements ReadTreeResponse {
               }
 
               await callback(entry, readStream);
+              zipfile.readEntry();
             });
+          } else {
+            zipfile.readEntry();
           }
-          zipfile.readEntry();
         });
         zipfile.once('end', () => resolve());
         zipfile.on('error', e => reject(e));