Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RedisStore is required with each limiter #171

Closed
Zirafnik opened this issue Jul 11, 2023 · 1 comment
Closed

RedisStore is required with each limiter #171

Zirafnik opened this issue Jul 11, 2023 · 1 comment

Comments

@Zirafnik
Copy link

Zirafnik commented Jul 11, 2023

Description

This not an bug, but just an issue I faced, which will hopefully save somebody hours of work.

For most this is probably already obvious, but you need to create a new RedisStore for each limiter.

You cannot define a RedisStore and then re-use it accross all limiters. This does NOT work:

const defaultSettings = {
    standardHeaders: true,
    legacyHeaders: false,
    store: new RedisStore({
            sendCommand: (...args: string[]) => client.sendCommand(args),
            prefix: 'rateLimit:',
    }),
};

const limiterOne = rateLimit({
    ...defaultSettings,
    windowMs: 24 * 60 * 60 * 1000, // 24 hours
    max: 500, 
});


const limiterTwo = rateLimit({
    ...defaultSettings,
    windowMs: 60 * 1000, // 1 min
    max: 10, 
});

It doesn't work, because RedisStore reads the windowMs property when initializing, and uses it to set reset-timer. Without it, the reset-timer is set by default to 24 hours. Additionally, in the above example the RedisStore would always use the same key prefix, which means that different API routes would count towards the same limit.

So in short, you always need to create a new RedisStore with each limiter.

const defaultSettings = {
    standardHeaders: true,
    legacyHeaders: false,
};

const limiterOne = rateLimit({
    ...defaultSettings,
    windowMs: 24 * 60 * 60 * 1000, // 24 hours
    max: 500, 
    store: new RedisStore({
            sendCommand: (...args: string[]) => client.sendCommand(args),
            prefix: 'rateLimitOne:',
    }),
});


const limiterTwo = rateLimit({
    ...defaultSettings,
    windowMs: 60 * 1000, // 1 min
    max: 10, 
    store: new RedisStore({
            sendCommand: (...args: string[]) => client.sendCommand(args),
            prefix: 'rateLimitTwo:',
    }),

EXTRA

You can extract the sendCommand function to re-use it across all new RedisStores.

const defaultSettings = {
    standardHeaders: true,
    legacyHeaders: false,
};

const sendCommand = (...args) => client.sendCommand(args);

const limiterOne = rateLimit({
    ...defaultSettings,
    windowMs: 24 * 60 * 60 * 1000, // 24 hours
    max: 500, 
    store: new RedisStore({
            sendCommand,
            prefix: 'rateLimitOne:',
    }),
});


const limiterTwo = rateLimit({
    ...defaultSettings,
    windowMs: 60 * 1000, // 1 min
    max: 10, 
    store: new RedisStore({
            sendCommand,
            prefix: 'rateLimitTwo:',
    }),
@nfriedly
Copy link
Member

express-rate-limit/express-rate-limit#364 should help reduce the confusion here. It will make it log an error if the first request is counted more than once with the same store/key/prefix. The error will have a link to https://express-rate-limit.github.io/ERR_ERL_DOUBLE_COUNT/ where we can put a bit of additional information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants