Repository: redis/redis-py

Issue:
resp3 parser async parser incorrectly parses maps
Issue discovered in pr #2947

the evaulation order of `key:val` in `{key():val() for foo in bar}` dict comprehension is undefined in Python.  It mostly is left-right,
but some versions, notably recent 3.7 versions, don't behave that way.  It is best to use explicit ordering when reading from  a stream.

This had already been fixed for __non-async__ parsing, but in pr #2947 caused some scheduling behaviour which triggered this to happen. A fix for __async__ has been added there.


Discussion:
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/redis/redis-py/pull/2982?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) Report
Attention: `5 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`5391c5f`)](https://app.codecov.io/gh/redis/redis-py/commit/5391c5fc39a2c9aaa1b2e5acc799b93444270b88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) 91.36% compared to head [(`8ae4f6d`)](https://app.codecov.io/gh/redis/redis-py/pull/2982?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) 77.71%.

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           master    #2982       +/-   ##
===========================================
- Coverage   91.36%   77.71%   -13.66%     
===========================================
  Files         126      126               
  Lines       32617    32621        +4     
===========================================
- Hits        29801    25350     -4451     
- Misses       2816     7271     +4455     
```


| [Files](https://app.codecov.io/gh/redis/redis-py/pull/2982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | Coverage Δ | |
|---|---|---|
| [redis/\_parsers/resp3.py](https://app.codecov.io/gh/redis/redis-py/pull/2982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvX3BhcnNlcnMvcmVzcDMucHk=) | `46.25% <0.00%> (-1.30%)` | :arrow_down: |

... and [27 files with indirect coverage changes](https://app.codecov.io/gh/redis/redis-py/pull/2982/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/redis/redis-py/pull/2982?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).


Patch:
diff --git a/redis/_parsers/resp3.py b/redis/_parsers/resp3.py
index ad766a8..569e7ee 100644
--- a/redis/_parsers/resp3.py
+++ b/redis/_parsers/resp3.py
@@ -96,8 +96,9 @@ class _RESP3Parser(_RESPBase):
                 pass
         # map response
         elif byte == b"%":
-            # we use this approach and not dict comprehension here
-            # because this dict comprehension fails in python 3.7
+            # We cannot use a dict-comprehension to parse stream.
+            # Evaluation order of key:val expression in dict comprehension only
+            # became defined to be left-right in version 3.8
             resp_dict = {}
             for _ in range(int(response)):
                 key = self._read_response(disable_decoding=disable_decoding)
@@ -225,12 +226,16 @@ class _AsyncRESP3Parser(_AsyncRESPBase):
                 pass
         # map response
         elif byte == b"%":
-            response = {
-                (await self._read_response(disable_decoding=disable_decoding)): (
-                    await self._read_response(disable_decoding=disable_decoding)
+            # We cannot use a dict-comprehension to parse stream.
+            # Evaluation order of key:val expression in dict comprehension only
+            # became defined to be left-right in version 3.8
+            resp_dict = {}
+            for _ in range(int(response)):
+                key = await self._read_response(disable_decoding=disable_decoding)
+                resp_dict[key] = await self._read_response(
+                    disable_decoding=disable_decoding, push_request=push_request
                 )
-                for _ in range(int(response))
-            }
+            response = resp_dict
         # push response
         elif byte == b">":
             response = [
diff --git a/redis/asyncio/connection.py b/redis/asyncio/connection.py
index 7b04434..7731221 100644
--- a/redis/asyncio/connection.py
+++ b/redis/asyncio/connection.py
@@ -880,6 +880,7 @@ URL_QUERY_ARGUMENT_PARSERS: Mapping[str, Callable[..., object]] = MappingProxyTy
         "max_connections": int,
         "health_check_interval": int,
         "ssl_check_hostname": to_bool,
+        "timeout": float,
     }
 )
 
diff --git a/redis/connection.py b/redis/connection.py
index b39ba28..fead613 100644
--- a/redis/connection.py
+++ b/redis/connection.py
@@ -853,6 +853,7 @@ URL_QUERY_ARGUMENT_PARSERS = {
     "max_connections": int,
     "health_check_interval": int,
     "ssl_check_hostname": to_bool,
+    "timeout": float,
 }
 
 


Test patch:
diff --git a/tests/test_asyncio/test_connection_pool.py b/tests/test_asyncio/test_connection_pool.py
index c93fa91..ed90fc7 100644
--- a/tests/test_asyncio/test_connection_pool.py
+++ b/tests/test_asyncio/test_connection_pool.py
@@ -454,6 +454,31 @@ class TestConnectionPoolURLParsing:
         )
 
 
+class TestBlockingConnectionPoolURLParsing:
+    def test_extra_typed_querystring_options(self):
+        pool = redis.BlockingConnectionPool.from_url(
+            "redis://localhost/2?socket_timeout=20&socket_connect_timeout=10"
+            "&socket_keepalive=&retry_on_timeout=Yes&max_connections=10&timeout=13.37"
+        )
+
+        assert pool.connection_class == redis.Connection
+        assert pool.connection_kwargs == {
+            "host": "localhost",
+            "db": 2,
+            "socket_timeout": 20.0,
+            "socket_connect_timeout": 10.0,
+            "retry_on_timeout": True,
+        }
+        assert pool.max_connections == 10
+        assert pool.timeout == 13.37
+
+    def test_invalid_extra_typed_querystring_options(self):
+        with pytest.raises(ValueError):
+            redis.BlockingConnectionPool.from_url(
+                "redis://localhost/2?timeout=_not_a_float_"
+            )
+
+
 class TestConnectionPoolUnixSocketURLParsing:
     def test_defaults(self):
         pool = redis.ConnectionPool.from_url("unix:///socket")
diff --git a/tests/test_connection_pool.py b/tests/test_connection_pool.py
index ef70a8f..d1e984e 100644
--- a/tests/test_connection_pool.py
+++ b/tests/test_connection_pool.py
@@ -359,6 +359,31 @@ class TestConnectionPoolURLParsing:
         )
 
 
+class TestBlockingConnectionPoolURLParsing:
+    def test_extra_typed_querystring_options(self):
+        pool = redis.BlockingConnectionPool.from_url(
+            "redis://localhost/2?socket_timeout=20&socket_connect_timeout=10"
+            "&socket_keepalive=&retry_on_timeout=Yes&max_connections=10&timeout=42"
+        )
+
+        assert pool.connection_class == redis.Connection
+        assert pool.connection_kwargs == {
+            "host": "localhost",
+            "db": 2,
+            "socket_timeout": 20.0,
+            "socket_connect_timeout": 10.0,
+            "retry_on_timeout": True,
+        }
+        assert pool.max_connections == 10
+        assert pool.timeout == 42.0
+
+    def test_invalid_extra_typed_querystring_options(self):
+        with pytest.raises(ValueError):
+            redis.BlockingConnectionPool.from_url(
+                "redis://localhost/2?timeout=_not_a_float_"
+            )
+
+
 class TestConnectionPoolUnixSocketURLParsing:
     def test_defaults(self):
         pool = redis.ConnectionPool.from_url("unix:///socket")