Skip to content

Commit

Permalink
Be more strict about url scheme parsing (#2343)
Browse files Browse the repository at this point in the history
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.

Co-authored-by: dvora-h <[email protected]>
  • Loading branch information
vanschelven and dvora-h authored Aug 21, 2022
1 parent fde03fb commit b5ebada
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
17 changes: 11 additions & 6 deletions redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1166,6 +1166,16 @@ def to_bool(value):


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 = {}

Expand All @@ -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:
Expand All @@ -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

Expand Down
8 changes: 8 additions & 0 deletions tests/test_connection_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,14 @@ def test_invalid_scheme_raises_error(self):
"(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):
Expand Down

0 comments on commit b5ebada

Please sign in to comment.