Repository: redis/redis-py
Pull request #2343 (2022-08-19 07:53:51)

Be more strict about url scheme parsing
### Pull Request check-list

_Please make sure to review and check all of these items:_

- [ ] Does `$ tox` pass with this change (including linting)?
- [ ] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
- [x] Is the new or changed code fully tested?
- [ ] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
- [ ] Is there an example added to the examples folder (if applicable)?
- [ ] Was the change added to CHANGES file?

_NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open._

### Description of change

The error message implied that urls had to start with `scheme://`.
However, if the double slash was left out, the url parsed just fine
and the part that was ostensibly intended to be the hostname ended
up as part of the path, whereas the default (localhost) would be
used for the hostname. This commit makes the check as strict as the
error message implies by including a check for the double slash.

vanschelven: I think the pipeline troubles are independent of this fix (see https://github.com/redis/redis-py/issues/2344)
codecov-commenter: # [Codecov](https://codecov.io/gh/redis/redis-py/pull/2343?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) Report
> Merging [#2343](https://codecov.io/gh/redis/redis-py/pull/2343?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) (4b6d68e) into [master](https://codecov.io/gh/redis/redis-py/commit/031b2087b57713d29b98140fb1f0b9f7b4fc7a21?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) (031b208) will **decrease** coverage by `13.61%`.
> The diff coverage is `100.00%`.

```diff
@@             Coverage Diff             @@
##           master    #2343       +/-   ##
===========================================
- Coverage   92.19%   78.57%   -13.62%     
===========================================
  Files         111      111               
  Lines       28777    28781        +4     
===========================================
- Hits        26530    22616     -3914     
- Misses       2247     6165     +3918     
```


| [Impacted Files](https://codecov.io/gh/redis/redis-py/pull/2343?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | Coverage Δ | |
|---|---|---|
| [redis/connection.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY29ubmVjdGlvbi5weQ==) | `77.73% <100.00%> (-9.17%)` | :arrow_down: |
| [tests/test\_connection\_pool.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-dGVzdHMvdGVzdF9jb25uZWN0aW9uX3Bvb2wucHk=) | `100.00% <100.00%> (ø)` | |
| [tests/test\_asyncio/test\_cluster.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-dGVzdHMvdGVzdF9hc3luY2lvL3Rlc3RfY2x1c3Rlci5weQ==) | `18.88% <0.00%> (-78.40%)` | :arrow_down: |
| [tests/test\_cluster.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-dGVzdHMvdGVzdF9jbHVzdGVyLnB5) | `18.58% <0.00%> (-78.27%)` | :arrow_down: |
| [redis/asyncio/cluster.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvYXN5bmNpby9jbHVzdGVyLnB5) | `16.52% <0.00%> (-73.77%)` | :arrow_down: |
| [redis/cluster.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY2x1c3Rlci5weQ==) | `18.81% <0.00%> (-71.04%)` | :arrow_down: |
| [redis/asyncio/parser.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvYXN5bmNpby9wYXJzZXIucHk=) | `17.30% <0.00%> (-63.47%)` | :arrow_down: |
| [redis/commands/cluster.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY29tbWFuZHMvY2x1c3Rlci5weQ==) | `37.57% <0.00%> (-56.07%)` | :arrow_down: |
| [redis/crc.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY3JjLnB5) | `45.45% <0.00%> (-54.55%)` | :arrow_down: |
| [redis/utils.py](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvdXRpbHMucHk=) | `72.97% <0.00%> (-16.22%)` | :arrow_down: |
| ... and [27 more](https://codecov.io/gh/redis/redis-py/pull/2343/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis)


Patch:
diff --git a/README.md b/README.md
index 80e2cc2e..f5ad9a41 100644
--- a/README.md
+++ b/README.md
@@ -63,7 +63,7 @@ Alternatively, you might want to look at [Async connections](https://redis.readt
 
 ### Redis Commands
 
-There is built-in support for all of the [out-of-the-box Redis commands](https://redis.io/commands). They are exposed using the raw Redis command names (`HSET`, `HGETALL`, etc.) except where a work (i.e del) is reserved by the language. The complete set of commands can be found [here](https://github.com/redis/redis-py/tree/master/redis/commands), or [the documentation](https://redis.readthedocs.io/en/stable/commands.html).
+There is built-in support for all of the [out-of-the-box Redis commands](https://redis.io/commands). They are exposed using the raw Redis command names (`HSET`, `HGETALL`, etc.) except where a word (i.e del) is reserved by the language. The complete set of commands can be found [here](https://github.com/redis/redis-py/tree/master/redis/commands), or [the documentation](https://redis.readthedocs.io/en/stable/commands.html).
 
 ## Advanced Topics
 
diff --git a/redis/connection.py b/redis/connection.py
index 3438bafe..96645ba3 100755
--- a/redis/connection.py
+++ b/redis/connection.py
@@ -1166,6 +1166,16 @@ URL_QUERY_ARGUMENT_PARSERS = {
 
 
 def parse_url(url):
+    if not (
+        url.startswith("redis://")
+        or url.startswith("rediss://")
+        or url.startswith("unix://")
+    ):
+        raise ValueError(
+            "Redis URL must specify one of the following "
+            "schemes (redis://, rediss://, unix://)"
+        )
+
     url = urlparse(url)
     kwargs = {}
 
@@ -1192,7 +1202,7 @@ def parse_url(url):
             kwargs["path"] = unquote(url.path)
         kwargs["connection_class"] = UnixDomainSocketConnection
 
-    elif url.scheme in ("redis", "rediss"):
+    else:  # implied:  url.scheme in ("redis", "rediss"):
         if url.hostname:
             kwargs["host"] = unquote(url.hostname)
         if url.port:
@@ -1208,11 +1218,6 @@ def parse_url(url):
 
         if url.scheme == "rediss":
             kwargs["connection_class"] = SSLConnection
-    else:
-        raise ValueError(
-            "Redis URL must specify one of the following "
-            "schemes (redis://, rediss://, unix://)"
-        )
 
     return kwargs
 


Test patch:
diff --git a/tests/test_connection_pool.py b/tests/test_connection_pool.py
index a836f5b2..636b04ef 100644
--- a/tests/test_connection_pool.py
+++ b/tests/test_connection_pool.py
@@ -339,6 +339,14 @@ class TestConnectionPoolURLParsing:
             "(redis://, rediss://, unix://)"
         )
 
+    def test_invalid_scheme_raises_error_when_double_slash_missing(self):
+        with pytest.raises(ValueError) as cm:
+            redis.ConnectionPool.from_url("redis:foo.bar.com:12345")
+        assert str(cm.value) == (
+            "Redis URL must specify one of the following schemes "
+            "(redis://, rediss://, unix://)"
+        )
+
 
 class TestConnectionPoolUnixSocketURLParsing:
     def test_defaults(self):