fix: handle sanitising better, add tests

Author: billiegoose

index.js

@@ -5,19 +5,24 @@ var spawn = childProcess.spawn;
 var exec = childProcess.exec;
 
 module.exports = function (pid, signal, callback) {
-    if (typeof pid !== "number") {
-        throw new Error("pid must be a number");
+    if (typeof signal === 'function' && callback === undefined) {
+        callback = signal;
+        signal = undefined;
+    }
+
+    pid = parseInt(pid);
+    if (Number.isNaN(pid)) {
+        if (callback) {
+            return callback(new Error("pid must be a number"));
+        } else {
+            throw new Error("pid must be a number");
+        }
     }
 
     var tree = {};
     var pidsToProcess = {};
     tree[pid] = [];
     pidsToProcess[pid] = 1;
-    
-    if (typeof signal === 'function' && callback === undefined) {
-      callback = signal;
-      signal = undefined;
-    }
 
     switch (process.platform) {
     case 'win32':

test/test.js

@@ -31,4 +31,27 @@ describe('kill()', function(){
             return done()
         })
     })
+
+    it('should reject invalid pid', function(done){
+        var p = fork('./test/spin')
+        assert.ok(p.pid)
+
+        kill('rm -rf /dev/null', function(err) {
+            assert.ok(typeof err === 'object')
+            return done()
+        })
+    })
+
+    it('should reject invalid pids even if no callback', function(done){
+        var p = fork('./test/spin')
+        assert.ok(p.pid)
+
+        try {
+            kill('rm -rf /dev/null')
+            assert.fail('should have thrown')
+        } catch (err) {
+            assert.ok(typeof err === 'object')
+            return done();
+        }
+    })
 })