From a004a9792f8cf9e87da019b93379efe37e29f2ff Mon Sep 17 00:00:00 2001 From: Stuart Colville Date: Mon, 30 Nov 2015 12:31:52 +0000 Subject: [PATCH] Check types when extracting from JSON --- docs/rules.md | 2 ++ src/messages/manifestjson.js | 17 +++++++++++++ src/parsers/manifestjson.js | 14 +++++++++-- tests/parsers/test.manifestjson.js | 40 ++++++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/docs/rules.md b/docs/rules.md index ea9ccb619bf..c2870d042fc 100644 --- a/docs/rules.md +++ b/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 | diff --git a/src/messages/manifestjson.js b/src/messages/manifestjson.js index 667b780bdfd..0e425518537 100644 --- a/src/messages/manifestjson.js +++ b/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) { diff --git a/src/parsers/manifestjson.js b/src/parsers/manifestjson.js index 54cf29e46cd..abdce1e354b 100644 --- a/src/parsers/manifestjson.js +++ b/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; } diff --git a/tests/parsers/test.manifestjson.js b/tests/parsers/test.manifestjson.js index eddf4564a97..7e1eeca1828 100644 --- a/tests/parsers/test.manifestjson.js +++ b/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); + }); + });