ashish-goyal / redis

Automatically exported from code.google.com/p/redis
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

setex silent data loss when using unix time for expiration value #660

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Setex specifies that the expiration value is a delta in seconds. However, it is 
easy to pass an absolute timestamp by mistake.

If this happens then redis accepts the command, however it does not store 
anything:

pie@reactor ~ % irb
irb(main):001:0> Time.now.to_i + 1000
=> 1315722345

pie@reactor ~ % redis-cli -p 5011
redis 127.0.0.1:5011> setex foo 1315722345 1
OK
redis 127.0.0.1:5011> get foo
(nil)
redis 127.0.0.1:5011> setex foo 1000 1
OK
redis 127.0.0.1:5011> get foo
"1"

Using redis 2.2.12 on freebsd 8.2. This is a 32-bit system.

Original issue reported on code.google.com by googlec...@bsdpower.com on 11 Sep 2011 at 6:12

GoogleCodeExporter commented 8 years ago
Hello, this is a bug indeed and it is due to an integer overflow that only 
happens in 32 bit builds.

However there is no perfect fix as time_t is 32 bit in 32 bit archs. What I'm 
probably going to do is to set it to the max unix time when it would overflow, 
so that those keys will expire in 2036. But this somewhat violates the 
semantics as you set a given expire and read a different one using the TTL 
command... better to think a bit more about it :) But this is likely the best 
fix for 32 bit.

Thanks for reporting.

Salvatore

Original comment by anti...@gmail.com on 12 Sep 2011 at 11:33

GoogleCodeExporter commented 8 years ago

Original comment by anti...@gmail.com on 12 Sep 2011 at 11:34

GoogleCodeExporter commented 8 years ago
Redis already appears to reject larger expiration times (e.g. anything over 
2**31 on my system). I think that rejecting values that would overflow a 32-bit 
value after being added to the current time would be an acceptable solution.

An argument may be made that redis should not fall apart around year 2037 and 
should support 64-bit times on all architectures. This argument is somewhat 
orthogonal to the bug here - redis should never overflow the time value; if on 
a 64-bit system I specify 2**64-1000 as the expiration value it should either 
be rejected or treated correctly as a future value, instead of being wrapped 
around at 2**64.

Due to redis requesting a delta for the expiration time, people who want to 
specify a value far in the future are likely to use something like "10 years 
from now" as opposed to "January 1, 2020". As we get closer to 2037 having 
32-bit values for times will begin to affect more users.

Requiring a 64-bit redis for proper time handling has its own drawbacks. One is 
increased memory consumption of 64-bit redis, which is addressed in the faq. 
Another one is continuing existence of low-spec servers with under 4 gb of ram, 
where running a 64-bit operating system makes little or no sense.

In my case there was a bug in the code and I actually wanted a reasonable 
expiration time, but due to porting the code from memcached it misspecified the 
delta as a timestamp.

Original comment by googlec...@bsdpower.com on 13 Sep 2011 at 2:10

GoogleCodeExporter commented 8 years ago
Thanks for the additional details, I moved the issue to the new issue system at 
github so that we can address it ASAP. Cheers.

Original comment by anti...@gmail.com on 20 Sep 2011 at 10:19