Repository: redis/hiredis-py (language: python)

Issue:
Bump hiredis to 0.14.1

Upgrade hiredis version to v1.0.0
Since hiredis was bumped to version 1.0.0 and became stable, I think this is a good time to upgrade.


Discussion / hints:
@ifduyue 0.14.1 includes a fix to CVE-2020-7105 (failure to
check for NULL return when allocating memory).
See: https://github.com/redis/hiredis/releases/tag/v0.14.1
I'm planning to upgrade hiredis, but simply upgrading hiredis version will break support for windows platform.  This situation was also mentioned in https://github.com/redis/hiredis-py/issues/84#issuecomment-590869906
Thanks @ifduyue!
Why is it breaking the windows support?
>  Why is it breaking the windows support?

hiredis v0.14 and later versions don't support windows anymore, they are using stdint.h and `__attribute__ ((__packed__))`, see https://github.com/redis/hiredis/issues/599
We have hiredis [compiling and running tests](https://github.com/redis/hiredis/pull/776) in Windows, although I've only used MSVC 2017 and MSVC 2019.

Would it be possible to just compile using a newer version of MSVC?

If not, these c99 issues are probably patchable (e.g. do `pragma push(pack(1))` in MSVC and `__attribute__ ((__packed__))` everywhere else.

Happy to take a crack at it if there is interest.
@michael-grunder Please go ahead if you can try to crack.

If it's too hard to crack or need too much effort to maintain, we can use a fallback python reader, just like [hiredis-rb](https://github.com/redis/hiredis-rb/tree/master/lib/hiredis/ruby)
> Would it be possible to just compile using a newer version of MSVC?

https://wiki.python.org/moin/WindowsCompilers#Which_Microsoft_Visual_C.2B-.2B-_compiler_to_use_with_a_specific_Python_version_.3F

<img width="511" alt="image" src="https://user-images.githubusercontent.com/568900/78331843-732dc380-75b9-11ea-92d6-ef63a3a1aad7.png">

It seems we have to drop py2.7 support if we want to use newer version of MSVC
For what it's worth, Python 2 is no longer receiving bug fixes, security patches, etc as of January 1 2020. Selfishly I'd love to drop Python 2 support in redis-py as it will dramatically reduce complexity and maintenance headaches. My 2 cents.
The plan is to release a version that supports both Python 2 and 3, and later versions will go Python 3 only
> The plan is to release a version that supports both Python 2 and 3, and later versions will go Python 3 only

It looks like you released a version after the message. Does it mean that Python 2 support can be dropped in favor of vendoring newer hiredis? 
> It looks like you released a version after the message. Does it mean that Python 2 support can be dropped in favor of vendoring newer hiredis?

That release didn't provide a fallback reader for windows. But we can move forward. 
> That release didn't provide a fallback reader for windows. But we can move forward.

Great! @ifduyue please take a look at #103
We have built against system hiredis on alpine linux, and it segfaults as is when linking against system hiredis 1.0.0. I haven't investigated why.

I agree that it would be nice to bump to 1.0.0 hiredis.

Fix patch:
diff --git a/setup.py b/setup.py
index 409ea45..580aea7 100755
--- a/setup.py
+++ b/setup.py
@@ -12,7 +12,7 @@ def version():
 
 ext = Extension("hiredis.hiredis",
   sources=sorted(glob.glob("src/*.c") +
-                 ["vendor/hiredis/%s.c" % src for src in ("read", "sds")]),
+                 ["vendor/hiredis/%s.c" % src for src in ("alloc", "read", "sds")]),
   include_dirs=["vendor"])
 
 setup(
diff --git a/src/reader.c b/src/reader.c
index 5a3dca1..48630c7 100644
--- a/src/reader.c
+++ b/src/reader.c
@@ -69,8 +69,26 @@ static void *tryParentize(const redisReadTask *task, PyObject *obj) {
     PyObject *parent;
     if (task && task->parent) {
         parent = (PyObject*)task->parent->obj;
-        assert(PyList_CheckExact(parent));
-        PyList_SET_ITEM(parent, task->idx, obj);
+        switch (task->parent->type) {
+            case REDIS_REPLY_MAP:
+                if (task->idx % 2 == 0) {
+                    /* Set a temporary item to save the object as a key. */
+                    PyDict_SetItem(parent, obj, Py_None);
+                } else {
+                    /* Pop the temporary item and set proper key and value. */
+                    PyObject *last_item = PyObject_CallMethod(parent, "popitem", NULL);
+                    PyObject *last_key = PyTuple_GetItem(last_item, 0);
+                    PyDict_SetItem(parent, last_key, obj);
+                }
+                break;
+            case REDIS_REPLY_SET:
+                assert(PyAnySet_CheckExact(parent));
+                PySet_Add(parent, obj);
+                break;
+            default:
+                assert(PyList_CheckExact(parent));
+                PyList_SET_ITEM(parent, task->idx, obj);
+        }
     }
     return obj;
 }
@@ -127,14 +145,28 @@ static void *createStringObject(const redisReadTask *task, char *str, size_t len
             Py_INCREF(obj);
         }
     } else {
+        if (task->type == REDIS_REPLY_VERB) {
+            /* Skip 4 bytes of verbatim type header. */
+            memmove(str, str+4, len);
+            len -= 4;
+        }
         obj = createDecodedString(self, str, len);
     }
     return tryParentize(task, obj);
 }
 
-static void *createArrayObject(const redisReadTask *task, int elements) {
+static void *createArrayObject(const redisReadTask *task, size_t elements) {
     PyObject *obj;
-    obj = PyList_New(elements);
+    switch (task->type) {
+        case REDIS_REPLY_MAP:
+            obj = PyDict_New();
+            break;
+        case REDIS_REPLY_SET:
+            obj = PySet_New(NULL);
+            break;
+        default:
+            obj = PyList_New(elements);
+    }
     return tryParentize(task, obj);
 }
 
@@ -144,28 +176,42 @@ static void *createIntegerObject(const redisReadTask *task, long long value) {
     return tryParentize(task, obj);
 }
 
+static void *createDoubleObject(const redisReadTask *task, double value, char *str, size_t le) {
+    PyObject *obj;
+    obj = PyFloat_FromDouble(value);
+    return tryParentize(task, obj);
+}
+
 static void *createNilObject(const redisReadTask *task) {
     PyObject *obj = Py_None;
     Py_INCREF(obj);
     return tryParentize(task, obj);
 }
 
+static void *createBoolObject(const redisReadTask *task, int bval) {
+    PyObject *obj;
+    obj = PyBool_FromLong((long)bval);
+    return tryParentize(task, obj);
+}
+
 static void freeObject(void *obj) {
     Py_XDECREF(obj);
 }
 
 redisReplyObjectFunctions hiredis_ObjectFunctions = {
     createStringObject,  // void *(*createString)(const redisReadTask*, char*, size_t);
-    createArrayObject,   // void *(*createArray)(const redisReadTask*, int);
+    createArrayObject,   // void *(*createArray)(const redisReadTask*, size_t);
     createIntegerObject, // void *(*createInteger)(const redisReadTask*, long long);
+    createDoubleObject,  // void *(*createDoubleObject)(const redisReadTask*, double, char*, size_t);
     createNilObject,     // void *(*createNil)(const redisReadTask*);
+    createBoolObject,    // void *(*createBoolObject)(const redisReadTask*, int);
     freeObject           // void (*freeObject)(void*);
 };
 
 static void Reader_dealloc(hiredis_ReaderObject *self) {
     // we don't need to free self->encoding as the buffer is managed by Python
     // https://docs.python.org/3/c-api/arg.html#strings-and-buffers
-    redisReplyReaderFree(self->reader);
+    redisReaderFree(self->reader);
     Py_XDECREF(self->protocolErrorClass);
     Py_XDECREF(self->replyErrorClass);
 
@@ -289,7 +335,7 @@ static PyObject *Reader_feed(hiredis_ReaderObject *self, PyObject *args) {
       goto error;
     }
 
-    redisReplyReaderFeed(self->reader, (char *)buf.buf + off, len);
+    redisReaderFeed(self->reader, (char *)buf.buf + off, len);
     PyBuffer_Release(&buf);
     Py_RETURN_NONE;
 
@@ -308,8 +354,8 @@ static PyObject *Reader_gets(hiredis_ReaderObject *self, PyObject *args) {
         return NULL;
     }
 
-    if (redisReplyReaderGetReply(self->reader, (void**)&obj) == REDIS_ERR) {
-        errstr = redisReplyReaderGetError(self->reader);
+    if (redisReaderGetReply(self->reader, (void**)&obj) == REDIS_ERR) {
+        errstr = redisReaderGetError(self->reader);
         /* protocolErrorClass might be a callable. call it, then use it's type */
         err = createError(self->protocolErrorClass, errstr, strlen(errstr));
         if (err != NULL) {
diff --git a/vendor/hiredis b/vendor/hiredis
index 0107560..d5b4c69 160000
--- a/vendor/hiredis
+++ b/vendor/hiredis
@@ -1,1 +1,1 @@
-Subproject commit 010756025e8cefd1bc66c6d4ed3b1648ef6f1f95
+Subproject commit d5b4c69b7113213c1da3a0ccbfd1ee1b40443c7a


Test patch:
diff --git a/test/reader.py b/test/reader.py
index 5cea587..ea4f512 100644
--- a/test/reader.py
+++ b/test/reader.py
@@ -110,6 +110,42 @@ def test_integer(self):
     self.reader.feed((":%d\r\n" % value).encode("ascii"))
     self.assertEquals(value, self.reply())
 
+  def test_float(self):
+    value = -99.99
+    self.reader.feed(b",%f\r\n" % value)
+    self.assertEqual(value, self.reply())
+
+  def test_boolean_true(self):
+    self.reader.feed(b"#t\r\n")
+    self.assertTrue(self.reply())
+
+  def test_boolean_false(self):
+    self.reader.feed(b"#f\r\n")
+    self.assertFalse(False, self.reply())
+
+  def test_none(self):
+    self.reader.feed(b"_\r\n")
+    self.assertIsNone(self.reply())
+
+  def test_set(self):
+    self.reader.feed(b"~3\r\n+tangerine\r\n_\r\n,10.5\r\n")
+    self.assertEqual({b"tangerine", None, 10.5}, self.reply())
+
+  def test_dict(self):
+    self.reader.feed(b"%2\r\n+radius\r\n,4.5\r\n+diameter\r\n:9\r\n")
+    self.assertEqual({b"radius": 4.5, b"diameter": 9}, self.reply())
+
+  def test_vector(self):
+    self.reader.feed(b">4\r\n+pubsub\r\n+message\r\n+channel\r\n+message\r\n")
+    self.assertEqual(
+      [b"pubsub", b"message", b"channel", b"message"], self.reply()
+    )
+
+  def test_verbatim_string(self):
+    value = b"text"
+    self.reader.feed(b"=8\r\ntxt:%s\r\n" % value)
+    self.assertEqual(value, self.reply())
+
   def test_status_string(self):
     self.reader.feed(b"+ok\r\n")
     self.assertEquals(b"ok", self.reply())