From e27ada575f0bae003b31035731948bd040321e9d Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Jul 2022 13:02:49 +0200 Subject: [PATCH] chore: small re-factor, and update changeset Signed-off-by: blam --- .changeset/nine-mails-crash.md | 2 +- packages/backend-common/package.json | 4 +- .../src/reading/tree/ZipArchiveResponse.ts | 104 ++++++++---------- 3 files changed, 50 insertions(+), 60 deletions(-) diff --git a/.changeset/nine-mails-crash.md b/.changeset/nine-mails-crash.md index 3dc4334f17..ecb047802e 100644 --- a/.changeset/nine-mails-crash.md +++ b/.changeset/nine-mails-crash.md @@ -6,4 +6,4 @@ The `ZipArchiveResponse` now correctly handles corrupt ZIP archives. Before this change, certain corrupt ZIP archives either cause the inflater to throw (as expected), or will hang the parser indefinitely. -This change introduces a default timeout of 3000ms before throwing an error message when trying to unzip an archive. +By switching out the `zip` parsing library, we now write to a temporary directory, and load from disk which should ensure that the parsing of the `.zip` files are done correctly because `streaming` of `zip` paths is technically impossible without being able to parse the headers at the end of he file. diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 40beb3deca..3cc689f32a 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -49,7 +49,6 @@ "@types/express": "^4.17.6", "@types/luxon": "^2.0.4", "@types/webpack-env": "^1.15.2", - "@types/yauzl": "^2.10.0", "archiver": "^5.0.2", "aws-sdk": "^2.840.0", "base64-stream": "^1.0.0", @@ -79,7 +78,6 @@ "selfsigned": "^2.0.0", "stoppable": "^1.1.0", "tar": "^6.1.2", - "unzipper": "^0.10.11", "winston": "^3.2.1", "yauzl": "^2.10.0", "yn": "^4.0.0" @@ -108,7 +106,7 @@ "@types/stoppable": "^1.1.0", "@types/supertest": "^2.0.8", "@types/tar": "^6.1.1", - "@types/unzipper": "^0.10.3", + "@types/yauzl": "^2.10.0", "@types/webpack-env": "^1.15.2", "aws-sdk-mock": "^5.2.1", "better-sqlite3": "^7.5.0", diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index b40780d125..15bc0d0258 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -80,6 +80,54 @@ export class ZipArchiveResponse implements ReadTreeResponse { return true; } + private async streamToTemporaryFile(stream: Readable): Promise { + const tmpDir = await fs.mkdtemp( + platformPath.join(this.workDir, 'backstage-tmp'), + ); + const tmpFile = platformPath.join(tmpDir, 'tmp.zip'); + + const writeStream = fs.createWriteStream(tmpFile); + + return new Promise((resolve, reject) => { + writeStream.on('error', reject); + writeStream.on('finish', () => resolve(tmpFile)); + stream.pipe(writeStream); + }); + } + + private forEveryZipEntry( + zip: string, + callback: (entry: Entry, content: Readable) => Promise, + ): Promise { + return new Promise((resolve, reject) => { + yauzl.open(zip, { lazyEntries: true }, (err, zipfile) => { + if (err || !zipfile) { + reject(err || new Error(`Failed to open zip file ${zip}`)); + return; + } + + zipfile.on('entry', async (entry: Entry) => { + if (!/\/$/.test(entry.fileName) && this.shouldBeIncluded(entry)) { + zipfile.openReadStream(entry, async (openErr, readStream) => { + if (openErr || !readStream) { + reject( + openErr || + new Error(`Failed to open zip entry ${entry.fileName}`), + ); + return; + } + + await callback(entry, readStream); + }); + } + zipfile.readEntry(); + }); + zipfile.once('end', () => resolve()); + zipfile.readEntry(); + }); + }); + } + async files(): Promise { this.onlyOnce(); const files = Array(); @@ -116,62 +164,6 @@ export class ZipArchiveResponse implements ReadTreeResponse { return archive; } - private async streamToTemporaryFile(stream: Readable): Promise { - const tmpDir = await fs.mkdtemp( - platformPath.join(this.workDir, 'backstage-tmp'), - ); - const tmpFile = platformPath.join(tmpDir, 'tmp.zip'); - - const writeStream = fs.createWriteStream(tmpFile); - - return new Promise((resolve, reject) => { - writeStream.on('error', reject); - writeStream.on('finish', () => resolve(tmpFile)); - stream.pipe(writeStream); - }); - } - - private forEveryZipEntry( - zip: string, - callback: (entry: Entry, content: Readable) => Promise, - ): Promise { - return new Promise((resolve, reject) => { - yauzl.open(zip, { lazyEntries: true }, (err, zipfile) => { - if (err) { - reject(err); - return; - } - - if (!zipfile) { - reject(new Error('Zip file is empty')); - return; - } - - zipfile.on('entry', async (entry: Entry) => { - // If it's not a directory, and it's included, then grab the contents of the file from the buffer - if (!/\/$/.test(entry.fileName) && this.shouldBeIncluded(entry)) { - zipfile.openReadStream(entry, async (openErr, readStream) => { - if (openErr) { - reject(openErr); - return; - } - - if (!readStream) { - reject(new Error('Zip file is empty')); - return; - } - - await callback(entry, readStream); - }); - } - zipfile.readEntry(); - }); - zipfile.once('end', () => resolve()); - zipfile.readEntry(); - }); - }); - } - async dir(options?: ReadTreeResponseDirOptions): Promise { this.onlyOnce(); const dir =