From 669112025c5d0d5fea3261bee9d80d00165f51cc Mon Sep 17 00:00:00 2001 From: teppeis Date: Thu, 6 Apr 2017 02:37:11 +0900 Subject: [PATCH 1/4] feat: implement `format:url` --- index.js | 6 +++ manifest-schema.json | 12 +++-- package.json | 3 +- src/validate-https-url.js | 20 ++++++++ test/fixtures/invalid-http-url.json | 14 ++++++ test/fixtures/invalid-relative-url.json | 12 +++++ test/index.js | 15 ++++++ test/validate-https-url.js | 61 +++++++++++++++++++++++++ yarn.lock | 26 +++++++++++ 9 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 src/validate-https-url.js create mode 100644 test/fixtures/invalid-http-url.json create mode 100644 test/fixtures/invalid-relative-url.json create mode 100644 test/validate-https-url.js diff --git a/index.js b/index.js index 4dde53d..0afc36a 100644 --- a/index.js +++ b/index.js @@ -2,6 +2,7 @@ const Ajv = require('ajv'); const jsonSchema = require('./manifest-schema.json'); +const validateUrl = require('./src/validate-https-url'); /** * @param {Object} json @@ -12,6 +13,11 @@ module.exports = function(json) { allErrors: true, unknownFormats: true, errorDataPath: 'property', + formats: { + url: str => validateUrl(str, true), + 'https-url': str => validateUrl(str), + 'relative-path': str => true, + }, }); const validate = ajv.compile(jsonSchema); const valid = validate(json); diff --git a/manifest-schema.json b/manifest-schema.json index 9eb867b..1f0a5da 100644 --- a/manifest-schema.json +++ b/manifest-schema.json @@ -73,17 +73,17 @@ "ja": { "type": "string", "minLength": 1, - "format": "uri" + "format": "url" }, "en": { "type": "string", "minLength": 1, - "format": "uri" + "format": "url" }, "zh": { "type": "string", "minLength": 1, - "format": "uri" + "format": "url" } } }, @@ -96,7 +96,11 @@ "items": { "description": "internal or external", "type": "string", - "format": "uri", + "anyOf": [{ + "format": "https-url" + }, { + "format": "relative-path" + }], "maxItems": 30 } }, diff --git a/package.json b/package.json index f7d5d07..474cb10 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,8 @@ "test": "run-s lint mocha" }, "dependencies": { - "ajv": "^4.11.5" + "ajv": "^4.11.5", + "isomorphic-url": "^1.0.0-alpha12" }, "devDependencies": { "eslint": "^3.19.0", diff --git a/src/validate-https-url.js b/src/validate-https-url.js new file mode 100644 index 0000000..cc9b283 --- /dev/null +++ b/src/validate-https-url.js @@ -0,0 +1,20 @@ +'use strict'; + +const URL = require('isomorphic-url').URL; + +/** + * @param {string} str + * @param {boolean=} opt_allowHttp + * @return {boolean} + */ +function validateHttpsUrl(str, opt_allowHttp) { + const allowHttp = !!opt_allowHttp; + try { + const url = new URL(str); + return url.protocol === 'https:' || (allowHttp && url.protocol === 'http:'); + } catch (e) { + return false; + } +} + +module.exports = validateHttpsUrl; diff --git a/test/fixtures/invalid-http-url.json b/test/fixtures/invalid-http-url.json new file mode 100644 index 0000000..7792a87 --- /dev/null +++ b/test/fixtures/invalid-http-url.json @@ -0,0 +1,14 @@ +{ + "manifest_version": 1, + "version": 1, + "type": "APP", + "name": { + "en": "sample plugin" + }, + "icon": "image/icon.png", + "desktop": { + "js": [ + "http://example.com/icon.png" + ] + } +} diff --git a/test/fixtures/invalid-relative-url.json b/test/fixtures/invalid-relative-url.json new file mode 100644 index 0000000..113366d --- /dev/null +++ b/test/fixtures/invalid-relative-url.json @@ -0,0 +1,12 @@ +{ + "manifest_version": 1, + "version": 1, + "type": "APP", + "name": { + "en": "sample plugin" + }, + "icon": "image/icon.png", + "homepage_url": { + "en": "image/icon.png" + } +} diff --git a/test/index.js b/test/index.js index 964ccb7..61c2621 100644 --- a/test/index.js +++ b/test/index.js @@ -98,4 +98,19 @@ describe('validator', () => { assert(actual.valid === false); assert(actual.errors.length === 2); }); + + it('invalid relative url', () => { + const actual = validator(require('./fixtures/invalid-relative-url.json')); + assert(actual.valid === false); + assert(actual.errors.length === 1); + assert(actual.errors[0].params.format === 'url'); + }); + + // enable after implement relative-path + it.skip('invalid http url', () => { + const actual = validator(require('./fixtures/invalid-http-url.json')); + assert(actual.valid === false); + console.log(actual.errors); + assert(actual.errors.length === 1); + }); }); diff --git a/test/validate-https-url.js b/test/validate-https-url.js new file mode 100644 index 0000000..c303de0 --- /dev/null +++ b/test/validate-https-url.js @@ -0,0 +1,61 @@ +'use strict'; + +const assert = require('assert'); +const validate = require('../src/validate-https-url'); + +describe('validate-https-url', () => { + context('valid', () => { + [ + 'https://example.com/path/to?foo=bar&baz=piyo#hash', + 'https://user:pass@example.com/', + ].forEach(url => { + it(url, () => { + assert(validate(url)); + }); + }); + }); + + context('invalid', () => { + [ + 'http://example.com', + 'ftp://example.com', + '://example.com', + '//example.com', + '/path/to/foo', + './path/to/foo', + 'path/to/foo', + ].forEach(url => { + it(url, () => { + assert(!validate(url)); + }); + }); + }); + + context('`allowHttp`', () => { + context('valid', () => { + [ + 'https://example.com', + 'http://example.com', + ].forEach(url => { + it(url, () => { + assert(validate(url, true)); + }); + }); + }); + + context('invalid', () => { + [ + 'ftp://example.com', + '://example.com', + '//example.com', + '/path/to/foo', + './path/to/foo', + 'path/to/foo', + ].forEach(url => { + it(url, () => { + assert(!validate(url)); + }); + }); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 67da565..86474e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -696,6 +696,10 @@ has@^1.0.1: dependencies: function-bind "^1.0.2" +hasurl@^1.0.0-alpha: + version "1.0.0-alpha" + resolved "https://registry.yarnpkg.com/hasurl/-/hasurl-1.0.0-alpha.tgz#59e74e2d5ea91625421b3466dc74c9ba8d9f6d55" + hosted-git-info@^2.1.4: version "2.4.1" resolved "https://registry.yarnpkg.com/hosted-git-info/-/hosted-git-info-2.4.1.tgz#4b0445e41c004a8bd1337773a4ff790ca40318c8" @@ -836,6 +840,13 @@ isexe@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10" +isomorphic-url@^1.0.0-alpha12: + version "1.0.0-alpha12" + resolved "https://registry.yarnpkg.com/isomorphic-url/-/isomorphic-url-1.0.0-alpha12.tgz#eadba13966a5e0b53f9f29b404b77464a0dfda37" + dependencies: + hasurl "^1.0.0-alpha" + whatwg-url "^4.5.1" + js-tokens@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-3.0.1.tgz#08e9f132484a2c45a30907e9dc4d5567b7f114d7" @@ -1460,6 +1471,10 @@ through@2, through@^2.3.6, through@~2.3, through@~2.3.1: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" +tr46@~0.0.3: + version "0.0.3" + resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" + traverse@^0.6.6: version "0.6.6" resolved "https://registry.yarnpkg.com/traverse/-/traverse-0.6.6.tgz#cbdf560fd7b9af632502fed40f918c157ea97137" @@ -1507,6 +1522,17 @@ validate-npm-package-license@^3.0.1: spdx-correct "~1.0.0" spdx-expression-parse "~1.0.0" +webidl-conversions@^3.0.0: + version "3.0.1" + resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-3.0.1.tgz#24534275e2a7bc6be7bc86611cc16ae0a5654871" + +whatwg-url@^4.5.1: + version "4.7.0" + resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-4.7.0.tgz#202035ac1955b087cdd20fa8b58ded3ab1cd2af5" + dependencies: + tr46 "~0.0.3" + webidl-conversions "^3.0.0" + which@^1.2.9: version "1.2.14" resolved "https://registry.yarnpkg.com/which/-/which-1.2.14.tgz#9a87c4378f03e827cecaf1acdf56c736c01c14e5" From 5c22bba3dcd66f98e0a9a9fefea5f26155781498 Mon Sep 17 00:00:00 2001 From: teppeis Date: Thu, 6 Apr 2017 03:32:18 +0900 Subject: [PATCH 2/4] feat: implement `format:relative-path` --- index.js | 10 ++++++++-- test/index.js | 13 ++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 0afc36a..28aec28 100644 --- a/index.js +++ b/index.js @@ -6,9 +6,15 @@ const validateUrl = require('./src/validate-https-url'); /** * @param {Object} json + * @param {Object=} options * @return {{valid: boolean, errors: Array}} errors is null if valid */ -module.exports = function(json) { +module.exports = function(json, options) { + options = options || {}; + let relativePath = str => true; + if (options.relativePath) { + relativePath = options.relativePath; + } const ajv = new Ajv({ allErrors: true, unknownFormats: true, @@ -16,7 +22,7 @@ module.exports = function(json) { formats: { url: str => validateUrl(str, true), 'https-url': str => validateUrl(str), - 'relative-path': str => true, + 'relative-path': relativePath, }, }); const validate = ajv.compile(jsonSchema); diff --git a/test/index.js b/test/index.js index 61c2621..4613d22 100644 --- a/test/index.js +++ b/test/index.js @@ -106,11 +106,14 @@ describe('validator', () => { assert(actual.errors[0].params.format === 'url'); }); - // enable after implement relative-path - it.skip('invalid http url', () => { - const actual = validator(require('./fixtures/invalid-http-url.json')); + it('invalid http url', () => { + const actual = validator(require('./fixtures/invalid-http-url.json'), { + relativePath: str => false, + }); assert(actual.valid === false); - console.log(actual.errors); - assert(actual.errors.length === 1); + assert(actual.errors.length === 3); + assert(actual.errors[0].keyword === 'format'); + assert(actual.errors[1].keyword === 'format'); + assert(actual.errors[2].keyword === 'anyOf'); }); }); From b7786043fe5965ea9f902c97f37cb627186f202c Mon Sep 17 00:00:00 2001 From: teppeis Date: Fri, 7 Apr 2017 12:50:17 +0900 Subject: [PATCH 3/4] test: replace fixtures with inline JSON --- test/fixtures/2-errors.json | 9 ---- test/fixtures/invalid-http-url.json | 14 ------ test/fixtures/invalid-relative-url.json | 12 ----- test/fixtures/minimal.json | 9 ---- test/fixtures/no-english-description.json | 12 ----- test/fixtures/no-version.json | 8 ---- test/fixtures/type-is-not-app.json | 9 ---- test/fixtures/version-is-string.json | 9 ---- test/fixtures/version-is-zero.json | 9 ---- test/index.js | 55 +++++++++++++++++------ 10 files changed, 41 insertions(+), 105 deletions(-) delete mode 100644 test/fixtures/2-errors.json delete mode 100644 test/fixtures/invalid-http-url.json delete mode 100644 test/fixtures/invalid-relative-url.json delete mode 100644 test/fixtures/minimal.json delete mode 100644 test/fixtures/no-english-description.json delete mode 100644 test/fixtures/no-version.json delete mode 100644 test/fixtures/type-is-not-app.json delete mode 100644 test/fixtures/version-is-string.json delete mode 100644 test/fixtures/version-is-zero.json diff --git a/test/fixtures/2-errors.json b/test/fixtures/2-errors.json deleted file mode 100644 index a61acbc..0000000 --- a/test/fixtures/2-errors.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "manifest_version": "a", - "version": 0, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png" -} diff --git a/test/fixtures/invalid-http-url.json b/test/fixtures/invalid-http-url.json deleted file mode 100644 index 7792a87..0000000 --- a/test/fixtures/invalid-http-url.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "manifest_version": 1, - "version": 1, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png", - "desktop": { - "js": [ - "http://example.com/icon.png" - ] - } -} diff --git a/test/fixtures/invalid-relative-url.json b/test/fixtures/invalid-relative-url.json deleted file mode 100644 index 113366d..0000000 --- a/test/fixtures/invalid-relative-url.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "manifest_version": 1, - "version": 1, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png", - "homepage_url": { - "en": "image/icon.png" - } -} diff --git a/test/fixtures/minimal.json b/test/fixtures/minimal.json deleted file mode 100644 index 9ae94b1..0000000 --- a/test/fixtures/minimal.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "manifest_version": 1, - "version": 1, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png" -} diff --git a/test/fixtures/no-english-description.json b/test/fixtures/no-english-description.json deleted file mode 100644 index 893715a..0000000 --- a/test/fixtures/no-english-description.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "manifest_version": 1, - "version": 1, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "description": { - "ja": "Japanese description" - }, - "icon": "image/icon.png" -} diff --git a/test/fixtures/no-version.json b/test/fixtures/no-version.json deleted file mode 100644 index 9302e87..0000000 --- a/test/fixtures/no-version.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "manifest_version": 1, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png" -} diff --git a/test/fixtures/type-is-not-app.json b/test/fixtures/type-is-not-app.json deleted file mode 100644 index 6d28de5..0000000 --- a/test/fixtures/type-is-not-app.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "manifest_version": 1, - "version": 1, - "type": "FOO", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png" -} diff --git a/test/fixtures/version-is-string.json b/test/fixtures/version-is-string.json deleted file mode 100644 index 57ee18c..0000000 --- a/test/fixtures/version-is-string.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "manifest_version": 1, - "version": "1", - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png" -} diff --git a/test/fixtures/version-is-zero.json b/test/fixtures/version-is-zero.json deleted file mode 100644 index 3b2db59..0000000 --- a/test/fixtures/version-is-zero.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "manifest_version": 1, - "version": 0, - "type": "APP", - "name": { - "en": "sample plugin" - }, - "icon": "image/icon.png" -} diff --git a/test/index.js b/test/index.js index 4613d22..ffbd069 100644 --- a/test/index.js +++ b/test/index.js @@ -1,7 +1,5 @@ 'use strict'; -/* eslint-disable global-require */ - const assert = require('assert'); const validator = require('../'); @@ -11,11 +9,13 @@ describe('validator', () => { }); it('minimal valid JSON', () => { - assert.deepEqual(validator(require('./fixtures/minimal.json')), {valid: true, errors: null}); + assert.deepEqual(validator(json()), {valid: true, errors: null}); }); it('missing property', () => { - assert.deepEqual(validator(require('./fixtures/no-version.json')), { + const manifestJson = json(); + delete manifestJson.version; + assert.deepEqual(validator(manifestJson), { valid: false, errors: [{ dataPath: '.version', @@ -30,7 +30,7 @@ describe('validator', () => { }); it('invalid type', () => { - assert.deepEqual(validator(require('./fixtures/version-is-string.json')), { + assert.deepEqual(validator(json({version: '1'})), { valid: false, errors: [{ dataPath: '.version', @@ -45,7 +45,7 @@ describe('validator', () => { }); it('integer is out of range', () => { - assert.deepEqual(validator(require('./fixtures/version-is-zero.json')), { + assert.deepEqual(validator(json({version: 0})), { valid: false, errors: [{ dataPath: '.version', @@ -62,7 +62,7 @@ describe('validator', () => { }); it('invalid enum value', () => { - assert.deepEqual(validator(require('./fixtures/type-is-not-app.json')), { + assert.deepEqual(validator(json({type: 'FOO'})), { valid: false, errors: [{ dataPath: '.type', @@ -79,7 +79,7 @@ describe('validator', () => { }); it('no English description', () => { - assert.deepEqual(validator(require('./fixtures/no-english-description.json')), { + assert.deepEqual(validator(json({description: {}})), { valid: false, errors: [{ dataPath: '.description.en', @@ -94,21 +94,30 @@ describe('validator', () => { }); it('2 errors', () => { - const actual = validator(require('./fixtures/2-errors.json')); + const actual = validator(json({ + manifest_version: 'a', + version: 0, + })); assert(actual.valid === false); assert(actual.errors.length === 2); }); - it('invalid relative url', () => { - const actual = validator(require('./fixtures/invalid-relative-url.json')); + it('relative path is invalid for `url`', () => { + const actual = validator(json({homepage_url: {en: 'foo/bar.html'}})); assert(actual.valid === false); assert(actual.errors.length === 1); assert(actual.errors[0].params.format === 'url'); }); - it('invalid http url', () => { - const actual = validator(require('./fixtures/invalid-http-url.json'), { - relativePath: str => false, + it('"http:" is invalid for `https-url`', () => { + const actual = validator(json({ + desktop: { + js: [ + 'http://example.com/icon.png' + ] + } + }), { + relativePath: str => !/^https?:/.test(str), }); assert(actual.valid === false); assert(actual.errors.length === 3); @@ -117,3 +126,21 @@ describe('validator', () => { assert(actual.errors[2].keyword === 'anyOf'); }); }); + +/** + * Generate minimum valid manifest.json and overwrite with source + * + * @param {Object=} source + * @return {!Object} + */ +function json(source) { + return Object.assign({ + manifest_version: 1, + version: 1, + type: 'APP', + name: { + en: 'sample plugin', + }, + icon: 'image/icon.png', + }, source); +} From e5c930da8c5bfae4d11b9b7beef7eee7fb47143a Mon Sep 17 00:00:00 2001 From: teppeis Date: Fri, 7 Apr 2017 13:09:58 +0900 Subject: [PATCH 4/4] feat: use `$ref: #resources` in JSON Schmea --- manifest-schema.json | 71 +++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/manifest-schema.json b/manifest-schema.json index 1f0a5da..1fa204a 100644 --- a/manifest-schema.json +++ b/manifest-schema.json @@ -65,7 +65,7 @@ "type": "string", "description": "internal only", "minLength": 1, - "format": "uri" + "format": "relative-path" }, "homepage_url": { "type": "object", @@ -91,28 +91,10 @@ "type": "object", "properties": { "js": { - "type": "array", - "uniqueItems": true, - "items": { - "description": "internal or external", - "type": "string", - "anyOf": [{ - "format": "https-url" - }, { - "format": "relative-path" - }], - "maxItems": 30 - } + "$ref": "#resources" }, "css": { - "type": "array", - "uniqueItems": true, - "items": { - "description": "internal or external", - "type": "string", - "format": "uri", - "maxItems": 30 - } + "$ref": "#resources" } } }, @@ -120,14 +102,7 @@ "type": "object", "properties": { "js": { - "type": "array", - "uniqueItems": true, - "items": { - "description": "internal or external", - "type": "string", - "format": "uri", - "maxItems": 30 - } + "$ref": "#resources" } } }, @@ -137,28 +112,14 @@ "html": { "description": "internal only", "type": "string", - "format": "uri", + "format": "relative-path", "minLength": 1 }, "js": { - "type": "array", - "uniqueItems": true, - "items": { - "description": "internal or external", - "type": "string", - "format": "uri", - "maxItems": 30 - } + "$ref": "#resources" }, "css": { - "type": "array", - "uniqueItems": true, - "items": { - "description": "internal or external", - "type": "string", - "format": "uri", - "maxItems": 30 - } + "$ref": "#resources" }, "required_params": { "type": "array", @@ -177,5 +138,21 @@ "type", "name", "icon" - ] + ], + "definitions": { + "resources": { + "id": "#resources", + "type": "array", + "uniqueItems": true, + "items": { + "type": "string", + "anyOf": [{ + "format": "https-url" + }, { + "format": "relative-path" + }], + "maxItems": 30 + } + } + } }