Lua script takes non integer argument as input which causes random error on user supplied input

[shameersheik]

The argument 1 used in the INCRBY statement in the Lua script can be anything and not necessarily an integer.

https://github.com/animir/node-rate-limiter-flexible/blob/d4e26c0748e9213b5646694f8a3d46db8ba56824/lib/RateLimiterRedis.js#L5

This will cause "“ERR value is not an integer or out of range script: , on @user_script:1. ”

The problems :

1. No one knows this is a lua script excplicity.
2. Which of the lua script commands causes this.
3. Based on my understanding we set points to IRateLimiterOptions and this points value is divided by the allowedrequests. If points/allowedrequests gives a decimal number. This error will occur.
4. This will occur solely based on the value of allowedrequests and will appear as a random issue.

Solution :

1. Make sure to use a Math.floor option in consume function for pointstoconsume. https://github.com/animir/node-rate-limiter-flexible/blob/d4e26c0748e9213b5646694f8a3d46db8ba56824/lib/RateLimiterRedis.js#L80

[animir]

@shameersheik Hi, this is interesting. Could you help to reproduce this error or provide a code snippet? Could you also provide rate-limiter-flexible version and redis client package name and version you have installed? Thank you.

[shameersheik]

Yep.

Way to reproduce.

1. Set your client budget as 10

`const CLIENT_BUDGET = 10; // Here client budget is 10

const RATE_LIMITER_OPTIONS: IRateLimiterOptions = { duration: RATE_LIMITER_WINDOW, points: CLIENT_BUDGET, };

//create your Redis ratelimiter as neccessary new RedisRateLimiter({ storeClient: redisClient, ...RATE_LIMITER_OPTIONS });`

2. Set your points to consume as "3.33" any non integer value to the consume function of RedisRateLimiter.

const pointsToConsume = (clientMaxTPSAllowed: number): number => CLIENT_BUDGET / clientMaxTPSAllowed; // Here the pointsthat are supposed to be consumed is calculated

// if clientMaxTPSAllowed =2 , pointsToConsume is 10/2 = 5 (no problem) , this is an integer // if clientMaxTPSAllowed =3 , pointsToConsume is 10/3 = 3.33 (Yes problem), this is not an integer

// if we call consume like below here which will call RateLimiterRedis.consume pointsToConsume can be integer or a float.

RedisRateLimiter.consume("somekey",pointsToConsume); // here pointsToConsume can be 5 or 3.33

3. This consume function will call the lua script with ARGV[1] as a non integer, which will give the error "This will cause "“ERR value is not an integer or out of range script: , on @user_script:1. ”" , So we will have to make sure that ARGV[1] is always an integer. INCRBY only accepts integer 64 bit signed integers. https://redis.io/commands/incrby/

https://github.com/animir/node-rate-limiter-flexible/blob/d4e26c0748e9213b5646694f8a3d46db8ba56824/lib/RateLimiterRedis.js#L5

rate-limiter-flexible version : "^2.3.4" ioredis : "^4.28.1"

4. I believe this error can only been caught with reconnectonError https://github.com/animir/node-rate-limiter-flexible/issues/205. I just connected with my Redis server and hit the below command, got the same error. "incrby" "somekey" "3.33"

[shameersheik]

@animir Sorry Forgot to tag.

[shameersheik]

Hi any update on this ?

[animir]

@shameersheik Hi, I looked into your issue. When we use this package, we set number of points and it is kind of supposed that points are integers not decimal. If you need to consume 3.33 from 10 available it is always possible to consume 333 points from 1000. So the issue is not in decimals. You have interesting example, as you didn't know you were going to consume decimal and that's the problem. So there should be a way to know, if somebody's code consume decimal number of points. If the package rounds that silently, there may be consequences for some package users who use this package to count money. Yes, it may be unexpected, but there are companies how do that for their pricing plans to limit number of actions per period and account, for example. Considering that, we can't silently round that number. What we can do is throw an exception with an error, so it would be possible to understand the issue and decide what to do in this case.

In your case, you may round that number of points before points are consumed. Or, as mentioned before, you can increase the number of points in the limiter and consume 333 from 1000.

I hope this helps.

When I or somebody from the community have time to work on your issue, we'll do that. You can also contribute and add that additional check to consume method. The error would say something like Consuming decimal number of points is not supported by this package. That would be helpful in cases like yours.

Note, I also edited some docs on wiki and added integer where it was appropriate.

[shameersheik]

When I or somebody from the community have time to work on your issue, we'll do that. You can also contribute and add that additional check to consume method. The error would say something like Consuming decimal number of points is not supported by this package. That would be helpful in cases like yours.

Thank you will take a look at this.