diff --git a/.gitignore b/.gitignore index 1000315b..ddeef2ef 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,3 @@ redis.pid *.log coverage/ .DS_Store -libredis_cell.so diff --git a/NEWS.md b/NEWS.md index b61c816f..588166b9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # Changelog +## 6.0.1 +Released 2018-mm-dd + ## 6.0.0 Released 2018-03-19 Backward incompatible changes: @@ -9,6 +12,7 @@ New features: - Upgrades camshaft to 0.61.8 - Upgrades cartodb-redis to 1.0.0 - Rate limit feature (disabled by default) + - Fixes for tests with PG11 ## 5.4.0 Released 2018-03-15 diff --git a/lib/cartodb/middleware/rate-limit.js b/lib/cartodb/middleware/rate-limit.js index 9a3daa9b..b3c6d7fe 100644 --- a/lib/cartodb/middleware/rate-limit.js +++ b/lib/cartodb/middleware/rate-limit.js @@ -39,12 +39,16 @@ function rateLimit(userLimitsApi, endpointGroup = null) { res.set({ 'Carto-Rate-Limit-Limit': limit, 'Carto-Rate-Limit-Remaining': remaining, - 'Retry-After': retry, 'Carto-Rate-Limit-Reset': reset }); if (isBlocked) { - const rateLimitError = new Error('You are over the limits.'); + // retry is floor rounded in seconds by redis-cell + res.set('Retry-After', retry + 1); + + let rateLimitError = new Error( + 'You are over platform\'s limits. Please contact us to know more details' + ); rateLimitError.http_status = 429; rateLimitError.type = 'limit'; rateLimitError.subtype = 'rate-limit'; diff --git a/lib/cartodb/middleware/vector-error.js b/lib/cartodb/middleware/vector-error.js index 75e93b0a..8268ec1f 100644 --- a/lib/cartodb/middleware/vector-error.js +++ b/lib/cartodb/middleware/vector-error.js @@ -5,7 +5,7 @@ module.exports = function vectorError() { return function vectorErrorMiddleware(err, req, res, next) { if(req.params.format === 'mvt') { - if (isTimeoutError(err)) { + if (isTimeoutError(err) || isRateLimitError(err)) { res.set('Content-Type', 'application/x-protobuf'); return res.status(429).send(timeoutErrorVectorTile); } @@ -27,3 +27,7 @@ function isDatasourceTimeoutError (err) { function isTimeoutError (err) { return isRenderTimeoutError(err) || isDatasourceTimeoutError(err); } + +function isRateLimitError (err) { + return err.type === 'limit' && err.subtype === 'rate-limit'; +} diff --git a/package.json b/package.json index b72f9b8c..8b222d24 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "6.0.0", + "version": "6.0.1", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" diff --git a/run_tests.sh b/run_tests.sh index 8412a851..f26cad6d 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -50,17 +50,6 @@ die() { exit 1 } -get_redis_cell() { - if test x"$OPT_REDIS_CELL" = xyes; then - echo "Downloading redis-cell" - curl -L https://github.com/brandur/redis-cell/releases/download/v0.2.2/redis-cell-v0.2.2-x86_64-unknown-linux-gnu.tar.gz --output redis-cell.tar.gz > /dev/null 2>&1 - tar xvzf redis-cell.tar.gz > /dev/null 2>&1 - mv libredis_cell.so ${BASEDIR}/test/support/libredis_cell.so - rm redis-cell.tar.gz - rm libredis_cell.d - fi -} - trap 'cleanup_and_exit' 1 2 3 5 9 13 while [ -n "$1" ]; do @@ -122,9 +111,13 @@ fi TESTS=$@ if test x"$OPT_CREATE_REDIS" = xyes; then - get_redis_cell echo "Starting redis on port ${REDIS_PORT}" - echo "port ${REDIS_PORT}" | redis-server - --loadmodule ${BASEDIR}/test/support/libredis_cell.so > ${BASEDIR}/test.log & + REDIS_CELL_PATH="${BASEDIR}/test/support/libredis_cell.so" + if [[ "$OSTYPE" == "darwin"* ]]; then + REDIS_CELL_PATH="${BASEDIR}/test/support/libredis_cell.dylib" + fi + + echo "port ${REDIS_PORT}" | redis-server - --loadmodule ${REDIS_CELL_PATH} > ${BASEDIR}/test.log & PID_REDIS=$! echo ${PID_REDIS} > ${BASEDIR}/redis.pid fi diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index a1a93e40..dfb6174f 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -1336,7 +1336,7 @@ describe(suiteName, function() { status: 403 }, function(res) { - assert.ok(res.body.match(/permission denied for relation test_table_private_1/)); + assert.ok(res.body.match(/permission denied for .+?test_table_private_1/)); done(); } ); diff --git a/test/acceptance/named_layers.js b/test/acceptance/named_layers.js index 5844c479..2090ffe2 100644 --- a/test/acceptance/named_layers.js +++ b/test/acceptance/named_layers.js @@ -659,7 +659,7 @@ describe('named_layers', function() { } var parsedBody = JSON.parse(response.body); - assert.ok(parsedBody.errors[0].match(/permission denied for relation test_table_private_1/)); + assert.ok(parsedBody.errors[0].match(/permission denied for .+?test_table_private_1/)); return null; }, diff --git a/test/acceptance/rate-limit.test.js b/test/acceptance/rate-limit.test.js index 339e187e..eb9f8954 100644 --- a/test/acceptance/rate-limit.test.js +++ b/test/acceptance/rate-limit.test.js @@ -15,6 +15,7 @@ let redisClient; let testClient; let keysToDelete = ['user:localhost:mapviews:global']; const user = 'localhost'; +let layergroupid; const query = ` SELECT @@ -68,13 +69,13 @@ const createMapConfig = ({ }); -function setLimit(count, period, burst) { +function setLimit(count, period, burst, endpoint = RATE_LIMIT_ENDPOINTS_GROUPS.ANONYMOUS) { redisClient.SELECT(8, err => { if (err) { return; } - const key = `limits:rate:store:${user}:maps:${RATE_LIMIT_ENDPOINTS_GROUPS.ANONYMOUS}`; + const key = `limits:rate:store:${user}:maps:${endpoint}`; redisClient.rpush(key, burst); redisClient.rpush(key, count); redisClient.rpush(key, period); @@ -87,8 +88,12 @@ function getReqAndRes() { req: {}, res: { headers: {}, - set(headers) { - this.headers = headers; + set(headers, value) { + if(typeof headers === 'object') { + this.headers = headers; + } else { + this.headers[headers] = value; + } }, locals: { user: 'localhost' @@ -97,18 +102,21 @@ function getReqAndRes() { }; } -function assertGetLayergroupRequest (status, limit, remaining, reset, retry, done = null) { - const response = { +function assertGetLayergroupRequest (status, limit, remaining, reset, retry, done) { + let response = { status, headers: { 'Content-Type': 'application/json; charset=utf-8', 'Carto-Rate-Limit-Limit': limit, 'Carto-Rate-Limit-Remaining': remaining, - 'Carto-Rate-Limit-Reset': reset, - 'Retry-After': retry + 'Carto-Rate-Limit-Reset': reset } }; + if(retry) { + response.headers['Retry-After'] = retry; + } + testClient.getLayergroup({ response }, err => { assert.ifError(err); if (done) { @@ -117,21 +125,26 @@ function assertGetLayergroupRequest (status, limit, remaining, reset, retry, don }); } -function assertRateLimitRequest (status, limit, remaining, reset, retry, done = null) { +function assertRateLimitRequest (status, limit, remaining, reset, retry, done) { const { req, res } = getReqAndRes(); rateLimit(req, res, function (err) { - assert.deepEqual(res.headers, { + let expectedHeaders = { "Carto-Rate-Limit-Limit": limit, "Carto-Rate-Limit-Remaining": remaining, - "Carto-Rate-Limit-Reset": reset, - "Retry-After": retry - }); + "Carto-Rate-Limit-Reset": reset + }; + + if(retry) { + expectedHeaders['Retry-After'] = retry; + } + + assert.deepEqual(res.headers, expectedHeaders); if(status === 200) { assert.ifError(err); } else { assert.ok(err); - assert.equal(err.message, 'You are over the limits.'); + assert.equal(err.message, 'You are over platform\'s limits. Please contact us to know more details'); assert.equal(err.http_status, 429); assert.equal(err.type, 'limit'); assert.equal(err.subtype, 'rate-limit'); @@ -178,7 +191,7 @@ describe('rate limit', function() { const burst = 1; setLimit(count, period, burst); - assertGetLayergroupRequest(200, '2', '1', '1', '-1', done); + assertGetLayergroupRequest(200, '2', '1', '1', null, done); }); it("1 req/sec: 2 req/seg should be limited", function(done) { @@ -187,12 +200,12 @@ describe('rate limit', function() { const burst = 1; setLimit(count, period, burst); - assertGetLayergroupRequest(200, '2', '1', '1', '-1'); - setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1', '-1'), 250); - setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '0'), 500); - setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '0'), 750); - setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '0'), 950); - setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1', '-1', done), 1050); + assertGetLayergroupRequest(200, '2', '1', '1'); + setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1'), 250); + setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '1'), 500); + setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '1'), 750); + setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '1'), 950); + setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1', null, done), 1050); }); }); @@ -234,12 +247,12 @@ describe('rate limit middleware', function () { }); it("1 req/sec: 2 req/seg should be limited", function (done) { - assertRateLimitRequest(200, 1, 0, 1, -1); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 250); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 500); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 750); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 950); - setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, -1, done), 1050); + assertRateLimitRequest(200, 1, 0, 1); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 250); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 500); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 750); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 950); + setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, null, done), 1050); }); it("1 req/sec: 2 req/seg should be limited, removing SHA script from Redis", function (done) { @@ -248,13 +261,100 @@ describe('rate limit middleware', function () { 'SCRIPT', ['FLUSH'], function () { - assertRateLimitRequest(200, 1, 0, 1, -1); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 500); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 500); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 750); - setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 950); - setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, -1, done), 1050); + assertRateLimitRequest(200, 1, 0, 1); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 500); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 500); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 750); + setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 950); + setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, null, done), 1050); } ); }); }); + +describe('rate limit and vector tiles', function () { + + before(function(done) { + global.environment.enabledFeatures.rateLimitsEnabled = true; + global.environment.enabledFeatures.rateLimitsByEndpoint.tile = true; + + redisClient = redis.createClient(global.environment.redis.port); + const count = 1; + const period = 1; + const burst = 0; + setLimit(count, period, burst, RATE_LIMIT_ENDPOINTS_GROUPS.TILE); + + testClient = new TestClient(createMapConfig(), 1234); + testClient.getLayergroup({status: 200}, (err, res) => { + assert.ifError(err); + + layergroupid = res.layergroupid; + + done(); + }); + }); + + after(function() { + global.environment.enabledFeatures.rateLimitsEnabled = false; + global.environment.enabledFeatures.rateLimitsByEndpoint.tile = false; + }); + + afterEach(function(done) { + keysToDelete.forEach( key => { + redisClient.del(key); + }); + + redisClient.SELECT(0, () => { + redisClient.del('user:localhost:mapviews:global'); + + redisClient.SELECT(5, () => { + redisClient.del('user:localhost:mapviews:global'); + done(); + }); + }); + }); + + it('mvt rate limited', function (done) { + const tileParams = (status, limit, remaining, reset, retry, contentType) => { + let headers = { + "Content-Type": contentType, + "Carto-Rate-Limit-Limit": limit, + "Carto-Rate-Limit-Remaining": remaining, + "Carto-Rate-Limit-Reset": reset + }; + + if (retry) { + headers['Retry-After'] = retry; + } + + return { + layergroupid: layergroupid, + format: 'mvt', + response: {status, headers} + }; + }; + + testClient.getTile(0, 0, 0, tileParams(204, '1', '0', '1'), (err) => { + assert.ifError(err); + + testClient.getTile( + 0, + 0, + 0, + tileParams(429, '1', '0', '0', '1', 'application/x-protobuf'), + (err, res, tile) => { + assert.ifError(err); + + var tileJSON = tile.toJSON(); + assert.equal(Array.isArray(tileJSON), true); + assert.equal(tileJSON.length, 2); + assert.equal(tileJSON[0].name, 'errorTileSquareLayer'); + assert.equal(tileJSON[1].name, 'errorTileStripesLayer'); + + done(); + } + ); + }); + + }); +}); diff --git a/test/acceptance/turbo-carto/regressions.js b/test/acceptance/turbo-carto/regressions.js index 9e2d377f..3f3ebc57 100644 --- a/test/acceptance/turbo-carto/regressions.js +++ b/test/acceptance/turbo-carto/regressions.js @@ -88,7 +88,7 @@ describe('turbo-carto regressions', function() { var turboCartoError = layergroup.errors_with_context[0]; assert.ok(turboCartoError); assert.equal(turboCartoError.type, 'layer'); - assert.ok(turboCartoError.message.match(/permission\sdenied\sfor\srelation\stest_table_private_1/)); + assert.ok(turboCartoError.message.match(/permission\sdenied\sfor\s.+?test_table_private_1/)); done(); }); diff --git a/test/support/libredis_cell.dylib b/test/support/libredis_cell.dylib new file mode 100755 index 00000000..7c7d6c88 Binary files /dev/null and b/test/support/libredis_cell.dylib differ diff --git a/test/support/libredis_cell.so b/test/support/libredis_cell.so new file mode 100755 index 00000000..1da8aa66 Binary files /dev/null and b/test/support/libredis_cell.so differ