From aa4c540c6e67b65bfd98f76ab93ad500713d7e0b Mon Sep 17 00:00:00 2001 From: Fabian Jakobs Date: Thu, 8 Jun 2017 13:05:06 +0000 Subject: [PATCH 1/2] use token bucket for rate limiting --- node_modules/c9/ratelimit.js | 43 ++++++++++++------------------- node_modules/c9/ratelimit_test.js | 15 +++++------ 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/node_modules/c9/ratelimit.js b/node_modules/c9/ratelimit.js index 7871b58a..f72c3f4e 100644 --- a/node_modules/c9/ratelimit.js +++ b/node_modules/c9/ratelimit.js @@ -1,4 +1,5 @@ var error = require("http-error"); +var RateLimiter = require('limiter').RateLimiter; var MAX_EXPIRE_INTERVAL = 5000; @@ -8,34 +9,14 @@ var MAX_EXPIRE_INTERVAL = 5000; module.exports = ratelimit; function ratelimit(key, duration, max) { - var requests = Object.create(null); // in case there handles like 'constructor' + + var buckets = Object.create(null); // in case there handles like 'constructor' var rootKey = "params"; if (/^req\./.test(key)) { rootKey = null; key = key.replace(/^req\./, ""); } - setInterval(function() { - Object.keys(requests).forEach(expireRequests); - }, Math.min(duration * 0.75, MAX_EXPIRE_INTERVAL)); - - function expireRequests(handle) { - var requestsForHandle = requests[handle]; - var totalToSplice = 0; - var expireTime = Date.now() - duration; - /* Requests are already sorted by date as they are appended, so we just loop - until we find one that shouldn't have expired and splice them from the list */ - for (totalToSplice = 0; totalToSplice < requestsForHandle.length; totalToSplice++) { - if (requestsForHandle[totalToSplice] >= expireTime) break; - } - requests[handle].splice(0, totalToSplice); - if (requests[handle].length == 0) { - delete requests[handle]; - } - - return true; - } - // Returns a deep value from an object. E.g. resolveValue({user: {id: 5}}, "user.id") === 5 function resolveValue(obj, path) { if (path === "*") @@ -45,19 +26,29 @@ function ratelimit(key, duration, max) { return prev ? prev[curr] : undefined; }, obj); } + + // cleanup empty buckets + setInterval(function() { + Object.keys(buckets).forEach(function(handle) { + var bucket = buckets[handle]; + if (bucket.bucket.content === 0) { + delete buckets[handle]; + } + }); + }, 5 * 1000); return function(req, res, next) { var root = rootKey ? req[rootKey] : req; var handle = resolveValue(root, key); - requests[handle] = requests[handle] || []; - if (requests[handle].length >= max) { + buckets[handle] = buckets[handle] || new RateLimiter(max, duration, true); + var removed = buckets[handle].tryRemoveTokens(1); + if (!removed) { var err = new error.TooManyRequests("Rate limit exceeded"); err.retryIn = Math.min(duration, 5000); return next(err); } - - requests[handle].push(Date.now()); + return next(); }; } diff --git a/node_modules/c9/ratelimit_test.js b/node_modules/c9/ratelimit_test.js index 13a798b7..ffa9b8db 100644 --- a/node_modules/c9/ratelimit_test.js +++ b/node_modules/c9/ratelimit_test.js @@ -111,25 +111,24 @@ describe("ratelimit", function() { it("Should expire keys at the correct times", function (done) { var clock = sinon.useFakeTimers(); var limiter = ratelimit("username", 50, 2); - limiter({params: {username: "mario"}}, null, function(err){ + limiter({params: {username: "mario"}}, null, function(err) { assert(!err, err); }); clock.tick(40); - limiter({params: {username: "mario"}}, null, function(err){ + limiter({params: {username: "mario"}}, null, function(err) { assert(!err, err); }); clock.tick(45); - limiter({params: {username: "mario"}}, null, function(err){ + limiter({params: {username: "mario"}}, null, function(err) { assert(!err, err); }); - limiter({params: {username: "mario"}}, null, function(err){ + limiter({params: {username: "mario"}}, null, function(err) { + assert(!err, err); + }); + limiter({params: {username: "mario"}}, null, function(err) { assert(err); assert.equal(err.code, 429); }); - clock.tick(40); - limiter({params: {username: "mario"}}, null, function(err){ - assert(!err, err); - }); done(); }); }); \ No newline at end of file From 97aa3d17bd7ce5ba3b2b5a0aca97d60d05bab976 Mon Sep 17 00:00:00 2001 From: Fabian Jakobs Date: Thu, 8 Jun 2017 14:32:17 +0000 Subject: [PATCH 2/2] fix cleanup --- node_modules/c9/ratelimit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_modules/c9/ratelimit.js b/node_modules/c9/ratelimit.js index f72c3f4e..034c3003 100644 --- a/node_modules/c9/ratelimit.js +++ b/node_modules/c9/ratelimit.js @@ -31,7 +31,7 @@ function ratelimit(key, duration, max) { setInterval(function() { Object.keys(buckets).forEach(function(handle) { var bucket = buckets[handle]; - if (bucket.bucket.content === 0) { + if (bucket.tokenBucket.content === 0) { delete buckets[handle]; } });