fix: Detect unsupported images used in static themes manifest

docs/rules.md

@@ -121,3 +121,13 @@ Rules are sorted by severity.
 | `MANIFEST_MULTIPLE_DICTS`   | error    | Multiple dictionaries found                                          |
 | `MANIFEST_EMPTY_DICTS`      | error    | Empty `dictionaries` object                                          |
 | `MANIFEST_DICT_MISSING_ID`  | error    | Missing `applications.gecko.id` property for a dictionary            |
+
+### Static Theme / manifest.json
+
+| Message code                         | Severity | Description                                                 |
+| ------------------------------------ | -------- | ----------------------------------------------------------- |
+| `MANIFEST_THEME_IMAGE_MIME_MISMATCH` | warning  | Theme image file extension should match its mime type       |
+| `MANIFEST_THEME_IMAGE_NOT_FOUND`     | error    | Theme images must not be missing                            |
+| `MANIFEST_THEME_IMAGE_CORRUPTED`     | error    | Theme images must not be corrupted                          |
+| `MANIFEST_THEME_IMAGE_WRONG_EXT`     | error    | Theme images must have one of the supported file extensions |
+| `MANIFEST_THEME_IMAGE_WRONG_MIME`    | error    | Theme images mime type must be a supported format           |

src/const.js

@@ -109,6 +109,19 @@ export const IMAGE_FILE_EXTENSIONS = [
   'svg',
 ];
 
+// Map the image mime to the expected file extensions
+// (used in the the static theme images validation).
+export const MIME_TO_FILE_EXTENSIONS = {
+  'image/svg+xml': ['svg'],
+  'image/gif': ['gif'],
+  'image/jpeg': ['jpg', 'jpeg'],
+  'image/png': ['png'],
+  'image/webp': ['webp'],
+};
+
+// List of the mime types for the allowed static theme images.
+export const STATIC_THEME_IMAGE_MIMES = Object.keys(MIME_TO_FILE_EXTENSIONS);
+
 // A list of magic numbers that we won't allow.
 export const FLAGGED_FILE_MAGIC_NUMBERS = [
   [0x4d, 0x5a], // EXE or DLL,

src/linter.js

@@ -259,6 +259,10 @@ export default class Linter {
       if (manifestParser.parsedJSON.icons) {
         await manifestParser.validateIcons();
       }
+      if (manifestParser.isStaticTheme) {
+        await manifestParser.validateStaticThemeImages();
+      }
+
       this.addonMetadata = manifestParser.getMetadata();
     } else {
       _log.warn(

src/messages/manifestjson.js

@@ -296,6 +296,84 @@ export function corruptIconFile({ path }) {
   };
 }
 
+export const MANIFEST_THEME_IMAGE_NOT_FOUND = 'MANIFEST_THEME_IMAGE_NOT_FOUND';
+export function manifestThemeImageMissing(path, type) {
+  return {
+    code: MANIFEST_THEME_IMAGE_NOT_FOUND,
+    message: i18n.sprintf(
+      'Theme image for "%(type)s" could not be found in the package',
+      { type }
+    ),
+    description: i18n.sprintf(
+      i18n._('Theme image for "%(type)s" could not be found at "%(path)s"'),
+      { path, type }
+    ),
+    file: MANIFEST_JSON,
+  };
+}
+
+export const MANIFEST_THEME_IMAGE_CORRUPTED = 'MANIFEST_THEME_IMAGE_CORRUPTED';
+export function manifestThemeImageCorrupted({ path }) {
+  return {
+    code: MANIFEST_THEME_IMAGE_CORRUPTED,
+    message: i18n._('Corrupted theme image file'),
+    description: i18n.sprintf(
+      i18n._('Theme image file at "%(path)s" is corrupted'),
+      { path }
+    ),
+    file: MANIFEST_JSON,
+  };
+}
+
+export const MANIFEST_THEME_IMAGE_WRONG_EXT = 'MANIFEST_THEME_IMAGE_WRONG_EXT';
+export function manifestThemeImageWrongExtension({ path }) {
+  return {
+    code: MANIFEST_THEME_IMAGE_WRONG_EXT,
+    message: i18n._('Theme image file has an unsupported file extension'),
+    description: i18n.sprintf(
+      i18n._(
+        'Theme image file at "%(path)s" has an unsupported file extension'
+      ),
+      { path }
+    ),
+    file: MANIFEST_JSON,
+  };
+}
+
+export const MANIFEST_THEME_IMAGE_WRONG_MIME =
+  'MANIFEST_THEME_IMAGE_WRONG_MIME';
+export function manifestThemeImageWrongMime({ path, mime }) {
+  return {
+    code: MANIFEST_THEME_IMAGE_WRONG_MIME,
+    message: i18n._('Theme image file has an unsupported mime type'),
+    description: i18n.sprintf(
+      i18n._(
+        'Theme image file at "%(path)s" has the unsupported mime type "%(mime)s"'
+      ),
+      { path, mime }
+    ),
+    file: MANIFEST_JSON,
+  };
+}
+
+export const MANIFEST_THEME_IMAGE_MIME_MISMATCH =
+  'MANIFEST_THEME_IMAGE_MIME_MISMATCH';
+export function manifestThemeImageMimeMismatch({ path, mime }) {
+  return {
+    code: MANIFEST_THEME_IMAGE_MIME_MISMATCH,
+    message: i18n._(
+      'Theme image file mime type does not match its file extension'
+    ),
+    description: i18n.sprintf(
+      i18n._(
+        'Theme image file extension at "%(path)s" does not match its actual mime type "%(mime)s"'
+      ),
+      { path, mime }
+    ),
+    file: MANIFEST_JSON,
+  };
+}
+
 export const PROP_NAME_MISSING = manifestPropMissing('name');
 export const PROP_VERSION_MISSING = manifestPropMissing('version');
 

src/parsers/manifestjson.js

@@ -21,6 +21,8 @@ import {
   IMAGE_FILE_EXTENSIONS,
   LOCALES_DIRECTORY,
   MESSAGES_JSON,
+  STATIC_THEME_IMAGE_MIMES,
+  MIME_TO_FILE_EXTENSIONS,
 } from 'const';
 import log from 'logger';
 import * as messages from 'messages';
@@ -392,6 +394,87 @@ export default class ManifestJSONParser extends JSONParser {
     return Promise.all(promises);
   }
 
+  async validateThemeImage(imagePath, manifestPropName) {
+    const _path = normalizePath(imagePath);
+    const ext = path
+      .extname(imagePath)
+      .substring(1)
+      .toLowerCase();
+
+    const fileExists = this.validateFileExistsInPackage(
+      _path,
+      `theme.images.${manifestPropName}`,
+      messages.manifestThemeImageMissing
+    );
+
+    // No need to validate the image format if the file doesn't exist
+    // on disk.
+    if (!fileExists) {
+      return;
+    }
+
+    if (!IMAGE_FILE_EXTENSIONS.includes(ext) || ext === 'webp') {
+      this.collector.addError(
+        messages.manifestThemeImageWrongExtension({ path: _path })
+      );
+      this.isValid = false;
+      return;
+    }
+
+    try {
+      const info = await getImageMetadata(this.io, _path);
+      if (
+        !STATIC_THEME_IMAGE_MIMES.includes(info.mime) ||
+        info.mime === 'image/webp'
+      ) {
+        this.collector.addError(
+          messages.manifestThemeImageWrongMime({
+            path: _path,
+            mime: info.mime,
+          })
+        );
+        this.isValid = false;
+      } else if (!MIME_TO_FILE_EXTENSIONS[info.mime].includes(ext)) {
+        this.collector.addWarning(
+          messages.manifestThemeImageMimeMismatch({
+            path: _path,
+            mime: info.mime,
+          })
+        );
+      }
+    } catch (err) {
+      log.debug(
+        `Unexpected error raised while validating theme image "${_path}"`,
+        err.message
+      );
+      this.collector.addError(
+        messages.manifestThemeImageCorrupted({ path: _path })
+      );
+      this.isValid = false;
+    }
+  }
+
+  validateStaticThemeImages() {
+    const promises = [];
+    const themeImages = this.parsedJSON.theme && this.parsedJSON.theme.images;
+
+    // The theme.images manifest property is mandatory on Firefox < 60, but optional
+    // on Firefox >= 60.
+    if (themeImages) {
+      for (const prop of Object.keys(themeImages)) {
+        if (Array.isArray(themeImages[prop])) {
+          themeImages[prop].forEach((imagePath) => {
+            promises.push(this.validateThemeImage(imagePath, prop));
+          });
+        } else {
+          promises.push(this.validateThemeImage(themeImages[prop], prop));
+        }
+      }
+    }
+
+    return Promise.all(promises);
+  }
+
   validateFileExistsInPackage(
     filePath,
     type,
@@ -401,7 +484,9 @@ export default class ManifestJSONParser extends JSONParser {
     if (!Object.prototype.hasOwnProperty.call(this.io.files, _path)) {
       this.collector.addError(messageFunc(_path, type));
       this.isValid = false;
+      return false;
     }
+    return true;
   }
 
   validateContentScriptMatchPattern(matchPattern) {

tests/setup.js

@@ -8,6 +8,11 @@ global.sinon.createStubInstance = realSinon.createStubInstance;
 global.sinon.format = realSinon.format;
 global.sinon.assert = realSinon.assert;
 
+if (process.env.APPVEYOR) {
+  // Set an higher timeout while the tests are running on appveyor CI jobs.
+  jest.setTimeout(30000);
+}
+
 // mock the cli module for every test (the ones that needs to use the real
 // module may use jest.unmock, e.g. as in test.cli.js),
 // See #1762 for a rationale.

tests/unit/helpers.js

@@ -14,12 +14,51 @@ export const fakeMessageData = {
   message: 'message',
 };
 
+export const EMPTY_SVG = Buffer.from('<svg viewbox="0 0 1 1"></svg>');
+
 export const EMPTY_PNG = Buffer.from(
   oneLine`iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMA
           AQAABQABDQottAAAAABJRU5ErkJggg==`,
   'base64'
 );
 
+export const EMPTY_GIF = Buffer.from(
+  'R0lGODlhAQABAPAAAAAAAAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==',
+  'base64'
+);
+
+export const EMPTY_APNG = Buffer.from(
+  oneLine`iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAMAAAAoyzS7AAAAA1BMVEUAAACn
+          ej3aAAAAAXRSTlMAQObYZgAAAA1JREFUCNcBAgD9/wAAAAIAAXdw4VoAAAAY
+          dEVYdFNvZnR3YXJlAGdpZjJhcG5nLnNmLm5ldJb/E8gAAAAASUVORK5CYII=`,
+  'base64'
+);
+
+export const EMPTY_JPG = Buffer.from(
+  oneLine`/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQE
+          BAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/
+          wAALCAABAAEBAREA/8QAFAABAAAAAAAAAAAAAAAAAAAACf/EABQQAQAAAAAA
+          AAAAAAAAAAAAAAD/2gAIAQEAAD8AKp//2Q==`,
+  'base64'
+);
+
+export const EMPTY_WEBP = Buffer.from(
+  oneLine`UklGRkAAAABXRUJQVlA4WAoAAAAQAAAAAAAAAAAAQUxQSAIAAAAAAFZQOCAY
+          AAAAMAEAnQEqAQABAAEAHCWkAANwAP7+BtAA`,
+  'base64'
+);
+
+export const EMPTY_TIFF = Buffer.from(
+  oneLine`SUkqABIAAAB42mNgAAAAAgABEQAAAQMAAQAAAAEAAAABAQMAAQAAAAEAAAAC
+          AQMAAgAAAAgACAADAQMAAQAAAAgAAAAGAQMAAQAAAAEAAAAKAQMAAQAAAAEA
+          AAARAQQAAQAAAAgAAAASAQMAAQAAAAEAAAAVAQMAAQAAAAIAAAAWAQMAAQAA
+          AAEAAAAXAQQAAQAAAAoAAAAcAQMAAQAAAAEAAAApAQMAAgAAAAAAAQA9AQMA
+          AQAAAAIAAAA+AQUAAgAAABQBAAA/AQUABgAAAOQAAABSAQMAAQAAAAIAAAAA
+          AAAA/wnXo/////9/4XpU///////MzEz//////5mZmf////9/ZmYm/////+8o
+          XA//////fxsNUP//////VzlU/////w==`,
+  'base64'
+);
+
 export function getRuleFiles(ruleType) {
   const ruleFiles = fs.readdirSync(`src/rules/${ruleType}`);