fix(local-store): local store should have time window for each user (#…

…217)

package.json

@@ -7,7 +7,7 @@
     "lint": "standard",
     "lint:fix": "standard --fix",
     "redis": "docker run -p 6379:6379 --rm redis",
-    "test": "standard && tap test/*.test.js && npm run typescript",
+    "test": "standard && tap \"test/**/*.test.js\" && npm run typescript",
     "typescript": "tsd"
   },
   "repository": {

store/LocalStore.js

@@ -3,37 +3,24 @@
 const lru = require('tiny-lru')
 
 function LocalStore (timeWindow, cache, app, continueExceeding) {
-  this.lru = lru(cache || 5000)
-  this.interval = setInterval(beat.bind(this), timeWindow).unref()
+  this.lru = lru(cache || 5000, timeWindow)
   this.app = app
   this.timeWindow = timeWindow
   this.continueExceeding = continueExceeding
-
-  app.addHook('onClose', (instance, done) => {
-    clearInterval(this.interval)
-    done()
-  })
-
-  function beat () {
-    this.lru.clear()
-    this.msLastBeat = null
-  }
 }
 
 LocalStore.prototype.incr = function (ip, cb) {
-  let current = this.lru.get(ip) || 0
-  this.lru.set(ip, ++current)
+  const nowInMs = Date.now()
+  const current = this.lru.get(ip) || { count: 0, iterationStartMs: nowInMs }
+
+  current.count++
+
+  this.lru.set(ip, current)
 
   if (this.continueExceeding) {
-    this.msLastBeat = Date.now()
-    cb(null, { current, ttl: this.timeWindow })
+    cb(null, { current: current.count, ttl: this.timeWindow })
   } else {
-    // start counting from the first request/increment
-    if (this.msLastBeat === undefined || this.msLastBeat === null) {
-      this.msLastBeat = Date.now()
-    }
-
-    cb(null, { current, ttl: this.timeWindow - (Date.now() - this.msLastBeat) })
+    cb(null, { current: current.count, ttl: this.timeWindow - (nowInMs - current.iterationStartMs) })
   }
 }
 

test/github-issues/issue-215.test.js

@@ -0,0 +1,81 @@
+'use strict'
+
+const FakeTimers = require('@sinonjs/fake-timers')
+const t = require('tap')
+const test = t.test
+const Fastify = require('fastify')
+const rateLimit = require('../../index')
+
+t.beforeEach(t => {
+  t.context.clock = FakeTimers.install()
+})
+
+t.afterEach(t => {
+  t.context.clock.uninstall()
+})
+
+test('issue #215 - when using local store, 2nd user should not be rate limited when the time window is passed for the 1st user', async t => {
+  t.plan(5)
+  const fastify = Fastify()
+
+  fastify.register(rateLimit, {
+    global: false
+  })
+
+  fastify.get('/', {
+    config: {
+      rateLimit: {
+        max: 1,
+        timeWindow: 5000,
+        continueExceeding: false
+      }
+    }
+  }, async () => 'hello!')
+
+  const user1FirstRequest = await fastify.inject({
+    url: '/',
+    method: 'GET',
+    remoteAddress: '1.1.1.1'
+  })
+
+  // Waiting for the time to pass to make the 2nd user start in a different start point
+  t.context.clock.tick(3000)
+
+  const user2FirstRequest = await fastify.inject({
+    url: '/',
+    method: 'GET',
+    remoteAddress: '2.2.2.2'
+  })
+
+  const user2SecondRequestAndShouldBeRateLimited = await fastify.inject({
+    url: '/',
+    method: 'GET',
+    remoteAddress: '2.2.2.2'
+  })
+
+  // After this the total time passed for the 1st user is 6s and for the 2nd user only 3s
+  t.context.clock.tick(3000)
+
+  const user2ThirdRequestAndShouldStillBeRateLimited = await fastify.inject({
+    url: '/',
+    method: 'GET',
+    remoteAddress: '2.2.2.2'
+  })
+
+  // After this the total time passed for the 2nd user is 5.1s - he should not be rate limited
+  t.context.clock.tick(2100)
+
+  const user2OkResponseAfterRateLimitCompleted = await fastify.inject({
+    url: '/',
+    method: 'GET',
+    remoteAddress: '2.2.2.2'
+  })
+
+  t.equal(user1FirstRequest.statusCode, 200)
+  t.equal(user2FirstRequest.statusCode, 200)
+
+  t.equal(user2SecondRequestAndShouldBeRateLimited.statusCode, 429)
+  t.equal(user2ThirdRequestAndShouldStillBeRateLimited.statusCode, 429)
+
+  t.equal(user2OkResponseAfterRateLimitCompleted.statusCode, 200)
+})