feat: Move to yauzl to fix FD error (#264)

seebees · web-flow · commit 1dd5a864fb7a · 2020-02-27T15:26:45.000-08:00
`unzipper` would throw an FD error occasionally. Relevant issue: ZJONSSON/node-unzipper#104 There is a pending PR, but it seemed simpler to simply move to `yauzl`. The error: ``` events.js:174 throw er; // Unhandled 'error' event ^ Error: EBADF: bad file descriptor, read Emitted 'error' event at: at lazyFs.read (internal/fs/streams.js:165:12) at FSReqWrap.wrapper [as oncomplete] (fs.js:467:17) ```
diff --git a/modules/integration-browser/package.json b/modules/integration-browser/package.json
@@ -22,7 +22,7 @@
     "@trust/keyto": "^0.3.7",
     "@types/got": "^9.6.2",
     "@types/stream-to-promise": "^2.2.0",
-    "@types/unzipper": "^0.10.2",
+    "@types/yauzl": "^2.9.1",
     "@types/yargs": "^15.0.3",
     "got": "^10.6.0",
     "jasmine-core": "^3.4.0",
@@ -34,7 +34,7 @@
     "puppeteer": "^1.14.0",
     "stream-to-promise": "^2.2.0",
     "tslib": "^1.9.3",
-    "unzipper": "^0.9.11",
+    "yauzl": "^2.10.0",
     "webpack": "^4.30.0",
     "yargs": "^13.2.2"
   },
diff --git a/modules/integration-browser/src/build_decrypt_fixtures.ts b/modules/integration-browser/src/build_decrypt_fixtures.ts
@@ -14,9 +14,14 @@
  * limitations under the License.
  */
 
-import { Open } from 'unzipper'
+import {
+  open,
+  Entry, // eslint-disable-line no-unused-vars
+  ZipFile // eslint-disable-line no-unused-vars
+} from 'yauzl'
 import streamToPromise from 'stream-to-promise'
 import { writeFileSync } from 'fs'
+import { Readable } from 'stream' // eslint-disable-line no-unused-vars
 
 import { DecryptManifestList } from './types' // eslint-disable-line no-unused-vars
 
@@ -30,23 +35,22 @@ import { DecryptManifestList } from './types' // eslint-disable-line no-unused-v
 export async function buildDecryptFixtures (fixtures: string, vectorFile: string, testName?: string, slice?: string) {
   const [start = 0, end = 9999] = (slice || '').split(':').map(n => parseInt(n, 10))
 
-  const centralDirectory = await Open.file(vectorFile)
-  const filesMap = new Map(centralDirectory.files.map(file => [file.path, file]))
+  const filesMap = await centralDirectory(vectorFile)
 
   const readUriOnce = (() => {
     const cache = new Map()
     return async (uri: string) => {
       const has = cache.get(uri)
       if (has) return has
-      const fileInfo = filesMap.get(testUri2Path(uri))
+      const fileInfo = filesMap.get(uri)
       if (!fileInfo) throw new Error(`${uri} does not exist`)
-      const buffer = await fileInfo.buffer()
+      const buffer = await streamToPromise(await fileInfo.stream())
       cache.set(uri, buffer)
       return buffer
     }
   })()
 
-  const manifestBuffer = await readUriOnce('manifest.json')
+  const manifestBuffer = await readUriOnce('file://manifest.json')
   const { keys: keysFile, tests }: DecryptManifestList = JSON.parse(manifestBuffer.toString('utf8'))
   const keysBuffer = await readUriOnce(keysFile)
   const { keys } = JSON.parse(keysBuffer.toString('utf8'))
@@ -68,12 +72,12 @@ export async function buildDecryptFixtures (fixtures: string, vectorFile: string
     testNames.push(name)
 
     const { plaintext: plaintextFile, ciphertext, 'master-keys': masterKeys } = testInfo
-    const plainTextInfo = filesMap.get(testUri2Path(plaintextFile))
-    const cipherInfo = filesMap.get(testUri2Path(ciphertext))
+    const plainTextInfo = filesMap.get(plaintextFile)
+    const cipherInfo = filesMap.get(ciphertext)
     if (!cipherInfo || !plainTextInfo) throw new Error(`no file for ${name}: ${ciphertext} | ${plaintextFile}`)
 
-    const cipherText = await streamToPromise(<NodeJS.ReadableStream>cipherInfo.stream())
-    const plainText = await readUriOnce(plainTextInfo.path)
+    const cipherText = await streamToPromise(await cipherInfo.stream())
+    const plainText = await readUriOnce(`file://${plainTextInfo.fileName}`)
     const keysInfo = masterKeys.map(keyInfo => {
       const key = keys[keyInfo.key]
       if (!key) throw new Error(`no key for ${name}`)
@@ -83,7 +87,7 @@ export async function buildDecryptFixtures (fixtures: string, vectorFile: string
     const test = JSON.stringify({
       name,
       keysInfo,
-      cipherFile: cipherInfo.path,
+      cipherFile: cipherInfo.fileName,
       cipherText: cipherText.toString('base64'),
       plainText: plainText.toString('base64')
     })
@@ -94,6 +98,38 @@ export async function buildDecryptFixtures (fixtures: string, vectorFile: string
   writeFileSync(`${fixtures}/decrypt_tests.json`, JSON.stringify(testNames))
 }
 
-function testUri2Path (uri: string) {
-  return uri.replace('file://', '')
+interface StreamEntry extends Entry {
+  stream: () => Promise<Readable>
+}
+
+function centralDirectory (vectorFile: string): Promise<Map<string, StreamEntry>> {
+  const filesMap = new Map<string, StreamEntry>()
+  return new Promise((resolve, reject) => {
+    open(vectorFile, { lazyEntries: true, autoClose: false }, (err, zipfile) => {
+      if (err || !zipfile) return reject(err)
+
+      zipfile
+        .on('entry', (entry: StreamEntry) => {
+          entry.stream = curryStream(zipfile, entry)
+          filesMap.set(`file://${entry.fileName}`, entry)
+          zipfile.readEntry()
+        })
+        .on('end', () => {
+          resolve(filesMap)
+        })
+        .on('error', (err) => reject(err))
+        .readEntry()
+    })
+  })
+}
+
+function curryStream (zipfile: ZipFile, entry: Entry) {
+  return function stream (): Promise<Readable> {
+    return new Promise((resolve, reject) => {
+      zipfile.openReadStream(entry, (err, readStream) => {
+        if (err || !readStream) return reject(err)
+        resolve(readStream)
+      })
+    })
+  }
 }
diff --git a/modules/integration-node/package.json b/modules/integration-node/package.json
@@ -15,12 +15,14 @@
   "license": "Apache-2.0",
   "dependencies": {
     "@aws-crypto/client-node": "file:../client-node",
-    "@types/got": "^9.6.2",
-    "@types/unzipper": "^0.10.2",
+    "@types/got": "^9.6.9",
+    "@types/yauzl": "^2.9.1",
     "@types/yargs": "^15.0.3",
+    "@types/stream-to-promise": "^2.2.0",
     "got": "^10.6.0",
     "tslib": "^1.9.3",
-    "unzipper": "^0.9.11",
+    "yauzl": "^2.10.0",
+    "stream-to-promise": "^2.2.0",
     "yargs": "^13.2.2"
   },
   "sideEffects": false,
diff --git a/modules/integration-node/src/get_decrypt_test_iterator.ts b/modules/integration-node/src/get_decrypt_test_iterator.ts
@@ -14,52 +14,54 @@
  */
 
 import {
-  Open,
-  File // eslint-disable-line no-unused-vars
-} from 'unzipper'
+  open,
+  Entry, // eslint-disable-line no-unused-vars
+  ZipFile // eslint-disable-line no-unused-vars
+} from 'yauzl'
 import {
   DecryptManifestList, // eslint-disable-line no-unused-vars
   KeyList, // eslint-disable-line no-unused-vars
   KeyInfoTuple // eslint-disable-line no-unused-vars
 } from './types'
 import { Readable } from 'stream' // eslint-disable-line no-unused-vars
+import streamToPromise from 'stream-to-promise'
 
 export async function getDecryptTestVectorIterator (vectorFile: string) {
-  const centralDirectory = await Open.file(vectorFile)
-  // @ts-ignore
-  const filesMap = new Map(centralDirectory.files.map(file => [file.path, file]))
+  const filesMap = await centralDirectory(vectorFile)
 
   return _getDecryptTestVectorIterator(filesMap)
 }
 
 /* Just a simple more testable function */
-export async function _getDecryptTestVectorIterator (filesMap: Map<string, File>) {
+export async function _getDecryptTestVectorIterator (filesMap: Map<string, StreamEntry>) {
   const readUriOnce = (() => {
     const cache: Map<string, Buffer> = new Map()
-    return async (uri: string) => {
+    return async (uri: string): Promise<Buffer> => {
       const has = cache.get(uri)
       if (has) return has
-      const fileInfo = filesMap.get(testUri2Path(uri))
+      const fileInfo = filesMap.get(uri)
       if (!fileInfo) throw new Error(`${uri} does not exist`)
-      const buffer = await fileInfo.buffer()
+      const stream = await fileInfo.stream()
+
+      const buffer = await streamToPromise(stream)
       cache.set(uri, buffer)
       return buffer
     }
   })()
 
-  const manifestBuffer = await readUriOnce('manifest.json')
+  const manifestBuffer = await readUriOnce('file://manifest.json')
   const { keys: keysFile, tests }: DecryptManifestList = JSON.parse(manifestBuffer.toString('utf8'))
   const keysBuffer = await readUriOnce(keysFile)
   const { keys }: KeyList = JSON.parse(keysBuffer.toString('utf8'))
 
   return (function * nextTest (): IterableIterator<TestVectorInfo> {
     for (const [name, testInfo] of Object.entries(tests)) {
       const { plaintext: plaintextFile, ciphertext, 'master-keys': masterKeys } = testInfo
-      const plainTextInfo = filesMap.get(testUri2Path(plaintextFile))
-      const cipherInfo = filesMap.get(testUri2Path(ciphertext))
-      if (!cipherInfo || !plainTextInfo) throw new Error(`no file for ${name}: ${testUri2Path(ciphertext)} | ${testUri2Path(plaintextFile)}`)
-      const cipherStream = cipherInfo.stream()
-      const plainTextStream = plainTextInfo.stream()
+      const plainTextInfo = filesMap.get(plaintextFile)
+      const cipherInfo = filesMap.get(ciphertext)
+      if (!cipherInfo || !plainTextInfo) throw new Error(`no file for ${name}: ${ciphertext} | ${plaintextFile}`)
+      const cipherStream = cipherInfo.stream
+      const plainTextStream = plainTextInfo.stream
       const keysInfo = <KeyInfoTuple[]>masterKeys.map(keyInfo => {
         const key = keys[keyInfo.key]
         if (!key) throw new Error(`no key for ${name}`)
@@ -76,13 +78,45 @@ export async function _getDecryptTestVectorIterator (filesMap: Map<string, File>
   })()
 }
 
-function testUri2Path (uri: string) {
-  return uri.replace('file://', '')
-}
-
 export interface TestVectorInfo {
   name: string,
   keysInfo: KeyInfoTuple[],
-  cipherStream: Readable
-  plainTextStream: Readable
+  cipherStream: () => Promise<Readable>
+  plainTextStream: () => Promise<Readable>
+}
+
+interface StreamEntry extends Entry {
+  stream: () => Promise<Readable>
+}
+
+function centralDirectory (vectorFile: string): Promise<Map<string, StreamEntry>> {
+  const filesMap = new Map<string, StreamEntry>()
+  return new Promise((resolve, reject) => {
+    open(vectorFile, { lazyEntries: true, autoClose: false }, (err, zipfile) => {
+      if (err || !zipfile) return reject(err)
+
+      zipfile
+        .on('entry', (entry: StreamEntry) => {
+          entry.stream = curryStream(zipfile, entry)
+          filesMap.set('file://' + entry.fileName, entry)
+          zipfile.readEntry()
+        })
+        .on('end', () => {
+          resolve(filesMap)
+        })
+        .on('error', (err) => reject(err))
+        .readEntry()
+    })
+  })
+}
+
+function curryStream (zipfile: ZipFile, entry: Entry) {
+  return function stream (): Promise<Readable> {
+    return new Promise((resolve, reject) => {
+      zipfile.openReadStream(entry, (err, readStream) => {
+        if (err || !readStream) return reject(err)
+        resolve(readStream)
+      })
+    })
+  }
 }
diff --git a/modules/integration-node/src/integration_tests.ts b/modules/integration-node/src/integration_tests.ts
@@ -25,6 +25,7 @@ import { decryptMaterialsManagerNode, encryptMaterialsManagerNode } from './decr
 import { decrypt, encrypt, needs } from '@aws-crypto/client-node'
 import { URL } from 'url'
 import got from 'got'
+import streamToPromise from 'stream-to-promise'
 
 const notSupportedDecryptMessages = [
   'Not supported at this time.'
@@ -39,18 +40,17 @@ const notSupportedEncryptMessages = [
 export async function testDecryptVector ({ name, keysInfo, plainTextStream, cipherStream }: TestVectorInfo): Promise<TestVectorResults> {
   try {
     const cmm = decryptMaterialsManagerNode(keysInfo)
-    const knowGood: Buffer[] = []
-    plainTextStream.on('data', (chunk: Buffer) => knowGood.push(chunk))
-    const { plaintext } = await decrypt(cmm, cipherStream)
-    const result = Buffer.concat(knowGood).equals(plaintext)
+    const knowGood = await streamToPromise(await plainTextStream())
+    const { plaintext } = await decrypt(cmm, await cipherStream())
+    const result = knowGood.equals(plaintext)
     return { result, name }
   } catch (err) {
     return { result: false, name, err }
   }
 }
 
 // This is only viable for small streams, if we start get get larger streams, an stream equality should get written
-export async function testEncryptVector ({ name, keysInfo, encryptOp, plainTextData }: EncryptTestVectorInfo, decryptOracle: URL): Promise<TestVectorResults> {
+export async function testEncryptVector ({ name, keysInfo, encryptOp, plainTextData }: EncryptTestVectorInfo, decryptOracle: string): Promise<TestVectorResults> {
   try {
     const cmm = encryptMaterialsManagerNode(keysInfo)
     const { result: encryptResult } = await encrypt(cmm, plainTextData, encryptOp)
@@ -61,7 +61,7 @@ export async function testEncryptVector ({ name, keysInfo, encryptOp, plainTextD
         'Accept': 'application/octet-stream'
       },
       body: encryptResult,
-      encoding: null
+      responseType: 'buffer'
     })
     needs(decryptResponse.statusCode === 200, 'decrypt failure')
     const { body } = decryptResponse
@@ -97,7 +97,7 @@ export async function integrationDecryptTestVectors (vectorFile: string, tolerat
 }
 
 export async function integrationEncryptTestVectors (manifestFile: string, keyFile: string, decryptOracle: string, tolerateFailures: number = 0, testName?: string, concurrency: number = 1) {
-  const decryptOracleUrl = new URL(decryptOracle)
+  const decryptOracleUrl = new URL(decryptOracle).toString()
   const tests = await getEncryptTestVectorIterator(manifestFile, keyFile)
 
   return parallelTests(concurrency, tolerateFailures, runTest, tests)
diff --git a/modules/integration-node/test/get_decrypt_test_iterator.test.ts b/modules/integration-node/test/get_decrypt_test_iterator.test.ts
@@ -21,6 +21,7 @@ import {
   _getDecryptTestVectorIterator
 } from '../src/index'
 import { DecryptManifestList, KeyList } from '../src/types' // eslint-disable-line no-unused-vars
+import { PassThrough } from 'stream'
 
 const keyList: KeyList = {
   'manifest': {
@@ -63,29 +64,33 @@ const manifest:DecryptManifestList = {
 
 const filesMap = new Map([
   [
-    'manifest.json', {
-      async buffer () {
-        return Buffer.from(JSON.stringify(manifest))
+    'file://manifest.json', {
+      async stream () {
+        const stream = new PassThrough()
+        setImmediate(() => stream.end(Buffer.from(JSON.stringify(manifest))))
+        return stream
       }
     } as any
   ],
   [
-    'keys.json', {
-      async buffer () {
-        return Buffer.from(JSON.stringify(keyList))
+    'file://keys.json', {
+      async stream () {
+        const stream = new PassThrough()
+        setImmediate(() => stream.end(Buffer.from(JSON.stringify(keyList))))
+        return stream
       }
     } as any
   ],
   [
-    'ciphertexts/460bd892-c137-4178-8201-4ab5ee5d3041', {
-      stream () {
+    'file://ciphertexts/460bd892-c137-4178-8201-4ab5ee5d3041', {
+      async stream () {
         return {} as any
       }
     } as any
   ],
   [
-    'plaintexts/small', {
-      stream () {
+    'file://plaintexts/small', {
+      async stream () {
         return {} as any
       }
     } as any
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
