Merge pull request #3384 from adobe/dangoor/more-validation

Adds additional validation used primarily in brackets-registry

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";
 
@@ -37,15 +37,17 @@ 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,
+    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
 };
 
 /*
@@ -71,6 +73,69 @@ function validateName(name) {
     return false;
 }
 
+// Parses strings of the form "name <email> (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 <email> <url>". 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 <String|Object> 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;
+    }
+}
+
+/**
+ * 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.
@@ -87,7 +152,7 @@ function validateName(name) {
  * 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.<string>}} validation options
  * @param {function} callback (err, result)
  */
 function validate(path, options, callback) {
@@ -184,6 +249,41 @@ 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)
+                                    ];
+                                }
+                            }
+
+                            if (metadata.engines && metadata.engines.brackets) {
+                                var range = metadata.engines.brackets;
+                                if (!semver.validRange(range)) {
+                                    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;
@@ -220,5 +320,8 @@ function validate(path, options, callback) {
     });
 }
 
+// exported for unit testing
+exports._parsePersonString = parsePersonString;
+
 exports.errors = Errors;
 exports.validate = validate;

src/extensibility/node/spec/Validation.spec.js

@@ -40,17 +40,19 @@ 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"),
+    invalidBracketsVersion = path.join(testFilesDirectory, "invalid-brackets-version.zip");
 
 describe("Package Validation", function () {
     it("should handle a good package", function (done) {
@@ -61,6 +63,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 +164,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 +197,76 @@ 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 <foo@bar>")).toEqual({
+            name: "A Person",
+            email: "foo@bar"
+        });
+        expect(parse("A Person <foo@bar> (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();
+        });
+    });
+
+    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();
+        });
+    });
+
+    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();
+        });
+    });
 });

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.",