From f76606bc2632df81660fe0762aa2bbe2a0c4195e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 14:13:27 +0100 Subject: [PATCH] Do not use locals middleware in layergroup controller --- lib/cartodb/api/auth_api.js | 6 ++--- lib/cartodb/controllers/layergroup.js | 31 +++++++++------------- lib/cartodb/middleware/error-middleware.js | 12 +++------ lib/cartodb/middleware/layergroup-token.js | 5 ++-- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index e9f10262..d27ea3e1 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -25,7 +25,7 @@ module.exports = AuthApi; // null if the request is not signed by anyone // or will be a string cartodb username otherwise. // -AuthApi.prototype.authorizedBySigner = function(res, callback) { +AuthApi.prototype.authorizedBySigner = function(req, res, callback) { if ( ! res.locals.token || ! res.locals.signer ) { return callback(null, false); // no signer requested } @@ -33,7 +33,7 @@ AuthApi.prototype.authorizedBySigner = function(res, callback) { var self = this; var layergroup_id = res.locals.token; - var auth_token = res.locals.auth_token; + var auth_token = req.query.auth_token; this.mapStore.load(layergroup_id, function(err, mapConfig) { if (err) { @@ -180,7 +180,7 @@ AuthApi.prototype.authorize = function(req, res, callback) { }); } - this.authorizedBySigner(res, (err, isAuthorizedBySigner) => { + this.authorizedBySigner(req, res, (err, isAuthorizedBySigner) => { if (err) { return callback(err); } diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0930fbed..da9189ef 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -1,7 +1,6 @@ const cors = require('../middleware/cors'); const user = require('../middleware/user'); const vectorError = require('../middleware/vector-error'); -const locals = require('../middleware/locals'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); @@ -91,7 +90,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), @@ -110,17 +108,16 @@ LayergroupController.prototype.register = function(app) { surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), - sendResponse(), incrementErrorMetrics(global.statsClient), tileError(), - vectorError() + vectorError(), + sendResponse() ); app.get( `${mapConfigBasePath}/:token/:z/:x/:y.:format`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), @@ -139,10 +136,10 @@ LayergroupController.prototype.register = function(app) { surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), - sendResponse(), incrementErrorMetrics(global.statsClient), tileError(), - vectorError() + vectorError(), + sendResponse() ); app.get( @@ -150,7 +147,6 @@ LayergroupController.prototype.register = function(app) { distinguishLayergroupFromStaticRoute(), cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), @@ -169,17 +165,16 @@ LayergroupController.prototype.register = function(app) { surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), - sendResponse(), incrementErrorMetrics(global.statsClient), tileError(), - vectorError() + vectorError(), + sendResponse() ); app.get( `${mapConfigBasePath}/:token/:layer/attributes/:fid`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), layergroupToken(), @@ -206,7 +201,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, cors(), cleanUpQueryParams(['layer']), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), @@ -232,7 +226,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, cors(), cleanUpQueryParams(['layer']), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), @@ -261,7 +254,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/dataview/:dataviewName`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), @@ -286,7 +278,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), @@ -311,7 +302,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), @@ -336,7 +326,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), @@ -361,7 +350,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/analysis/node/:nodeId`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), layergroupToken(), @@ -521,7 +509,7 @@ function getFeatureAttributes (attributesBackend) { } function getStatusCode(tile, format){ - return tile.length === 0 && format === 'mvt'? 204 : 200; + return tile.length === 0 && format === 'mvt' ? 204 : 200; } function parseFormat (format = '') { @@ -654,6 +642,11 @@ function incrementErrorMetrics (statsClient) { function tileError () { return function tileErrorMiddleware (err, req, res, next) { + if (err.message === 'Tile does not exist' && req.params.format === 'mvt') { + res.statusCode = 204; + return next(); + } + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 85f38936..c02974b4 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -15,10 +15,6 @@ module.exports = function errorMiddleware (/* options */) { var statusCode = findStatusCode(err); - if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { - statusCode = 204; - } - setErrorHeader(allErrors, statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); @@ -186,15 +182,15 @@ function setErrorHeader(errors, statusCode, res) { subtype: error.subtype }; }); - + res.set('X-Tiler-Errors', stringifyForLogs(errorsLog)); } /** - * Remove problematic nested characters + * Remove problematic nested characters * from object for logs RegEx - * - * @param {Object} object + * + * @param {Object} object */ function stringifyForLogs(object) { Object.keys(object).map(key => { diff --git a/lib/cartodb/middleware/layergroup-token.js b/lib/cartodb/middleware/layergroup-token.js index 797b1b3d..c3fcec30 100644 --- a/lib/cartodb/middleware/layergroup-token.js +++ b/lib/cartodb/middleware/layergroup-token.js @@ -5,13 +5,12 @@ const authErrorMessageTemplate = function (signer, user) { module.exports = function layergroupToken () { return function layergroupTokenMiddleware (req, res, next) { - if (!res.locals.token) { + if (!req.params.token) { return next(); } const user = res.locals.user; - - const layergroupToken = LayergroupToken.parse(res.locals.token); + const layergroupToken = LayergroupToken.parse(req.params.token); res.locals.token = layergroupToken.token; res.locals.cache_buster = layergroupToken.cacheBuster;