From 4bd92c71db031509b7eb39a34c5f470f2efaeec6 Mon Sep 17 00:00:00 2001
From: Bruce Merry <bmerry@ska.ac.za>
Date: Wed, 24 Jan 2018 15:56:20 +0200
Subject: [PATCH] Fix handling of invalid expiry times

This fixes #5 (zero expiry time should be treated as invalid) and #7
(key should not be set when expiry time is invalid).
---
 fakenewsredis.py      | 19 +++++++++----------
 requirements.txt      |  2 +-
 test_fakenewsredis.py | 20 +++++++++++++++++++-
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/fakenewsredis.py b/fakenewsredis.py
index ce1e1ed..a104150 100644
--- a/fakenewsredis.py
+++ b/fakenewsredis.py
@@ -510,26 +510,25 @@ def renamenx(self, src, dst):
     def set(self, name, value, ex=None, px=None, nx=False, xx=False):
         if (not nx and not xx) or (nx and self._db.get(name, None) is None) \
                 or (xx and not self._db.get(name, None) is None):
-            self._db[name] = to_bytes(value)
             if ex is not None:
                 if isinstance(ex, timedelta):
                     ex = ex.seconds + ex.days * 24 * 3600
-                if ex < 0:
+                if ex <= 0:
                     raise ResponseError('invalid expire time in SETEX')
-                if ex > 0:
-                    self._db.expire(name, datetime.now() +
-                                    timedelta(seconds=ex))
+                self._db[name] = to_bytes(value)
+                self._db.expire(name, datetime.now() +
+                                timedelta(seconds=ex))
             elif px is not None:
                 if isinstance(px, timedelta):
                     ms = int(px.microseconds / 1000)
                     px = (px.seconds + px.days * 24 * 3600) * 1000 + ms
-                if px < 0:
+                if px <= 0:
                     raise ResponseError('invalid expire time in SETEX')
-                if px > 0:
-                    self._db.expire(name, datetime.now() +
-                                    timedelta(milliseconds=px))
+                self._db[name] = to_bytes(value)
+                self._db.expire(name, datetime.now() +
+                                timedelta(milliseconds=px))
             else:
-                self._db.persist(name)
+                self._db[name] = to_bytes(value)
             return True
         else:
             return None
diff --git a/requirements.txt b/requirements.txt
index df48512..12cc5c9 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
 # Flake8 3.0.0 does not support Python 2.6.
 flake8<3.0.0
 nose==1.3.4
-redis==2.10.5
+redis==2.10.6
diff --git a/test_fakenewsredis.py b/test_fakenewsredis.py
index 183591e..403d885 100644
--- a/test_fakenewsredis.py
+++ b/test_fakenewsredis.py
@@ -501,26 +501,44 @@ def test_set_px_using_timedelta(self):
     def test_set_raises_wrong_ex(self):
         with self.assertRaises(ResponseError):
             self.redis.set('foo', 'bar', ex=-100)
+        with self.assertRaises(ResponseError):
+            self.redis.set('foo', 'bar', ex=0)
+        self.assertFalse(self.redis.exists('foo'))
 
     def test_set_using_timedelta_raises_wrong_ex(self):
         with self.assertRaises(ResponseError):
             self.redis.set('foo', 'bar', ex=timedelta(seconds=-100))
+        with self.assertRaises(ResponseError):
+            self.redis.set('foo', 'bar', ex=timedelta(seconds=0))
+        self.assertFalse(self.redis.exists('foo'))
 
     def test_set_raises_wrong_px(self):
         with self.assertRaises(ResponseError):
             self.redis.set('foo', 'bar', px=-100)
+        with self.assertRaises(ResponseError):
+            self.redis.set('foo', 'bar', px=0)
+        self.assertFalse(self.redis.exists('foo'))
 
     def test_set_using_timedelta_raises_wrong_px(self):
         with self.assertRaises(ResponseError):
             self.redis.set('foo', 'bar', px=timedelta(milliseconds=-100))
+        with self.assertRaises(ResponseError):
+            self.redis.set('foo', 'bar', px=timedelta(milliseconds=0))
+        self.assertFalse(self.redis.exists('foo'))
 
     def test_setex_raises_wrong_ex(self):
         with self.assertRaises(ResponseError):
             self.redis.setex('foo', -100, 'bar')
+        with self.assertRaises(ResponseError):
+            self.redis.setex('foo', 0, 'bar')
+        self.assertFalse(self.redis.exists('foo'))
 
     def test_setex_using_timedelta_raises_wrong_ex(self):
         with self.assertRaises(ResponseError):
             self.redis.setex('foo', timedelta(seconds=-100), 'bar')
+        with self.assertRaises(ResponseError):
+            self.redis.setex('foo', timedelta(seconds=-100), 'bar')
+        self.assertFalse(self.redis.exists('foo'))
 
     def test_setnx(self):
         self.assertEqual(self.redis.setnx('foo', 'bar'), True)
@@ -3142,7 +3160,7 @@ def test_set_xx_set_value_when_exists(self):
 
     @attr('slow')
     def test_set_ex_should_expire_value(self):
-        self.redis.set('foo', 'bar', ex=0)
+        self.redis.set('foo', 'bar')
         self.assertEqual(self.redis.get('foo'), b'bar')
         self.redis.set('foo', 'bar', ex=1)
         sleep(2)