From 399561d076a69ce5ba7aa9b21a147ccc9931d12e Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 4 Aug 2017 17:30:46 +0200 Subject: [PATCH] Implement LZMA query param inflating as middleware The req2params method is doing too many things, this is an initial step to do fewer things in that method. --- lib/cartodb/controllers/base.js | 34 +--------------- lib/cartodb/middleware/lzma.js | 30 ++++++++++++++ lib/cartodb/server.js | 4 ++ test/unit/cartodb/lzmaMiddleware.test.js | 36 +++++++++++++++++ test/unit/cartodb/req2params.test.js | 51 +++++++++++------------- 5 files changed, 95 insertions(+), 60 deletions(-) create mode 100644 lib/cartodb/middleware/lzma.js create mode 100644 test/unit/cartodb/lzmaMiddleware.test.js diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 5aab7cb3..43235c82 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -4,9 +4,6 @@ var _ = require('underscore'); var step = require('step'); var debug = require('debug')('windshaft:cartodb'); -var LZMA = require('lzma').LZMA; -var lzmaWorker = new LZMA(); - // Whitelist query parameters and attach format var REQUEST_QUERY_PARAMS_WHITELIST = [ 'config', @@ -28,7 +25,7 @@ function BaseController(authApi, pgConnection) { module.exports = BaseController; -// jshint maxcomplexity:10 +// jshint maxcomplexity:8 /** * Whitelist input and get database name & default geometry type from * subdomain/user metadata held in CartoDB Redis @@ -38,35 +35,6 @@ module.exports = BaseController; BaseController.prototype.req2params = function(req, callback){ var self = this; - if ( req.query.lzma ) { - - // Decode (from base64) - var lzma = new Buffer(req.query.lzma, 'base64') - .toString('binary') - .split('') - .map(function(c) { - return c.charCodeAt(0) - 128; - }); - - - // Decompress - lzmaWorker.decompress( - lzma, - function(result) { - req.profiler.done('lzma'); - try { - delete req.query.lzma; - _.extend(req.query, JSON.parse(result)); - self.req2params(req, callback); - } catch (err) { - req.profiler.done('req2params'); - callback(new Error('Error parsing lzma as JSON: ' + err)); - } - } - ); - return; - } - var allowedQueryParams = REQUEST_QUERY_PARAMS_WHITELIST; if (Array.isArray(req.context.allowedQueryParams)) { allowedQueryParams = allowedQueryParams.concat(req.context.allowedQueryParams); diff --git a/lib/cartodb/middleware/lzma.js b/lib/cartodb/middleware/lzma.js new file mode 100644 index 00000000..d58f16cc --- /dev/null +++ b/lib/cartodb/middleware/lzma.js @@ -0,0 +1,30 @@ +'use strict'; + +var LZMA = require('lzma').LZMA; + +var lzmaWorker = new LZMA(); + +module.exports = function lzmaMiddleware(req, res, next) { + if (!req.query.hasOwnProperty('lzma')) { + return next(); + } + + // Decode (from base64) + var lzma = new Buffer(req.query.lzma, 'base64') + .toString('binary') + .split('') + .map(function(c) { + return c.charCodeAt(0) - 128; + }); + + // Decompress + lzmaWorker.decompress(lzma, function(result) { + try { + delete req.query.lzma; + Object.assign(req.query, JSON.parse(result)); + next(); + } catch (err) { + next(new Error('Error parsing lzma as JSON: ' + err)); + } + }); +}; diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index deb88e16..7177b12f 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -4,6 +4,8 @@ var RedisPool = require('redis-mpool'); var cartodbRedis = require('cartodb-redis'); var _ = require('underscore'); +var lzmaMiddleware = require('./middleware/lzma'); + var controller = require('./controllers'); var SurrogateKeysCache = require('./cache/surrogate_keys_cache'); @@ -345,6 +347,8 @@ function bootstrap(opts) { next(); }); + app.use(lzmaMiddleware); + // temporary measure until we upgrade to newer version expressjs so we can check err.status app.use(function(err, req, res, next) { if (err) { diff --git a/test/unit/cartodb/lzmaMiddleware.test.js b/test/unit/cartodb/lzmaMiddleware.test.js new file mode 100644 index 00000000..9a41030a --- /dev/null +++ b/test/unit/cartodb/lzmaMiddleware.test.js @@ -0,0 +1,36 @@ +var assert = require('assert'); +var testHelper = require('../../support/test_helper'); + +var lzmaMiddleware = require('../../../lib/cartodb/middleware/lzma'); + +describe('lzma-middleware', function() { + + it('it should extend params with decoded lzma', function(done) { + var qo = { + config: { + version: '1.3.0' + } + }; + testHelper.lzma_compress_to_base64(JSON.stringify(qo), 1, function(err, data) { + var req = { + headers: { + host:'localhost' + }, + query: { + api_key: 'test', + lzma: data + } + }; + lzmaMiddleware(req, {}, function(err) { + if ( err ) { + return done(err); + } + var query = req.query; + assert.deepEqual(qo.config, query.config); + assert.equal('test', query.api_key); + done(); + }); + }); + }); + +}); diff --git a/test/unit/cartodb/req2params.test.js b/test/unit/cartodb/req2params.test.js index 6293199e..7b055e99 100644 --- a/test/unit/cartodb/req2params.test.js +++ b/test/unit/cartodb/req2params.test.js @@ -1,6 +1,6 @@ var assert = require('assert'); var _ = require('underscore'); -var test_helper = require('../../support/test_helper'); +require('../../support/test_helper'); var RedisPool = require('redis-mpool'); var cartodbRedis = require('cartodb-redis'); @@ -98,34 +98,31 @@ describe('req2params', function() { }); }); - it('it should extend params with decoded lzma', function(done) { - var qo = { - config: { - version: '1.3.0' + it('it should remove invalid params', function(done) { + var config = { + version: '1.3.0' + }; + var req = { + headers: { + host:'localhost' + }, + query: { + non_included: 'toberemoved', + api_key: 'test', + style: 'override', + config: config } }; - test_helper.lzma_compress_to_base64(JSON.stringify(qo), 1, function(err, data) { - var req = { - headers: { - host:'localhost' - }, - query: { - non_included: 'toberemoved', - api_key: 'test', - style: 'override', - lzma: data - } - }; - baseController.req2params(prepareRequest(req), function(err, req) { - if ( err ) { - return done(err); - } - var query = req.params; - assert.deepEqual(qo.config, query.config); - assert.equal('test', query.api_key); - assert.equal(undefined, query.non_included); - done(); - }); + baseController.req2params(prepareRequest(req), function(err, req) { + if (err) { + return done(err); + } + var query = req.params; + assert.deepEqual(config, query.config); + assert.equal('test', query.api_key); + assert.equal(undefined, query.non_included); + assert.equal(undefined, query.style); + done(); }); });