fix: Add an INVALID_ZIPFILE lint error on zip file with invalid chars in filenames

[rpl]

This is the addons-linter counterpart of the proposed changes to detect and collect a linting error on zip files with invalid characters in zip entries filenames.

NOTE: this can't and shouldn't be merged until mozilla/addons-scanner-utils#190 has been reviewed, merged and released on npm in a new addons-scanner-utils release.

With this and the related changes to addons-scanner-utils, the linter should report the issue as follows:

$ ./bin/addons-linter ../addons-scanner-utils/src/tests/fixtures/io/archive-with-invalid-chars-in-filenames.zip
Validation Summary:

errors          1
notices         0
warnings        0

ERRORS:

Code              Message                 Description                                    File   Line   Column
INVALID_ZIPFILE   invalid characters in   The package is invalid. It may contain
                  fileName:               entries using invalid characters, as an
                  path\to\file.txt        example using '\' as a path separator is not
                                          allowed in Firefox. Try to re-zipping your
                                          add-on package and make sure all entries are
                                          using '/' as the path separator.

[willdurand]

Some early feedback, thanks for taking care of this problem.

[willdurand]

What's the difference between invalid and bad zipfile?

[rpl]

bad zipfile seems to be associated to a zip file that is so corrupt that we are unable to open it (at least that is what I gather from the message description)

[rpl]

@willdurand I looked a bit more into what would make BAD_ZIPFILE to be reported and how the error flows from yauzl to addons-linter:

• we recognize it based on the yauzl error message we expect to receive (See ZIP_LIB_CORRUPT_FILE_ERROR here

  addons-linter/src/const.js

  Lines 220 to 221 in c5975ad

  	export const ZIP_LIB_CORRUPT_FILE_ERROR =
  	'end of central directory record signature not found';

  )

• we get an error that includes the ZIP_LIB_CORRUPT_FILE_ERROR string from yauzl's open callback parameter (unlike the other yauzl errors that we are now going to handle with the changes in this PR, which are received through 'error' events emitted on the zipfile instance, after it has been at least open successfully)

And so, from my perspective BAD_ZIPFILE, DUPLICATE_XPI_ENTRY and the new INVALID_ZIPFILE looks all like different error conditions (and seems reasonable to use separate error codes):

• BAD_ZIPFILE: for zipfiles that are corrupted and can't be inspected by the linter
• DUPLICATE_XPI_ENTRu: for zipfiles that can be opened just fine but are considered invalid because there are multiple entries with the exact same filename
• INVALID_ZIPFILE: for zipfile that can be opened just fine, but some entries use characters that are considered invalid (and Firefox would refuse to load the zip file, even if the file isn't completely corrupted from a linter perspective and the linter was able to inspect the package)

Nevertheless, INVALID_ZIPFILE doesn't look like a good rule code name, it sounds too similar to BAD_ZIPFILE, and so I think that in this PR I should change the new rule name to something that would make it easier to don't confuse differetn errors between them.

I'm going to rename the new one into INVALID_XPI_ENTRY (insted of INVALID_ZIPFILE), because I think that we can assume that the "error" events emitted by yauzl on the zipfile instance after it has been opened successfully should always (or often enough) be related to invalid entries (instead of the entire zip file to be corrupted and the linter being unable to look into its entries).

[willdurand]

Excellent, thanks!

[willdurand]

Quick update: addons-scanner-utils v5 is on its way 🚀

[willdurand]

r+, thanks

[willdurand]

I do wonder if we should rename the exception in addons-scanner-utils now :D

[rpl]

Honestly I was also tempted to propose you that, but in the end from the linter perspective the fact that the rule name doesn't exactly match the addons-scanner-utils error name is annoying but probably not "annoying enough" to justify releasing a new addons-scanner-utils version just to rename the error class.