Merge pull request #143 from rwaldron/141

t2 rename: check valid for valid name before attempting connections. Closes gh-141, gh-142
tessel · Jun 4, 2015 · 46ceefc · 46ceefc
2 parents a0aafcb + 23841f7
commit 46ceefc
Show file tree

Hide file tree

Showing 5 changed files with 185 additions and 19 deletions.
diff --git a/lib/controller.js b/lib/controller.js
@@ -93,10 +93,10 @@ function getTessel(opts) {
     }, [])
     .then(function(connections){
       return new Promise(function(resolve, reject) {
-      if(!connections.length){
-        reject('No Tessels Found');
-      }
-      resolve(connections);
+        if(!connections.length){
+          reject('No Tessels Found');
+        }
+        resolve(connections);
       });
     })
     // Sort by ip address
@@ -214,13 +214,18 @@ function eraseScript(opts) {
 }
 
 function renameTessel(opts) {
+  opts = opts || {};
+
   // Grab the preferred tessel
   return new Promise(function(resolve, reject) {
     if (!opts.reset && !opts.newName) {
       reject("A new name must be provided.");
-    }
-    else {
-      resolve();
+    } else {
+      if (!Tessel.isValidName(opts.newName)) {
+        reject("Invalid name: " + opts.newName + ". The name must be a valid hostname string. See http://en.wikipedia.org/wiki/Hostname#Restrictions_on_valid_host_names.");
+      } else {
+        resolve();
+      }
     }
   })
   .then(getTessel.bind(null, opts))

diff --git a/lib/tessel/name.js b/lib/tessel/name.js
@@ -39,17 +39,7 @@ Tessel.prototype.getName = function(callback) {
 Tessel.prototype.setName = function(name, callback) {
   var self = this;
 
-  // Regex to test whether a string is a valid Linux hostname
-  // Picked up from http://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address
-  var validHostnameRegex = /^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$/;
-
-  // If the provided name isn't a string or a valid hostname
-  if (typeof name !== 'string' || !validHostnameRegex.test(name)) {
-    // And a callback was provided
-    if (typeof callback === 'function') {
-      // Return an error
-      callback("Invalid name: " + name + ". The name must be a valid hostname string. See http://en.wikipedia.org/wiki/Hostname#Restrictions_on_valid_host_names.");
-    }
+  if (!Tessel.isValidName(name)) {
     return;
   }
 
@@ -126,6 +116,11 @@ Tessel.prototype.rename = function(opts, callback) {
       });
     });
   } else {
+
+    if (!Tessel.isValidName(opts.newName)) {
+      return;
+    }
+
     self.getName(function(err, oldName) {
       // Handle any errors that may have occurred
       if (Tessel._commonErrorHandler(err)) {

diff --git a/lib/tessel/tessel.js b/lib/tessel/tessel.js
@@ -36,6 +36,13 @@ Tessel._commonErrorHandler = function(err, callback) {
   return false;
 };
 
+Tessel.isValidName = function(value) {
+  // Regex to test whether a string is a valid Linux hostname
+  // http://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address
+  var rvalidHostName =  /^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$/;
+  return value && typeof value === 'string' && rvalidHostName.test(value);
+};
+
 module.exports = Tessel;
 
 require('./provision');

diff --git a/package.json b/package.json
@@ -37,7 +37,8 @@
     "grunt-contrib-nodeunit": "^0.4.1",
     "grunt-contrib-watch": "^0.6.1",
     "grunt-jsbeautifier": "^0.2.10",
-    "grunt-jscs": "^1.8.0"
+    "grunt-jscs": "^1.8.0",
+    "sinon": "^1.14.1"
   },
   "dependencies": {
     "async": "^0.9.0",

diff --git a/test/unit/name.js b/test/unit/name.js
@@ -0,0 +1,158 @@
+var sinon = require("sinon");
+var Tessel = require("../../lib/tessel/tessel");
+var commands = require("../../lib/tessel/commands");
+var logs = require("../../lib/logs");
+var controller = require("../../lib/controller");
+
+exports["Tessel.prototype.rename"] = {
+  setUp: function(done) {
+
+    this.getName = sinon.stub(Tessel.prototype, "getName", function(callback) {
+      callback(null, "TheFakeName");
+    });
+    this._getMACAddress = sinon.stub(Tessel.prototype, "_getMACAddress", function(callback) {
+      callback(null, "TheFakeMACAddress");
+    });
+
+    this.isValidName = sinon.spy(Tessel, "isValidName");
+    this.renameTessel = sinon.spy(controller, "renameTessel");
+    this.setName = sinon.spy(Tessel.prototype, "setName");
+    this.setHostname = sinon.spy(commands, "setHostname");
+    this.getHostname = sinon.spy(commands, "getHostname");
+    this.logsWarn = sinon.stub(logs, "warn", function() {});
+    this.logsInfo = sinon.stub(logs, "info", function() {});
+
+    this.tessel = new Tessel();
+    this.tessel.connection = {
+      exec: sinon.spy()
+    };
+
+    done();
+  },
+
+  tearDown: function(done) {
+    this.isValidName.restore();
+    this.renameTessel.restore();
+    this.getName.restore();
+    this.setName.restore();
+    this._getMACAddress.restore();
+    this.setHostname.restore();
+    this.getHostname.restore();
+    this.logsWarn.restore();
+    this.logsInfo.restore();
+    done();
+  },
+
+  isValidName: function(test) {
+    test.expect(2);
+
+    // This needs more fleshing out.
+    //
+    test.equal(Tessel.isValidName("foo"), true);
+    test.equal(Tessel.isValidName("foo-"), false);
+
+    test.done();
+  },
+
+  renameTesselNoOpts: function(test) {
+    test.expect(1);
+
+    this.renameTessel().catch(function(error) {
+      test.equal(error, "A new name must be provided.");
+      test.done();
+    });
+  },
+
+  renameTesselInvalid: function(test) {
+    test.expect(1);
+
+    this.renameTessel({ newName: "!@#$" }).catch(function(error) {
+      test.equal(error, "Invalid name: !@#$. The name must be a valid hostname string. See http://en.wikipedia.org/wiki/Hostname#Restrictions_on_valid_host_names.");
+      test.done();
+    });
+  },
+
+  resetName: function(test) {
+    test.expect(6);
+
+    this.tessel.rename({ reset: true });
+
+    // When reset:
+    // - the mac address is requested
+    // - setName is called
+    // - the connection executes the setHostName command
+    test.equal(this._getMACAddress.callCount, 1);
+    test.equal(this.setName.callCount, 1);
+    test.equal(this.tessel.connection.exec.callCount, 1);
+    test.equal(this.setHostname.callCount, 1);
+    test.ok(this.setHostname.lastCall.calledWith("Tessel-TheFakeName"));
+
+    // getName is _not_ called.
+    test.equal(this.getName.callCount, 0);
+
+    test.done();
+  },
+
+  validRename: function(test) {
+    // test.expect(3);
+
+    this.tessel.rename({ newName: "ValidAndUnique" });
+
+    // When valid rename:
+    // - getName is called
+    // - setName is called
+    // - the connection executes the setHostName command
+    test.equal(this.getName.callCount, 1);
+    test.equal(this.setName.callCount, 1);
+    test.equal(this.tessel.connection.exec.callCount, 1);
+    test.equal(this.setHostname.callCount, 1);
+    test.ok(this.setHostname.lastCall.calledWith("ValidAndUnique"));
+
+    test.done();
+  },
+
+  validRenameSameAsCurrent: function(test) {
+    test.expect(2);
+
+    var spy = sinon.spy();
+
+    this.tessel.rename({ newName: "TheFakeName" }, spy);
+    // When renamed with same current name:
+    // - warning is logged
+    // - callback called
+    //
+    test.equal(this.logsWarn.callCount, 1);
+    test.equal(spy.callCount, 1);
+
+    test.done();
+  },
+
+  invalidRename: function(test) {
+    test.expect(2);
+
+    this.tessel.rename({ newName: "..." });
+
+    // When invalid rename:
+    // - name is checked
+    // - getName is NOT called
+    test.equal(this.isValidName.callCount, 1);
+    test.equal(this.getName.callCount, 0);
+
+    test.done();
+  },
+
+  invalidSetName: function(test) {
+    test.expect(3);
+
+    this.tessel.setName("...");
+
+    // When invalid rename:
+    // - name is checked
+    // - the connection NEVER executes the setHostName command
+    test.equal(this.isValidName.callCount, 1);
+    test.equal(this.tessel.connection.exec.callCount, 0);
+    test.equal(this.setHostname.callCount, 0);
+
+    test.done();
+  },
+};