illogical pttl value leads to functional blacklisting

[weisjohn]

I'm still trying to track down how this happened, but on my local redis (3.0.2) I see:

127.0.0.1:6379> get limit:::1:count
"-1"
127.0.0.1:6379> pttl limit:::1:count
(integer) -1

And according to http://redis.io/commands/pttl,

"The command returns -1 if the key exists but has no associated expire."

Should we check for -1 as well as null in https://github.com/scttcper/koa-simple-ratelimit/blob/master/index.js#L79 ?

As it is now, it can never get unset and the user (me) is hosed.

[weisjohn]

After deleting the key, everything was functioning correctly. I don't know how to reproduce the setting, however, it did happen without any special manipulation on my part.

[scttcper]

@weisjohn @niftylettuce it looks like if you have multiple connections at once we could have a race condition the results in -1. What they're talking about in this pull request. tj/node-ratelimiter#25 Maybe on the next request if the ttl is -1 we can delete the key.

[scttcper]

Nevermind what I said its not multiple connections, I've reproduced it.

If the ttl is very short say 1ms left or something.
The key expires and is removed
A new key is decreased.
This key has no ttl and blacklists the user with a -1 value.

[scttcper]

@niftylettuce opened #35 let me know if you have any thoughts here

[niftylettuce]

Just wanted to follow up here - it's because we were using Number.MAX_VALUE in development which Redis doesn't like 👍. Switching it to 10000 was better - but ultimately I'm disabling rate limiting in dev 👍