filtering whitespace-only env keys in instance PATCH/POST routes + tests

Author: cflynn07

lib/routes/instances/index.js

@@ -182,6 +182,22 @@ var removeInstanceContainer = // TODO: this has a lot of overlap with instance s
           }));
   };
 
+/**
+ * Remove any empty/whitespace-only env values
+ * @param {array} req.body.env
+ */
+var instanceTransformEnvs = flow.series(
+  mw.body('env').require()
+    .then(
+      mw.body('env').array(),
+      mw.body('env').mapValues(function (envArr) {
+        return envArr.filter(function (val) {
+          return !(/^\s*$/.test(val));
+        });
+      })
+    )
+);
+
 /** Get's the list of instances to be displayed to the user.  This should contain all of the
  *  instances owned by the owner, as well as those owned by groups (s)he is part of
  *  @event GET rest/instances
@@ -211,6 +227,7 @@ app.get('/instances/',
 app.post('/instances/',
   mw.body('build').require().validate(validations.isObjectId),
   mw.body('name', 'owner', 'env', 'parent', 'build').pick(),
+  instanceTransformEnvs,
   // validate body types
   mw.body('owner.github').require()
     .then(
@@ -421,6 +438,7 @@ app.patch('/instances/:id',
     .then(
       mw.body().unset('build')),
   mw.body({ or: ['public', 'name', 'build', 'env']}).require().pick(),
+  instanceTransformEnvs,
   bodyValidations,
   mw.body({ or: ['build', 'name'] }).require()
     .then(

test/instances-id/patch/index.js

@@ -477,6 +477,20 @@ describe('Instance - /instances/:id', {timeout:1000}, function () {
             ctx.instance.fetch(expects.success(200, expected, done));
           }));
         });
+        it('should filter empty/whitespace-only strings from env array', function (done) {
+          var body = {
+            env: ['', '  ', 'ONE=1']
+          };
+          var expected = {
+            env: ['ONE=1']
+          };
+          require('../../fixtures/mocks/github/user')(ctx.user);
+          ctx.instance.update(body, expects.success(200, expected, function (err) {
+            if (err) { return done(err); }
+            //sanity check
+            ctx.instance.fetch(expects.success(200, expected, done));
+          }));
+        });
       });
 
       var updates = [{

test/instances/post/index.js

@@ -309,7 +309,6 @@ describe('POST /instances', function () {
               'containers[0]': exists
             };
             require('../../fixtures/mocks/github/user')(ctx.user);
-            require('../../fixtures/mocks/github/user')(ctx.user);
             ctx.user.createInstance(json,
               expects.success(201, expected, done));
           });
@@ -324,6 +323,31 @@ describe('POST /instances', function () {
             ctx.user.createInstance(json,
               expects.errorStatus(400, /should be an array of strings/, done));
           });
+          it('should filter empty/whitespace-only strings from env array', function (done) {
+            var json = {
+              name: uuid(),
+              build: ctx.build.id(),
+              env: [
+                '', ' ', 'ONE=1'
+              ]
+            };
+            var expected = {
+              _id: exists,
+              name: json.name,
+              env: ['ONE=1'],
+              owner: {
+                github: ctx.user.json().accounts.github.id,
+                username: ctx.user.json().accounts.github.login
+              },
+              public: false,
+              'build._id': ctx.build.id(),
+              containers: exists,
+              'containers[0]': exists
+            };
+            require('../../fixtures/mocks/github/user')(ctx.user);
+            ctx.user.createInstance(json,
+              expects.success(201, expected, done));
+          });
           it('should error if body.env contains an invalid variable', function (done) {
             var json = {
               name: uuid(),