fix: Make sure Xpi class detects and throws an explicit error on inva…

…lid chars in zip entries filenames. (#190)

src/errors.ts

@@ -0,0 +1,13 @@
+/* eslint-disable max-classes-per-file */
+
+export class InvalidZipFileError extends Error {
+  get name() {
+    return 'InvalidZipFileError';
+  }
+}
+
+export class DuplicateZipEntryError extends Error {
+  get name() {
+    return 'DuplicateZipFileEntry';
+  }
+}

src/index.ts

@@ -2,3 +2,4 @@ export * from './const';
 export * from './functions';
 export * from './io';
 export * from './stdio';
+export * from './errors';

src/io/xpi.spec.ts

@@ -5,6 +5,7 @@ import { EventEmitter } from 'events';
 import yauzl, { Entry, ZipFile } from 'yauzl';
 import realSinon, { SinonSandbox, SinonStub } from 'sinon';
 
+import { DuplicateZipEntryError, InvalidZipFileError } from '../errors';
 import { Xpi } from './xpi';
 import { DEFLATE_COMPRESSION, NO_COMPRESSION } from './const';
 import {
@@ -306,9 +307,11 @@ describe(__filename, () => {
         entryCallback.call(null, dupeInstallFileEntry);
       };
 
-      await expect(myXpi.getFiles(onEventsSubscribed)).rejects.toThrow(
-        'DuplicateZipEntry',
+      const promise = myXpi.getFiles(onEventsSubscribed);
+      await expect(promise).rejects.toThrow(
+        'Entry "manifest.json" has already been seen',
       );
+      await expect(promise).rejects.toThrow(DuplicateZipEntryError);
     });
 
     it('should reject on errors in open()', async () => {
@@ -325,7 +328,7 @@ describe(__filename, () => {
         filePath: 'src/tests/fixtures/io/archive-with-duplicate-files.zip',
       });
 
-      await expect(xpi.getFiles()).rejects.toThrow('DuplicateZipEntry');
+      await expect(xpi.getFiles()).rejects.toThrow(DuplicateZipEntryError);
     });
 
     it('throws an exception when a duplicate entry has been found, even when autoClose is disabled', async () => {
@@ -335,7 +338,20 @@ describe(__filename, () => {
         stderr: createFakeStderr(),
       });
 
-      await expect(xpi.getFiles()).rejects.toThrow('DuplicateZipEntry');
+      await expect(xpi.getFiles()).rejects.toThrow(DuplicateZipEntryError);
+
+      xpi.close();
+    });
+
+    it('throws a InvalidZipFileError exception on xpi files with invalid characters', async () => {
+      const xpi = new Xpi({
+        autoClose: false,
+        filePath:
+          'src/tests/fixtures/io/archive-with-invalid-chars-in-filenames.zip',
+        stderr: createFakeStderr(),
+      });
+
+      await expect(xpi.getFiles()).rejects.toThrow(InvalidZipFileError);
 
       xpi.close();
     });

src/io/xpi.ts

@@ -6,6 +6,7 @@ import stripBomStream from 'strip-bom-stream';
 import { oneLine } from 'common-tags';
 
 import { IOBaseConstructorParams, IOBase } from './base';
+import { InvalidZipFileError, DuplicateZipEntryError } from '../errors';
 
 type Files = { [filename: string]: Entry };
 
@@ -57,7 +58,17 @@ export class Xpi extends IOBase {
 
       this.zipLib.open(
         this.path,
-        { autoClose: this.autoClose },
+        {
+          autoClose: this.autoClose,
+          // Enable checks on invalid chars in zip entries filenames.
+          strictFileNames: true,
+          // Decode automatically filenames and zip entries content from buffer into strings
+          // and autodetects their encoding.
+          //
+          // NOTE: this is also mandatory because without this option set to true
+          // strictFileNames option is ignored.
+          decodeStrings: true,
+        },
         (err, zipfile) => {
           if (err) {
             return reject(err);
@@ -84,8 +95,8 @@ export class Xpi extends IOBase {
         in package`);
 
       reject(
-        new Error(oneLine`DuplicateZipEntry: Entry
-        "${entry.fileName}" has already been seen`),
+        new DuplicateZipEntryError(oneLine`Entry "${entry.fileName}" has already
+          been seen`),
       );
       return;
     }
@@ -111,6 +122,10 @@ export class Xpi extends IOBase {
     const zipfile = await this.open();
 
     return new Promise((resolve, reject) => {
+      zipfile.on('error', (err: Error) => {
+        reject(new InvalidZipFileError(err.message));
+      });
+
       zipfile.on('entry', (entry: Entry) => {
         this.handleEntry(entry, reject);
       });