From f582b93f7f9bd9ad9fb8500b9ec1d19263574078 Mon Sep 17 00:00:00 2001 From: Kevin Dangoor Date: Tue, 9 Apr 2013 10:21:59 -0400 Subject: [PATCH 1/4] Normalizes people in package.json. author and contributors fields are normalized to objects per the description here: https://npmjs.org/doc/json.html#people-fields-author-contributors --- src/extensibility/node/package-validator.js | 68 +++++++++++++++- .../node/spec/Validation.spec.js | 73 +++++++++++++++--- .../basic-valid-extension-2.0.zip | Bin 512 -> 649 bytes .../basic-valid-extension.zip | Bin 470 -> 519 bytes .../one-level-extension-master.zip | Bin 1163 -> 1179 bytes 5 files changed, 129 insertions(+), 12 deletions(-) diff --git a/src/extensibility/node/package-validator.js b/src/extensibility/node/package-validator.js index f19a2bcd0b4..cbe55f13e52 100644 --- a/src/extensibility/node/package-validator.js +++ b/src/extensibility/node/package-validator.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, node: true, nomen: true, -indent: 4, maxerr: 50 */ +indent: 4, maxerr: 50, regexp: true */ "use strict"; @@ -71,6 +71,50 @@ function validateName(name) { return false; } +// Parses strings of the form "name (url)" where email and url are optional +var _personRegex = /^([^<\(]+)(?:\s+<([^>]+)>)?(?:\s+\(([^\)]+)\))?$/; + +/** + * Normalizes person fields from package.json. + * + * These fields can be an object with name, email and url properties or a + * string of the form "name ". This does a tolerant parsing of + * the data to try to return an object with name and optional email and url. + * If the string does not match the format, the string is returned as the + * name on the resulting object. + * + * If an object other than a string is passed in, it's returned as is. + * + * @param object to normalize + * @return {Object} person object with name and optional email and url + */ +function parsePersonString(obj) { + if (typeof (obj) === "string") { + var parts = _personRegex.exec(obj); + + // No regex match, so we just synthesize an object with an opaque name string + if (!parts) { + return { + name: obj + }; + } else { + var result = { + name: parts[1] + }; + if (parts[2]) { + result.email = parts[2]; + } + if (parts[3]) { + result.url = parts[3]; + } + return result; + } + } else { + // obj is not a string, so return as is + return obj; + } +} + /** * Implements the "validate" command in the "extensions" domain. * Validates the zipped package at path. @@ -184,6 +228,25 @@ function validate(path, options, callback) { } else if (!semver.valid(metadata.version)) { errors.push([Errors.INVALID_VERSION_NUMBER, metadata.version, path]); } + + // normalize the author + if (metadata.author) { + metadata.author = parsePersonString(metadata.author); + } + + // contributors should be an array of people. + // normalize each entry. + if (metadata.contributors) { + if (metadata.contributors.map) { + metadata.contributors = metadata.contributors.map(function (person) { + return parsePersonString(person); + }); + } else { + metadata.contributors = [ + parsePersonString(metadata.contributors) + ]; + } + } }); } else if (fileName === "main.js") { foundMainIn = commonPrefix; @@ -220,5 +283,8 @@ function validate(path, options, callback) { }); } +// exported for unit testing +exports._parsePersonString = parsePersonString; + exports.errors = Errors; exports.validate = validate; diff --git a/src/extensibility/node/spec/Validation.spec.js b/src/extensibility/node/spec/Validation.spec.js index 58b918494c0..010a5af298f 100644 --- a/src/extensibility/node/spec/Validation.spec.js +++ b/src/extensibility/node/spec/Validation.spec.js @@ -40,17 +40,18 @@ var testFilesDirectory = path.join(path.dirname(module.filename), "spec", "extension-test-files"); -var basicValidExtension = path.join(testFilesDirectory, "basic-valid-extension.zip"), - missingPackageJSON = path.join(testFilesDirectory, "missing-package-json.zip"), - invalidJSON = path.join(testFilesDirectory, "invalid-json.zip"), - invalidZip = path.join(testFilesDirectory, "invalid-zip-file.zip"), - missingNameVersion = path.join(testFilesDirectory, "missing-name-version.zip"), - missingMain = path.join(testFilesDirectory, "missing-main.zip"), - oneLevelDown = path.join(testFilesDirectory, "one-level-extension-master.zip"), - bogusTopDir = path.join(testFilesDirectory, "bogus-top-dir.zip"), - badname = path.join(testFilesDirectory, "badname.zip"), - mainInDirectory = path.join(testFilesDirectory, "main-in-directory.zip"), - invalidVersion = path.join(testFilesDirectory, "invalid-version.zip"); +var basicValidExtension = path.join(testFilesDirectory, "basic-valid-extension.zip"), + basicValidExtension2 = path.join(testFilesDirectory, "basic-valid-extension-2.0.zip"), + missingPackageJSON = path.join(testFilesDirectory, "missing-package-json.zip"), + invalidJSON = path.join(testFilesDirectory, "invalid-json.zip"), + invalidZip = path.join(testFilesDirectory, "invalid-zip-file.zip"), + missingNameVersion = path.join(testFilesDirectory, "missing-name-version.zip"), + missingMain = path.join(testFilesDirectory, "missing-main.zip"), + oneLevelDown = path.join(testFilesDirectory, "one-level-extension-master.zip"), + bogusTopDir = path.join(testFilesDirectory, "bogus-top-dir.zip"), + badname = path.join(testFilesDirectory, "badname.zip"), + mainInDirectory = path.join(testFilesDirectory, "main-in-directory.zip"), + invalidVersion = path.join(testFilesDirectory, "invalid-version.zip"); describe("Package Validation", function () { it("should handle a good package", function (done) { @@ -61,6 +62,9 @@ describe("Package Validation", function () { expect(metadata.name).toEqual("basic-valid-extension"); expect(metadata.version).toEqual("1.0.0"); expect(metadata.title).toEqual("Basic Valid Extension"); + expect(metadata.author.name).toEqual("Alfred Einstein"); + expect(metadata.author.email).toEqual("alfred_not_albert@thoseeinsteins.org"); + expect(metadata.author.url).toBeUndefined(); done(); }); }); @@ -159,6 +163,7 @@ describe("Package Validation", function () { expect(result.errors.length).toEqual(0); expect(result.metadata.name).toEqual("one-level-extension"); expect(result.commonPrefix).toEqual("one-level-extension-master"); + expect(result.metadata.author.name).toEqual("A Person"); done(); }); }); @@ -191,4 +196,50 @@ describe("Package Validation", function () { done(); }); }); + + it("should handle a variety of person forms", function () { + var parse = packageValidator._parsePersonString; + expect(parse("A Person")).toEqual({ + name: "A Person" + }); + expect(parse({ + name: "A Person" + })).toEqual({ + name: "A Person" + }); + expect(parse("A Person (http://foo.bar)")).toEqual({ + name: "A Person", + url: "http://foo.bar" + }); + expect(parse("A Person ")).toEqual({ + name: "A Person", + email: "foo@bar" + }); + expect(parse("A Person (http://foo.bar)")).toEqual({ + name: "A Person", + email: "foo@bar", + url: "http://foo.bar" + }); + }); + + it("should handle contributors", function (done) { + packageValidator.validate(basicValidExtension2, {}, function (err, result) { + expect(err).toBeNull(); + expect(result.errors.length).toEqual(0); + var contrib = result.metadata.contributors; + expect(contrib.length).toEqual(3); + expect(contrib[0]).toEqual({ + name: "Johan Einstein" + }); + expect(contrib[1]).toEqual({ + name: "Albert Einstein Jr.", + email: "not_that_albert@thoseeinsteins.org" + }); + expect(contrib[2]).toEqual({ + name: "Jens Einstein", + email: "jens@thoseeinsteins.org" + }); + done(); + }); + }); }); \ No newline at end of file diff --git a/test/spec/extension-test-files/basic-valid-extension-2.0.zip b/test/spec/extension-test-files/basic-valid-extension-2.0.zip index e64c65f30066ffccf56e17c7d0603e70634be8ef..34dbe2688fbacea5210eac3bf920074df6412789 100644 GIT binary patch delta 318 zcmZo*>13U7w4N`p(~0Z$L66G}3=A_F85npNWEct(ld}`kQ}wcn^YcPOI2o94E2IS8 z1LD#OZU#n{uRvuiB0%-E6MVT283?pqf2-*f#`TSTx>6FWx!i%?4xiF36HO+jh&Wc2 zPrkBh#SGsW)AK*wJEPmTqe<##$48+xH`K%@Bu#%XHK0Cnp{V?kGqQe7DGSo(C3*XA z{Hb2*DC~8d_p;4dYuR%lnU9}3z2n{Gw@_zcKn{=N4GmxE393@Ftga~8G0e^?p0Od$ zwL~iQ=J)7pbr;^R*%mcX@afDg8_yMpUg^*;VSSbPyJ2y3_1?087LN55+>^_4j%;L8 zw0(Lwh_znm^S0%Zf+gSb{PUgVV}E9!+mUPtyY5xTCCwEp{Xlf6Z8V+PMp`sKCe+ITvwc7@)gENK2*0%wq{b* Z#A0iJH!B;^at0~FRM5|FEoUcfqA}2O5ide zF0J5ZU}X8q$iToN0#qA&+L4RNkf-(GR$JGTw=^GzxrEHn-XUuwWf~Bi@kpVge7o*0 z!`F=g_BOFxSeax6BkbPoL&F;)KGRiB+_AB8RC z|GWP2;XKXjd-|vQJe};?8}WxNT0-?^N8zHJ-A+?qaGNZtx!Y8|n}4z*V Ss)1ZOc>|*YTM*EV3=9BqmRb=2 delta 179 zcmZo?xyC$UcKvjp948;6#ml@H7#Na)n1?}zp&&6iJ25>~FRM5|FEoUcfjLj#KClRg zODnh;7+JnDGBB`+0M)L3_9*0(_Zi>NO+f)(UOMM|uIYN8Iq7rJcddr!S?zOY&Iez5 z7{tJ|bkdngG8?qiCup@AOweKtF9}Uu>6)MyFn8g+M)rA)LgBjN43mQxBY9EWGIPCIM> diff --git a/test/spec/extension-test-files/one-level-extension-master.zip b/test/spec/extension-test-files/one-level-extension-master.zip index 2ac96ea00032a660ace17679bd150f0c242bc034..ce134daabbc9d0d776493549f540caa497a7dc88 100644 GIT binary patch delta 125 zcmeC?oXt65HglY4%EWmw%$7jvZzQv=Xv*YB#v;KGuTG~US3G%Q85kJqfLMKEf!yR@ zjDqY2qA7tOMM6yBqKrN^8@H$^@8?i?vqFF&ggN9;=xNr?c}&X~c~SIFR%CHx25Ov~ N%A&wl&jd7%0RV!DBoqJu delta 108 zcmbQu+08j&_CzP$i6@Mh)80Ey{2R#}dE0SvBx8}_C*M4$IJvEVeHj=SQh-=}Vu9S` yUyOq6->*0Z<^UB5F@>`>3We*6Z?0on#>k7JezGBpBQsFp Date: Tue, 9 Apr 2013 10:42:25 -0400 Subject: [PATCH 2/4] validates brackets.engine --- src/extensibility/node/package-validator.js | 26 +++++++++----- .../node/spec/Validation.spec.js | 35 ++++++++++++------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/extensibility/node/package-validator.js b/src/extensibility/node/package-validator.js index cbe55f13e52..9893807c0de 100644 --- a/src/extensibility/node/package-validator.js +++ b/src/extensibility/node/package-validator.js @@ -37,15 +37,16 @@ var unzip = require("unzip"), var Errors = { - NOT_FOUND_ERR: "NOT_FOUND_ERR", // {0} is path where ZIP file was expected - INVALID_ZIP_FILE: "INVALID_ZIP_FILE", // {0} is path to ZIP file - INVALID_PACKAGE_JSON: "INVALID_PACKAGE_JSON", // {0} is JSON parse error, {1} is path to ZIP file - MISSING_PACKAGE_NAME: "MISSING_PACKAGE_NAME", // {0} is path to ZIP file - BAD_PACKAGE_NAME: "BAD_PACKAGE_NAME", // {0} is the name - MISSING_PACKAGE_VERSION: "MISSING_PACKAGE_VERSION", // {0} is path to ZIP file - INVALID_VERSION_NUMBER: "INVALID_VERSION_NUMBER", // {0} is version string in JSON, {1} is path to ZIP file - MISSING_MAIN: "MISSING_MAIN", // {0} is path to ZIP file - MISSING_PACKAGE_JSON: "MISSING_PACKAGE_JSON" // {0} is path to ZIP file + NOT_FOUND_ERR: "NOT_FOUND_ERR", // {0} is path where ZIP file was expected + INVALID_ZIP_FILE: "INVALID_ZIP_FILE", // {0} is path to ZIP file + INVALID_PACKAGE_JSON: "INVALID_PACKAGE_JSON", // {0} is JSON parse error, {1} is path to ZIP file + MISSING_PACKAGE_NAME: "MISSING_PACKAGE_NAME", // {0} is path to ZIP file + BAD_PACKAGE_NAME: "BAD_PACKAGE_NAME", // {0} is the name + MISSING_PACKAGE_VERSION: "MISSING_PACKAGE_VERSION", // {0} is path to ZIP file + INVALID_VERSION_NUMBER: "INVALID_VERSION_NUMBER", // {0} is version string in JSON, {1} is path to ZIP file + MISSING_MAIN: "MISSING_MAIN", // {0} is path to ZIP file + MISSING_PACKAGE_JSON: "MISSING_PACKAGE_JSON", // {0} is path to ZIP file + INVALID_BRACKETS_VERSION: "INVALID_BRACKETS_VERSION" // {0} is the version string in JSON, {1} is the path to the zip file }; /* @@ -247,6 +248,13 @@ function validate(path, options, callback) { ]; } } + + if (metadata.engines && metadata.engines.brackets) { + var range = metadata.engines.brackets; + if (!semver.validRange(range)) { + errors.push([Errors.INVALID_BRACKETS_VERSION, range, path]); + } + } }); } else if (fileName === "main.js") { foundMainIn = commonPrefix; diff --git a/src/extensibility/node/spec/Validation.spec.js b/src/extensibility/node/spec/Validation.spec.js index 010a5af298f..e7872ecb162 100644 --- a/src/extensibility/node/spec/Validation.spec.js +++ b/src/extensibility/node/spec/Validation.spec.js @@ -40,18 +40,19 @@ var testFilesDirectory = path.join(path.dirname(module.filename), "spec", "extension-test-files"); -var basicValidExtension = path.join(testFilesDirectory, "basic-valid-extension.zip"), - basicValidExtension2 = path.join(testFilesDirectory, "basic-valid-extension-2.0.zip"), - missingPackageJSON = path.join(testFilesDirectory, "missing-package-json.zip"), - invalidJSON = path.join(testFilesDirectory, "invalid-json.zip"), - invalidZip = path.join(testFilesDirectory, "invalid-zip-file.zip"), - missingNameVersion = path.join(testFilesDirectory, "missing-name-version.zip"), - missingMain = path.join(testFilesDirectory, "missing-main.zip"), - oneLevelDown = path.join(testFilesDirectory, "one-level-extension-master.zip"), - bogusTopDir = path.join(testFilesDirectory, "bogus-top-dir.zip"), - badname = path.join(testFilesDirectory, "badname.zip"), - mainInDirectory = path.join(testFilesDirectory, "main-in-directory.zip"), - invalidVersion = path.join(testFilesDirectory, "invalid-version.zip"); +var basicValidExtension = path.join(testFilesDirectory, "basic-valid-extension.zip"), + basicValidExtension2 = path.join(testFilesDirectory, "basic-valid-extension-2.0.zip"), + missingPackageJSON = path.join(testFilesDirectory, "missing-package-json.zip"), + invalidJSON = path.join(testFilesDirectory, "invalid-json.zip"), + invalidZip = path.join(testFilesDirectory, "invalid-zip-file.zip"), + missingNameVersion = path.join(testFilesDirectory, "missing-name-version.zip"), + missingMain = path.join(testFilesDirectory, "missing-main.zip"), + oneLevelDown = path.join(testFilesDirectory, "one-level-extension-master.zip"), + bogusTopDir = path.join(testFilesDirectory, "bogus-top-dir.zip"), + badname = path.join(testFilesDirectory, "badname.zip"), + mainInDirectory = path.join(testFilesDirectory, "main-in-directory.zip"), + invalidVersion = path.join(testFilesDirectory, "invalid-version.zip"), + invalidBracketsVersion = path.join(testFilesDirectory, "invalid-brackets-version.zip"); describe("Package Validation", function () { it("should handle a good package", function (done) { @@ -242,4 +243,14 @@ describe("Package Validation", function () { done(); }); }); + + it("should validate the Brackets version", function (done) { + packageValidator.validate(invalidBracketsVersion, {}, function (err, result) { + expect(err).toBeNull(); + expect(result.errors.length).toEqual(1); + expect(result.errors[0][0]).toEqual("INVALID_BRACKETS_VERSION"); + expect(result.errors[0][1]).toEqual("foo"); + done(); + }); + }); }); \ No newline at end of file From 974225e62460d2f1daf66ad9327e916d71ea3f06 Mon Sep 17 00:00:00 2001 From: Kevin Dangoor Date: Tue, 9 Apr 2013 11:19:33 -0400 Subject: [PATCH 3/4] adds file for test in previous commit. --- .../invalid-brackets-version.zip | Bin 0 -> 504 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/spec/extension-test-files/invalid-brackets-version.zip diff --git a/test/spec/extension-test-files/invalid-brackets-version.zip b/test/spec/extension-test-files/invalid-brackets-version.zip new file mode 100644 index 0000000000000000000000000000000000000000..6b2ce096f42968a3a430179f25c3c91793cad533 GIT binary patch literal 504 zcmWIWW@Zs#U|`^2_~x7E^gYi-BLc`v0b+Ir8HU`%%sjoU;?NLI24;qC5POB%TxLkef)khJC=N3a_`e)%guu8xgC#8 z;f~^nkrHkybo#UGlE%Vxy#-qHl&@DlEG>9@_RzUPfvg>@XP#tmti3L?>&yHO_ut|H zaEF};?sVd0){q1_YBmt_03BA4n4F!Mo(gnZejeC$-&Iqvxo-8dM3ZvGojY?r_|n552BxKx&PfdnL4|1qgAT`L-X&JL_VJuL!KXt{voZvDGcw6B;|fFxpo Date: Tue, 9 Apr 2013 11:19:50 -0400 Subject: [PATCH 4/4] Allows the rejection of certain words in package.json name, title and description fields. Also adds the new validation strings. --- src/extensibility/node/package-validator.js | 33 +++++++++++++++++-- .../node/spec/Validation.spec.js | 16 +++++++++ src/nls/root/strings.js | 2 ++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/extensibility/node/package-validator.js b/src/extensibility/node/package-validator.js index 9893807c0de..7b7111a6a12 100644 --- a/src/extensibility/node/package-validator.js +++ b/src/extensibility/node/package-validator.js @@ -46,7 +46,8 @@ var Errors = { INVALID_VERSION_NUMBER: "INVALID_VERSION_NUMBER", // {0} is version string in JSON, {1} is path to ZIP file MISSING_MAIN: "MISSING_MAIN", // {0} is path to ZIP file MISSING_PACKAGE_JSON: "MISSING_PACKAGE_JSON", // {0} is path to ZIP file - INVALID_BRACKETS_VERSION: "INVALID_BRACKETS_VERSION" // {0} is the version string in JSON, {1} is the path to the zip file + INVALID_BRACKETS_VERSION: "INVALID_BRACKETS_VERSION", // {0} is the version string in JSON, {1} is the path to the zip file, + DISALLOWED_WORDS: "DISALLOWED_WORDS" // {0} is the field with the word, {1} is a string list of words that were in violation, {2} is the path to the zip file }; /* @@ -116,6 +117,25 @@ function parsePersonString(obj) { } } +/** + * Determines if any of the words in wordlist appear in str. + * + * @param {String[]} list of words to check + * @param {String} string to check for words + * @return {String[]} words that matched + */ +function containsWords(wordlist, str) { + var i; + var matches = []; + for (i = 0; i < wordlist.length; i++) { + var re = new RegExp("\\b" + wordlist[i] + "\\b", "i"); + if (re.exec(str)) { + matches.push(wordlist[i]); + } + } + return matches; +} + /** * Implements the "validate" command in the "extensions" domain. * Validates the zipped package at path. @@ -132,7 +152,7 @@ function parsePersonString(obj) { * read successfully from package.json in the zip file. * * @param {string} Absolute path to the package zip file - * @param {{requirePackageJSON: ?boolean}} validation options + * @param {{requirePackageJSON: ?boolean, disallowedWords: ?Array.}} validation options * @param {function} callback (err, result) */ function validate(path, options, callback) { @@ -255,6 +275,15 @@ function validate(path, options, callback) { errors.push([Errors.INVALID_BRACKETS_VERSION, range, path]); } } + + if (options.disallowedWords) { + ["title", "description", "name"].forEach(function (field) { + var words = containsWords(options.disallowedWords, metadata[field]); + if (words.length > 0) { + errors.push([Errors.DISALLOWED_WORDS, field, words.toString(), path]); + } + }); + } }); } else if (fileName === "main.js") { foundMainIn = commonPrefix; diff --git a/src/extensibility/node/spec/Validation.spec.js b/src/extensibility/node/spec/Validation.spec.js index e7872ecb162..21d79cf3ea6 100644 --- a/src/extensibility/node/spec/Validation.spec.js +++ b/src/extensibility/node/spec/Validation.spec.js @@ -253,4 +253,20 @@ describe("Package Validation", function () { done(); }); }); + + it("should reject a package with rejected words in title or description", function (done) { + packageValidator.validate(basicValidExtension, { + disallowedWords: ["valid"] + }, function (err, result) { + expect(err).toBeNull(); + expect(result.errors.length).toEqual(2); + expect(result.errors[0][0]).toEqual("DISALLOWED_WORDS"); + expect(result.errors[0][1]).toEqual("title"); + expect(result.errors[0][2]).toEqual("valid"); + expect(result.errors[1][0]).toEqual("DISALLOWED_WORDS"); + expect(result.errors[1][1]).toEqual("name"); + expect(result.errors[1][2]).toEqual("valid"); + done(); + }); + }); }); \ No newline at end of file diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 97f1e21fd75..c72663bc3ac 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -317,6 +317,8 @@ define({ "BAD_PACKAGE_NAME" : "{0} is an invalid package name.", "MISSING_PACKAGE_VERSION" : "The package.json file doesn't specify a package version.", "INVALID_VERSION_NUMBER" : "The package version number ({0}) is invalid.", + "INVALID_BRACKETS_VERSION" : "The Brackets compatibility string {{0}} is invalid.", + "DISALLOWED_WORDS" : "The words {{1}} are not allowed in the {{1}} field.", "API_NOT_COMPATIBLE" : "The extension isn't compatible with this version of Brackets. It's installed in your disabled extensions folder.", "MISSING_MAIN" : "The package has no main.js file.", "ALREADY_INSTALLED" : "An extension with the same name was already installed. The new extension is installed in your disabled extensions folder.",