Merge pull request #410 from nodejitsu/appname-validation

[fix] Add correct validations to name and subdomain property w/ tests!
nodejitsu · Mar 16, 2013 · 8b80443 · 8b80443
2 parents 9b2789c + f0c9921
commit 8b80443
Show file tree

Hide file tree

Showing 4 changed files with 260 additions and 10 deletions.
diff --git a/lib/jitsu/package.js b/lib/jitsu/package.js
@@ -9,6 +9,7 @@ var fs = require('fs'),
     path = require('path'),
     existsSync = fs.existsSync || path.existsSync,
     util = require('util'),
+    punycode = require('punycode'),
     spawnCommand = require('spawn-command'),
     zlib = require('zlib'),
     async = require('flatiron').common.async,
@@ -540,14 +541,36 @@ package.properties = function (dir) {
     {
       name: 'name',
       unique: true,
-      message: 'App name',
-      validator: /^[\w|\-]+$/,
+      message: 'Application name',
+      validator: /^(?!\.)(?!_)(?!node_modules)(?!favicon.ico)[^\/@\s\+%:\n]+$/,
+      warning: 'The application name  must follow the rules for npm package names.\n'+
+      '         They must not start with a \'.\' or \'_\', contain any whitespace \n'+
+      '         characters or any of the following characters(between quotes): "/@+%:". \n'+
+      '         Additionally, the name may not be \'node_modules\' or \'favicon.ico\'.',
       default: path.basename(dir)
     },
     {
       name: 'subdomain',
       unique: true,
-      validator: /^[\w|\-|\_]+$/,
+      message: 'Subdomain name',//+
+      warning: 'The subdomain must follow the rules for ARPANET host names.  They must\n'+
+      '         start with a letter, end with a letter or digit, and have as interior\n'+
+      '         characters only letters, digits, and hyphen.  There are also some\n'+
+      '         restrictions on the length.  Labels must be 63 characters or less.\n'+
+      '         There are a few exceptions, underscores may be used as an interior \n'+
+      '         character and unicode characters may be used that are supported under\n'+
+      '         punycode.',
+      validator: function(s){
+        var reValidSubdomain = /^[a-zA-Z]$|^[a-zA-Z][a-zA-Z\d]$|^[a-zA-Z][\w\-]{1,61}[a-zA-Z\d]$/;
+        if(s.indexOf('.') !== -1) { // We will support multiple level subdomains this for now warn user...
+          jitsu.log.warn("**WARNING** Do not use multiple level subdomains, they will be going away soon!");
+          var subdomainNames = s.split('.'),
+              names = subdomainNames.map(punycode.toASCII);
+          return !names.some(function(name){return !reValidSubdomain.test(name);});
+        } else {
+          return reValidSubdomain.test(punycode.toASCII(s));
+        }
+      },
       help: [
         '',
         'The ' + 'subdomain '.grey + 'is where the app will reside',

diff --git a/package.json b/package.json
@@ -50,7 +50,7 @@
   },
   "main": "./lib/jitsu",
   "scripts": {
-    "test": "vows test/commands/*-test.js --spec -i"
+    "test": "vows test/commands/*-test.js test/lib/*-test.js --spec -i"
   },
   "engines": {
     "node": ">= 0.6.0"

diff --git a/test/commands/package-test.js b/test/commands/package-test.js
@@ -4,7 +4,7 @@
  * (C) 2010, Nodejitsu Inc.
  *
  */
- 
+
 var assert = require('assert'),
     fs = require('fs'),
     path = require('path'),
@@ -16,13 +16,13 @@ var assert = require('assert'),
 
 var shouldNodejitsuOk = macros.shouldNodejitsuOk;
 
-vows.describe('jitsu/commands/package').addBatch({
+var suite = vows.describe('jitsu/commands/package').addBatch({
   'package create': shouldNodejitsuOk(
     'should create the target tarball',
     function (_, err) {
       var tmproot = jitsu.config.get('tmproot'),
           targetPackage = path.join(tmproot, 'tester-example-app-0.0.0-1.tgz');
-      
+
       try {
         fs.statSync(targetPackage);
       }
@@ -34,10 +34,10 @@ vows.describe('jitsu/commands/package').addBatch({
       var tmproot = jitsu.config.get('tmproot'),
           targetPackage = path.join(tmproot, 'tester-example-app-0.0.0-1.tgz'),
           packageFile = path.join(__dirname, '..', 'fixtures', 'example-app', 'package.json');;
-      
+
       jitsu.argv.noanalyze = true;
       jitsu.prompt.override['invite code'] = 'f4387f4';
-      
+
       //
       // Change directory to the sample app
       //
@@ -52,7 +52,7 @@ vows.describe('jitsu/commands/package').addBatch({
       };
 
       fs.writeFileSync(packageFile, JSON.stringify(pkg, true, 2))
-      
+
       //
       // Attempt to remove any existing tarballs
       //

diff --git a/test/lib/package-test.js b/test/lib/package-test.js
@@ -0,0 +1,227 @@
+/*
+ * package-test.js: Tests for `jitsu.package` functions.
+ *
+ * (C) 2010, Nodejitsu Inc.
+ *
+ */
+
+var assert = require('assert'),
+    fs = require('fs'),
+    path = require('path'),
+    vows = require('vows'),
+    jitsu = require('../../lib/jitsu'),
+    macros = require('../helpers/macros');
+
+var mainDirectory = process.cwd();
+
+jitsu.init();
+//jitsu.log.loggers.default.remove(app.log.loggers.default.transports.console); // make silent
+
+function setupPackage (options) {
+  options = options || {};
+  return {
+    name: options.name || 'example-app',
+    subdomain: options.subdomain || 'example-app',
+    scripts: { start: 'server.js' },
+    version: '0.0.0-1',
+    engines: { node: '0.6.x' }
+  };
+}
+
+function isValid(property, value) {
+  // Use package properties
+  var properties = jitsu.package.properties(mainDirectory);
+
+  // Filter by property name
+  properties = properties.filter(function (el) {
+    return el.name === property;
+  });
+
+  var desc = properties[0] || {};
+
+  if (desc.validator) {
+    if (desc.validator instanceof RegExp) {
+      return desc.validator.test(value);
+    }
+
+    return desc.validator(value);
+  }
+  return false;
+}
+
+var suite = vows.describe('jitsu/lib/package').addBatch({
+  'name': {
+    'starting with .': {
+      topic: setupPackage({ name: '.example-app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'starting with _': {
+      topic: setupPackage({ name: '_example-app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing -': {
+      topic: setupPackage(),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('name', topic.name));
+      }
+    },
+    'containing spaces': {
+      topic: setupPackage({ name: 'example app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing interior .': {
+      topic: setupPackage({ name: 'example.app' }),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('name', topic.name));
+      }
+    },
+    'containing %': {
+      topic: setupPackage({ name: 'example%app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing @': {
+      topic: setupPackage({ name: 'example@app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing :': {
+      topic: setupPackage({ name: 'example:app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing +': {
+      topic: setupPackage({ name: 'example+app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing /': {
+      topic: setupPackage({ name: 'example/app' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'node_modules': {
+      topic: setupPackage({ name: 'node_modules' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'favicon.ico': {
+      topic: setupPackage({ name: 'favicon.ico' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('name', topic.name));
+      }
+    },
+    'containing unicode(๛ಠ_ಠ☠☃❤⁂⍨⋙‽)': {
+      topic: setupPackage({ name: '๛ಠ_ಠ☠☃❤⁂⍨⋙‽' }),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('name', topic.name));
+      }
+    }
+  },
+  'subdomain': {
+    'starting with 9': {
+      topic: setupPackage({ subdomain: '9example' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'starting with _': {
+      topic: setupPackage({ subdomain: '_example' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing -': {
+      topic: setupPackage(),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing spaces': {
+      topic: setupPackage({ subdomain: 'example domain' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing .': { //will become invalid soon
+      topic: setupPackage({ subdomain: 'example.domain' }),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing %': {
+      topic: setupPackage({ subdomain: 'example%domain' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing @': {
+      topic: setupPackage({ subdomain: 'example@domain' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing :': {
+      topic: setupPackage({ subdomain: 'example:domain' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'end with number': {
+      topic: setupPackage({ subdomain: 'c9' }),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('subdomain', topic.subdomain));
+      }
+    },
+    'end with -': {
+      topic: setupPackage({ subdomain: 'c9-' }),
+
+      'should be invalid': function (topic) {
+        assert.ok(!isValid('subdomain', topic.subdomain));
+      }
+    },
+    'containing unicode(๛ಠ_ಠ☠☃❤⁂⍨⋙‽)': {
+      topic: setupPackage({ subdomain: '๛ಠ_ಠ☠☃❤⁂⍨⋙‽' }),
+
+      'should be valid': function (topic) {
+        assert.ok(isValid('subdomain', topic.subdomain));
+      }
+    }
+  }
+});
+
+suite.export(module);