hiredis-rb icon indicating copy to clipboard operation
hiredis-rb copied to clipboard

Unable call methods with a timeout greater than 2142 seconds

Open dsander opened this issue 2 years ago • 0 comments

Hi! 👋

We noticed that all call that take a timeout argument like brpoplpush do not accept a timeout greater than 2142 seconds.

$ r.brpoplpush('test', 'test2', timeout: 2143)

RangeError: integer 2148000000 too big to convert to `int'
from vendor/bundle/ruby/3.0.0/gems/redis-4.2.5/lib/redis/connection/hiredis.rb:41:in `timeout='

While this might not be a smart use case (a lot of things can happen to the connection within one hour), I believe it should be supported by hiredis-rb because it works with the plain ruby adapter.

I am pretty sure this line causes the exception:

https://github.com/redis/hiredis-rb/blob/5f0eb41b8fac37fee94cfbe3af482b904c82e48e/ext/hiredis_ext/connection.c#L470

A bit later in the function the usecs are converted back to seconds.

https://github.com/redis/hiredis-rb/blob/5f0eb41b8fac37fee94cfbe3af482b904c82e48e/ext/hiredis_ext/connection.c#L478-L480

This makes me think the NUM2INT cast could be done after the division. I didn't send a PR because I am not sure how regirous the validation of the resulting number should be. Is it safe to assume that t_time is always at least a 32bit integer, or are there smarter/safer ways to do it?

dsander avatar Aug 16 '21 12:08 dsander