Check types when extracting from JSON

docs/rules.md

@@ -348,4 +348,6 @@ TODO: A lot of these are generated so this will need expanded with each unique c
 | ----- | ------- | --------- | ---------- | ----------- | --------- | ---------- | -------- | -------- |
 | :white_check_mark: | error | Web extension | manifest_version in manifest.json is not valid. | manifest.json | | null | MANIFEST_VERSION_INVALID |
 | :white_check_mark: | error | Web extension | name property missing from manifest.json | manifest.json | | null | PROP_NAME_MISSING |
+| :white_check_mark: | error | Web extension | name property is invalid in manifest.json | manifest.json | | null | PROP_NAME_INVALID |
 | :white_check_mark: | error | Web extension | version property missing from manifest.json | manifest.json | | null | PROP_VERSION_MISSING |
+| :white_check_mark: | error | Web extension | version is invalid in manifest.json | manifest.json | | null | PROP_VERSION_INVALID |

src/messages/manifestjson.js

@@ -7,6 +7,23 @@ export const MANIFEST_VERSION_INVALID = {
   legacyCode: null,
   message: _('"manifest_version" in the manifest.json is not a valid value'),
   description: _('See http://mzl.la/20PenXl (MDN Docs) for more information.'),
+  file: MANIFEST_JSON,
+};
+
+export const PROP_NAME_INVALID = {
+  code: 'PROP_NAME_INVALID',
+  legacyCode: null,
+  message: _('The "name" property must be a string.'),
+  description: _('See http://mzl.la/1STmr48 (MDN Docs) for more information.'),
+  file: MANIFEST_JSON,
+};
+
+export const PROP_VERSION_INVALID = {
+  code: 'PROP_VERSION_INVALID',
+  legacyCode: null,
+  message: _('The "version" property must be a string.'),
+  description: _('See http://mzl.la/1kXIADa (MDN Docs) for more information.'),
+  file: MANIFEST_JSON,
 };
 
 export function manifestPropMissing(property) {

src/parsers/manifestjson.js

@@ -27,8 +27,10 @@ export default class ManifestJSONParser {
 
   _getManifestVersion() {
     // Manifest.json specific.
-    var manifestVersion = parseInt(this.parsedJSON.manifest_version, 10);
-    if (manifestVersion !== VALID_MANIFEST_VERSION) {
+    var manifestVersion = this.parsedJSON.manifest_version;
+    // manifestVersion must be a number.
+    if (typeof manifestVersion !== 'number' ||
+        manifestVersion !== VALID_MANIFEST_VERSION) {
       log.debug('Invalid manifest_version "%s"', manifestVersion);
       this.collector.addError(messages.MANIFEST_VERSION_INVALID);
       manifestVersion = null;
@@ -40,6 +42,10 @@ export default class ManifestJSONParser {
     var name = this.parsedJSON.name || null;
     if (!name) {
       this.collector.addError(messages.PROP_NAME_MISSING);
+    } else if (typeof name !== 'string') {
+      log.debug('Invalid name "%s"', name);
+      this.collector.addError(messages.PROP_NAME_INVALID);
+      name = null;
     }
     return name;
   }
@@ -48,6 +54,10 @@ export default class ManifestJSONParser {
     var version = this.parsedJSON.version || null;
     if (!version) {
       this.collector.addError(messages.PROP_VERSION_MISSING);
+    } else if (typeof version !== 'string') {
+      log.debug('Invalid version "%s"', version);
+      this.collector.addError(messages.PROP_VERSION_INVALID);
+      version = null;
     }
     return version;
   }

tests/parsers/test.manifestjson.js

@@ -21,6 +21,18 @@ describe('ManifestJSONParser._getManifestVersion()', function() {
     assert.equal(errors[0].code, messages.MANIFEST_VERSION_INVALID.code);
   });
 
+  it('should collect an error with numeric string value', () => {
+    var addonValidator = new Validator({_: ['bar']});
+    var json = validManifestJSON({manifest_version: '1'});
+    var manifestJSONParser = new ManifestJSONParser(json,
+                                                    addonValidator.collector);
+    var manifestVersion = manifestJSONParser._getManifestVersion();
+    assert.equal(manifestVersion, null);
+    var errors = addonValidator.collector.errors;
+    assert.equal(errors.length, 1);
+    assert.equal(errors[0].code, messages.MANIFEST_VERSION_INVALID.code);
+  });
+
   it('should have the right manifestVersion', () => {
     var addonValidator = new Validator({_: ['bar']});
     var json = validManifestJSON();
@@ -75,6 +87,18 @@ describe('ManifestJSONParser._getName()', function() {
     assert.equal(errors[0].code, messages.PROP_NAME_MISSING.code);
   });
 
+  it('should collect an error on non-string name value', () => {
+    var addonValidator = new Validator({_: ['bar']});
+    var json = validManifestJSON({name: 1});
+    var manifestJSONParser = new ManifestJSONParser(json,
+                                                    addonValidator.collector);
+    var name = manifestJSONParser._getName();
+    assert.equal(name, null);
+    var errors = addonValidator.collector.errors;
+    assert.equal(errors.length, 1);
+    assert.equal(errors[0].code, messages.PROP_NAME_INVALID.code);
+  });
+
 });
 
 describe('ManifestJSONParser._getVersion()', function() {
@@ -92,11 +116,23 @@ describe('ManifestJSONParser._getVersion()', function() {
     var json = validManifestJSON({version: undefined});
     var manifestJSONParser = new ManifestJSONParser(json,
                                                     addonValidator.collector);
-    var name = manifestJSONParser._getVersion();
-    assert.equal(name, null);
+    var version = manifestJSONParser._getVersion();
+    assert.equal(version, null);
     var errors = addonValidator.collector.errors;
     assert.equal(errors.length, 1);
     assert.equal(errors[0].code, messages.PROP_VERSION_MISSING.code);
   });
 
+  it('should collect an error on non-string version value', () => {
+    var addonValidator = new Validator({_: ['bar']});
+    var json = validManifestJSON({version: 1});
+    var manifestJSONParser = new ManifestJSONParser(json,
+                                                    addonValidator.collector);
+    var version = manifestJSONParser._getVersion();
+    assert.equal(version, null);
+    var errors = addonValidator.collector.errors;
+    assert.equal(errors.length, 1);
+    assert.equal(errors[0].code, messages.PROP_VERSION_INVALID.code);
+  });
+
 });