Skip to content

Loading…

[fix] Add correct validations to name and subdomain property #410

Merged
merged 8 commits into from

3 participants

@julianduque

@blakmatrix this should address the name validation problems, Do we need more test cases?

@blakmatrix

adresses #361

@julianduque

LGTM, can you add the correct test cases to subdomain and app name? in test/lib/package-test.js

@julianduque

According to npm docs the name can't start with an underscore.

The name ends up being part of a URL, an argument on the command line, and a folder name. Any name with non-url-safe characters will be rejected. Also, it can't start with a dot or an underscore.
@jcrugzz
nodejitsu member

I haven't come across any issues using it. LGTM :+1:

@blakmatrix blakmatrix merged commit 8b80443 into master

1 check passed

Details default The Travis build passed
@blakmatrix blakmatrix deleted the appname-validation branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 260 additions and 10 deletions.
  1. +26 −3 lib/jitsu/package.js
  2. +1 −1 package.json
  3. +6 −6 test/commands/package-test.js
  4. +227 −0 test/lib/package-test.js
View
29 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',
View
2 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"
View
12 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
//
View
227 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);
Something went wrong with that request. Please try again.