continueExceeding option does not work as expected

[erettozi]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.18.0

Plugin version

8.0.1

Node.js version

18.16.1

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Debian GNU/Linux 11 (bullseye)

Description

The problem appears to be caused by a bug in the redis implementation of the @fastify/rate-limit plugin. When the option continueExceeding=true, the operation does not work as expected. Some users were experiencing throttling even for legitimate traffic (below 20 requests/minute - so below the 80 threshold in my case).

There is probably a BUG here: fastify-rate-limit/store/RedisStore.js

'use strict'

const noop = () => { }

function RedisStore (redis, key, timeWindow, continueExceeding) {
  this.redis = redis
  this.timeWindow = timeWindow
  this.key = key
  this.continueExceeding = continueExceeding
}

RedisStore.prototype.incr = function (ip, cb) {
  const key = this.key + ip
  if (this.continueExceeding) {
    this.redis.pipeline()
      .incr(key)
      .pexpire(key, this.timeWindow)
      .exec((err, result) => {
        if (err) return cb(err, { current: 0 })
        if (result[0][0]) return cb(result[0][0], { current: 0 })
        cb(null, { current: result[0][1], ttl: this.timeWindow })
      })
  } else {
    this.redis.pipeline()
      .incr(key)
      .pttl(key)
      .exec((err, result) => {
        /**
         * result[0] => incr response: [0]: error, [1]: new incr value
         * result[1] => pttl response: [0]: error, [1]: ttl remaining
         */
        if (err) return cb(err, { current: 0 })
        if (result[0][0]) return cb(result[0][0], { current: 0 })
        if (result[1][1] === -1) {
          this.redis.pexpire(key, this.timeWindow, noop)
          result[1][1] = this.timeWindow
        }
        cb(null, { current: result[0][1], ttl: result[1][1] })
      })
  }
}

RedisStore.prototype.child = function (routeOptions) {
  const child = Object.create(this)
  child.key = this.key + routeOptions.routeInfo.method + routeOptions.routeInfo.url + '-'
  child.timeWindow = routeOptions.timeWindow
  child.continueExceeding = routeOptions.continueExceeding
  return child
}

module.exports = RedisStore

Steps to Reproduce

import Fastify from 'fastify'

(async () => {
  const fastify = Fastify()
  await fastify.register(import('@fastify/rate-limit'), {
    max: 80,
    timeWindow: '1 minute',
    continueExceeding: true
  })

  fastify.get('/', (_request, reply) => {
    reply.send({ hello: 'fastify' })
  })

  fastify.listen({ port: 8000 }, (err, address) => {
    if (err) throw err
    console.log('Server running on port 80')
  })
})()

Expected Behavior

When continueExceeding=true, renew user limitation when user sends a request to the server when still limited

[mcollina]

Note that the steps to reproduce does not have a way to generate/show the problem you are describing.

Anyhow, a PR to fix this would be highly welcomed.

[gurgunday]

Some users were experiencing throttling even for legitimate traffic (below 20 requests/minute - so below the 80 threshold in my case).

Make sure if you're using a proxy in production (most likely!) that trustProxy is properly set up, I had the exact same problem and it's not because of a bug in the code but rather because Fastify thinks only one IP is making the requests and applies rate limiting