Merge pull request #437 from CodeNow/SAN-694/ignore-whitespace-only-ENV-values-on-PATCH-to-instances-id

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

Author: tjmehta

lib/routes/instances/index.js

@@ -75,8 +75,12 @@ var bodyValidations = flow.series(
   ),
   mw.body('env').require()
     .then(
-      mw.body('env').array()
-        .validate(validations.isArrayOf('string')),
+      mw.body('env').array(),
+      mw.body('env').mapValues(function (envArr) {
+        return envArr.filter(function (val) {
+          return !(/^\s*$/.test(val));
+        });
+      }),
       mw.body('env').each(
         function (env, req, eachReq, res, next) {
           eachReq.env = env;

package.json

@@ -22,7 +22,7 @@
     "connect-datadog": "0.0.5",
     "connect-redis": "^2.0.0",
     "cors": "^2.4.1",
-    "dat-middleware": "^1.9.2",
+    "dat-middleware": "^1.9.3",
     "debug": "^1.0.2",
     "defaults": "^1.0.0",
     "docker-stream-cleanser": "0.1.2",

test/fixtures/types-test-util.js

@@ -35,7 +35,7 @@ function errorMessageSuffix (paramType, type) {
     'string': 'must be a string',
     'repo-string' : 'must be a string',
     'number': 'must be a number',
-    'array': 'should be an array',
+    'array': 'must be instance of Array',
     'object': 'must be an object',
     'ObjectId': 'is not an ObjectId',
   };
@@ -140,9 +140,7 @@ function setupArrayParamsTests (ctx, handler, def, types, param, buildBodyFuncti
         body[param.name].push(typeValue(ctx, arrayItemType));
         body[param.name].push(typeValue(ctx, arrayItemType));
         body[param.name].push(typeValue(ctx, arrayItemType));
-        // e.g. body parameter "env" should be an array of strings
-        var regexp = 'body parameter "' + param.name + '" ' + errorMessageSuffix(param.type, arrayItemType) +
-        ' of ' + param.itemType + 's';
+        regexp = '"env" should match';
         var message = new RegExp(regexp);
         var cb = expects.error(400, message, done);
         handler(body, cb);
@@ -190,4 +188,4 @@ exports.makeTestFromDef = function (def, ctx, handler) {
       setupArrayParamsTests(ctx, handler, def, types, param, buildBodyWithRequiredParams, index);
     });
   }
-};
+};

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));
           });
@@ -322,7 +321,32 @@ describe('POST /instances', function () {
               }]
             };
             ctx.user.createInstance(json,
-              expects.errorStatus(400, /should be an array of strings/, done));
+              expects.errorStatus(400, /"env" should match/, 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 = {