From cc74244b3354d07f2b5ec6b147391d79cc2ed1da Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@keybit.net>
Date: Mon, 6 May 2013 12:30:32 +0200
Subject: [PATCH] Do not choke on multiple `skipfields` parameter

---
 NEWS.md                     |  1 +
 app/controllers/app.js      | 39 ++++++++++++++++++++++++-------------
 test/acceptance/app.test.js | 20 +++++++++++++++++++
 3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 808fee50..59d9844f 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,6 @@
 1.3.9
 -----
+* Do not choke on multiple `skipfields` parameter
 
 1.3.8
 -----
diff --git a/app/controllers/app.js b/app/controllers/app.js
index 94dd375e..b390bd08 100755
--- a/app/controllers/app.js
+++ b/app/controllers/app.js
@@ -104,26 +104,39 @@ function handleQuery(req, res) {
     var requestedFilename = req.query.filename || body.filename
     var filename  = requestedFilename;
     var requestedSkipfields = req.query.skipfields || body.skipfields;
-    var skipfields = requestedSkipfields ? requestedSkipfields.split(',') : [];
+    var skipfields;
     var dp        = req.query.dp || body.dp; // decimal point digits (defaults to 6)
     var gn        = "the_geom"; // TODO: read from configuration file
     var user_id;
     var tableCacheItem;
 
-    // sanitize and apply defaults to input
-    dp        = (dp       === "" || _.isUndefined(dp))       ? '6'  : dp;
-    format    = (format   === "" || _.isUndefined(format))   ? 'json' : format.toLowerCase();
-    filename  = (filename === "" || _.isUndefined(filename)) ? 'cartodb-query' : sanitize_filename(filename);
-    sql       = (sql      === "" || _.isUndefined(sql))      ? null : sql;
-    database  = (database === "" || _.isUndefined(database)) ? null : database;
-    limit     = (_.isNumber(limit))  ? limit : null;
-    offset    = (_.isNumber(offset)) ? offset * limit : null;
-
-    // setup step run
-    var start = new Date().getTime();
-
     try {
 
+        // sanitize and apply defaults to input
+        dp        = (dp       === "" || _.isUndefined(dp))       ? '6'  : dp;
+        format    = (format   === "" || _.isUndefined(format))   ? 'json' : format.toLowerCase();
+        filename  = (filename === "" || _.isUndefined(filename)) ? 'cartodb-query' : sanitize_filename(filename);
+        sql       = (sql      === "" || _.isUndefined(sql))      ? null : sql;
+        database  = (database === "" || _.isUndefined(database)) ? null : database;
+        limit     = (_.isNumber(limit))  ? limit : null;
+        offset    = (_.isNumber(offset)) ? offset * limit : null;
+
+        // Accept both comma-separated string or array of comma-separated strings
+        if ( requestedSkipfields ) {
+          if ( _.isString(requestedSkipfields) ) skipfields = requestedSkipfields.split(',');
+          else if ( _.isArray(requestedSkipfields) ) {
+            skipfields = [];
+            _.each(requestedSkipfields, function(ele) {
+              skipfields = skipfields.concat(ele.split(','));
+            });
+          }
+        } else {
+          skipfields = [];
+        }
+
+        // setup step run
+        var start = new Date().getTime();
+
         if ( -1 === supportedFormats.indexOf(format) )
           throw new Error("Invalid format: " + format);
 
diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js
index cbdfb82a..3c07b475 100644
--- a/test/acceptance/app.test.js
+++ b/test/acceptance/app.test.js
@@ -602,6 +602,26 @@ test('skipfields controls included fields', function(done){
     });
 });
 
+test('multiple skipfields parameter do not kill the backend', function(done){
+    assert.response(app, {
+        url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&skipfields=unexistent,the_geom_webmercator&skipfields=cartodb_id,unexistant',
+        headers: {host: 'vizzuality.cartodb.com'},
+        method: 'GET'
+    },{ }, function(res){
+        assert.equal(res.statusCode, 200, res.body);
+        var row0 = JSON.parse(res.body).rows[0];
+        var checkfields = {'name':1, 'cartodb_id':0, 'the_geom':1, 'the_geom_webmercator':0};
+        for ( var f in checkfields ) {
+          if ( checkfields[f] ) {
+            assert.ok(row0.hasOwnProperty(f), "result does not include '" + f + "'");
+          } else {
+            assert.ok(!row0.hasOwnProperty(f), "result includes '" + f + "'");
+          }
+        }
+        done();
+    });
+});
+
 test('GET /api/v1/sql ensure cross domain set on errors', function(done){
     assert.response(app, {
         url: '/api/v1/sql?q=SELECT%20*gadfgadfg%20FROM%20untitle_table_4',