Merge branch 'master' into 2014BDuck-master

4 files changed, 126 insertions, 238 deletions

diff --git a/CHANGES b/CHANGES
index b164991..2b7dbb8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,9 @@
 * (in development)
-    * Removed support for end of life Python 2.7.
+    * BACKWARDS INCOMPATIBLE: Removed support for end of life Python 2.7. #1318
+    * BACKWARDS INCOMPATIBLE: All values within Redis URLs are unquoted via
+      urllib.parse.unquote. Prior versions of redis-py supported this by
+      specifying the ``decode_components`` flag to the ``from_url`` functions.
+      This is now done by default and cannot be disabled. #589
     * Provide a development and testing environment via docker. Thanks
       @abrookins. #1365
     * Added support for the LPOS command available in Redis 6.0.6. Thanks
diff --git a/redis/client.py b/redis/client.py
index 25ba9cd..1560c96 100755
--- a/redis/client.py
+++ b/redis/client.py
@@ -685,7 +685,7 @@ class Redis:
     }
 
     @classmethod
-    def from_url(cls, url, db=None, **kwargs):
+    def from_url(cls, url, **kwargs):
         """
         Return a Redis client object configured from the given URL
 
@@ -697,28 +697,35 @@ class Redis:
 
         Three URL schemes are supported:
 
-        - ```redis://``
-          <http://www.iana.org/assignments/uri-schemes/prov/redis>`_ creates a
-          normal TCP socket connection
-        - ```rediss://``
-          <http://www.iana.org/assignments/uri-schemes/prov/rediss>`_ creates a
-          SSL wrapped TCP socket connection
-        - ``unix://`` creates a Unix Domain Socket connection
+        - `redis://` creates a TCP socket connection. See more at:
+          <https://www.iana.org/assignments/uri-schemes/prov/redis>
+        - `rediss://` creates a SSL wrapped TCP socket connection. See more at:
+          <https://www.iana.org/assignments/uri-schemes/prov/rediss>
+        - ``unix://``: creates a Unix Domain Socket connection.
 
-        There are several ways to specify a database number. The parse function
-        will return the first specified option:
+        The username, password, hostname, path and all querystring values
+        are passed through urllib.parse.unquote in order to replace any
+        percent-encoded values with their corresponding characters.
+
+        There are several ways to specify a database number. The first value
+        found will be used:
             1. A ``db`` querystring option, e.g. redis://localhost?db=0
-            2. If using the redis:// scheme, the path argument of the url, e.g.
-               redis://localhost/0
-            3. The ``db`` argument to this function.
+            2. If using the redis:// or rediss:// schemes, the path argument
+               of the url, e.g. redis://localhost/0
+            3. A ``db`` keyword argument to this function.
+
+        If none of these options are specified, the default db=0 is used.
 
-        If none of these options are specified, db=0 is used.
+        All querystring options are cast to their appropriate Python types.
+        Boolean arguments can be specified with string values "True"/"False"
+        or "Yes"/"No". Values that cannot be properly cast cause a
+        ``ValueError`` to be raised. Once parsed, the querystring arguments
+        and keyword arguments are passed to the ``ConnectionPool``'s
+        class initializer. In the case of conflicting arguments, querystring
+        arguments always win.
 
-        Any additional querystring arguments and keyword arguments will be
-        passed along to the ConnectionPool class's initializer. In the case
-        of conflicting arguments, querystring arguments always win.
         """
-        connection_pool = ConnectionPool.from_url(url, db=db, **kwargs)
+        connection_pool = ConnectionPool.from_url(url, **kwargs)
         return cls(connection_pool=connection_pool)
 
     def __init__(self, host='localhost', port=6379,
diff --git a/redis/connection.py b/redis/connection.py
index a29f9b2..4a855b3 100755
--- a/redis/connection.py
+++ b/redis/connection.py
@@ -919,6 +919,7 @@ def to_bool(value):
 
 
 URL_QUERY_ARGUMENT_PARSERS = {
+    'db': int,
     'socket_timeout': float,
     'socket_connect_timeout': float,
     'socket_keepalive': to_bool,
@@ -929,6 +930,59 @@ URL_QUERY_ARGUMENT_PARSERS = {
 }
 
 
+def parse_url(url):
+    url = urlparse(url)
+    kwargs = {}
+
+    for name, value in parse_qs(url.query).items():
+        if value and len(value) > 0:
+            value = unquote(value[0])
+            parser = URL_QUERY_ARGUMENT_PARSERS.get(name)
+            if parser:
+                try:
+                    kwargs[name] = parser(value)
+                except (TypeError, ValueError):
+                    raise ValueError(
+                        "Invalid value for `%s` in connection URL." % name
+                    )
+            else:
+                kwargs[name] = value
+
+    if url.username:
+        kwargs['username'] = unquote(url.username)
+    if url.password:
+        kwargs['password'] = unquote(url.password)
+
+    # We only support redis://, rediss:// and unix:// schemes.
+    if url.scheme == 'unix':
+        if url.path:
+            kwargs['path'] = unquote(url.path)
+        kwargs['connection_class'] = UnixDomainSocketConnection
+
+    elif url.scheme in ('redis', 'rediss'):
+        if url.hostname:
+            kwargs['host'] = unquote(url.hostname)
+        if url.port:
+            kwargs['port'] = int(url.port)
+
+        # If there's a path argument, use it as the db argument if a
+        # querystring value wasn't specified
+        if url.path and 'db' not in kwargs:
+            try:
+                kwargs['db'] = int(unquote(url.path).replace('/', ''))
+            except (AttributeError, ValueError):
+                pass
+
+        if url.scheme == 'rediss':
+            kwargs['connection_class'] = SSLConnection
+    else:
+        valid_schemes = 'redis://, rediss://, unix://'
+        raise ValueError('Redis URL must specify one of the following '
+                         'schemes (%s)' % valid_schemes)
+
+    return kwargs
+
+
 class ConnectionPool:
     """
     Create a connection pool. ``If max_connections`` is set, then this
@@ -943,7 +997,7 @@ class ConnectionPool:
     ``connection_class``.
     """
     @classmethod
-    def from_url(cls, url, db=None, decode_components=False, **kwargs):
+    def from_url(cls, url, **kwargs):
         """
         Return a connection pool configured from the given URL.
 
@@ -955,114 +1009,35 @@ class ConnectionPool:
 
         Three URL schemes are supported:
 
-        - `redis://
-          <https://www.iana.org/assignments/uri-schemes/prov/redis>`_ creates a
-          normal TCP socket connection
-        - `rediss://
-          <https://www.iana.org/assignments/uri-schemes/prov/rediss>`_ creates
-          a SSL wrapped TCP socket connection
-        - ``unix://`` creates a Unix Domain Socket connection
+        - `redis://` creates a TCP socket connection. See more at:
+          <https://www.iana.org/assignments/uri-schemes/prov/redis>
+        - `rediss://` creates a SSL wrapped TCP socket connection. See more at:
+          <https://www.iana.org/assignments/uri-schemes/prov/rediss>
+        - ``unix://``: creates a Unix Domain Socket connection.
 
-        There are several ways to specify a database number. The parse function
-        will return the first specified option:
-            1. A ``db`` querystring option, e.g. redis://localhost?db=0
-            2. If using the redis:// scheme, the path argument of the url, e.g.
-               redis://localhost/0
-            3. The ``db`` argument to this function.
-
-        If none of these options are specified, db=0 is used.
-
-        The ``decode_components`` argument allows this function to work with
-        percent-encoded URLs. If this argument is set to ``True`` all ``%xx``
-        escapes will be replaced by their single-character equivalents after
-        the URL has been parsed. This only applies to the ``hostname``,
-        ``path``, ``username`` and ``password`` components.
-
-        Any additional querystring arguments and keyword arguments will be
-        passed along to the ConnectionPool class's initializer. The querystring
-        arguments ``socket_connect_timeout`` and ``socket_timeout`` if supplied
-        are parsed as float values. The arguments ``socket_keepalive`` and
-        ``retry_on_timeout`` are parsed to boolean values that accept
-        True/False, Yes/No values to indicate state. Invalid types cause a
-        ``UserWarning`` to be raised. In the case of conflicting arguments,
-        querystring arguments always win.
+        The username, password, hostname, path and all querystring values
+        are passed through urllib.parse.unquote in order to replace any
+        percent-encoded values with their corresponding characters.
 
+        There are several ways to specify a database number. The first value
+        found will be used:
+            1. A ``db`` querystring option, e.g. redis://localhost?db=0
+            2. If using the redis:// or rediss:// schemes, the path argument
+               of the url, e.g. redis://localhost/0
+            3. A ``db`` keyword argument to this function.
+
+        If none of these options are specified, the default db=0 is used.
+
+        All querystring options are cast to their appropriate Python types.
+        Boolean arguments can be specified with string values "True"/"False"
+        or "Yes"/"No". Values that cannot be properly cast cause a
+        ``ValueError`` to be raised. Once parsed, the querystring arguments
+        and keyword arguments are passed to the ``ConnectionPool``'s
+        class initializer. In the case of conflicting arguments, querystring
+        arguments always win.
         """
-        url = urlparse(url)
-        url_options = {}
-
-        for name, value in parse_qs(url.query).items():
-            if value and len(value) > 0:
-                parser = URL_QUERY_ARGUMENT_PARSERS.get(name)
-                if parser:
-                    try:
-                        url_options[name] = parser(value[0])
-                    except (TypeError, ValueError):
-                        warnings.warn(UserWarning(
-                            "Invalid value for `%s` in connection URL." % name
-                        ))
-                else:
-                    url_options[name] = value[0]
-
-        if decode_components:
-            username = unquote(url.username) if url.username else None
-            password = unquote(url.password) if url.password else None
-            path = unquote(url.path) if url.path else None
-            hostname = unquote(url.hostname) if url.hostname else None
-        else:
-            username = url.username or None
-            password = url.password or None
-            path = url.path
-            hostname = url.hostname
-
-        # We only support redis://, rediss:// and unix:// schemes.
-        if url.scheme == 'unix':
-            url_options.update({
-                'username': username,
-                'password': password,
-                'path': path,
-                'connection_class': UnixDomainSocketConnection,
-            })
-
-        elif url.scheme in ('redis', 'rediss'):
-            url_options.update({
-                'host': hostname,
-                'port': int(url.port or 6379),
-                'username': username,
-                'password': password,
-            })
-
-            # If there's a path argument, use it as the db argument if a
-            # querystring value wasn't specified
-            if 'db' not in url_options and path:
-                try:
-                    url_options['db'] = int(path.replace('/', ''))
-                except (AttributeError, ValueError):
-                    pass
-
-            if url.scheme == 'rediss':
-                url_options['connection_class'] = SSLConnection
-        else:
-            valid_schemes = ', '.join(('redis://', 'rediss://', 'unix://'))
-            raise ValueError('Redis URL must specify one of the following '
-                             'schemes (%s)' % valid_schemes)
-
-        # last shot at the db value
-        url_options['db'] = int(url_options.get('db', db or 0))
-
-        # update the arguments from the URL values
+        url_options = parse_url(url)
         kwargs.update(url_options)
-
-        # backwards compatability
-        if 'charset' in kwargs:
-            warnings.warn(DeprecationWarning(
-                '"charset" is deprecated. Use "encoding" instead'))
-            kwargs['encoding'] = kwargs.pop('charset')
-        if 'errors' in kwargs:
-            warnings.warn(DeprecationWarning(
-                '"errors" is deprecated. Use "encoding_errors" instead'))
-            kwargs['encoding_errors'] = kwargs.pop('errors')
-
         return cls(**kwargs)
 
     def __init__(self, connection_class=Connection, max_connections=None,
diff --git a/tests/test_connection_pool.py b/tests/test_connection_pool.py
index c26090b..7f9d054 100644
--- a/tests/test_connection_pool.py
+++ b/tests/test_connection_pool.py
@@ -160,7 +160,6 @@ class TestBlockingConnectionPool:
         pool = redis.ConnectionPool(
             host='localhost',
             port=6379,
-            db=0,
             client_name='test-client'
         )
         expected = ('ConnectionPool<Connection<'
@@ -171,7 +170,6 @@ class TestBlockingConnectionPool:
         pool = redis.ConnectionPool(
             connection_class=redis.UnixDomainSocketConnection,
             path='abc',
-            db=0,
             client_name='test-client'
         )
         expected = ('ConnectionPool<UnixDomainSocketConnection<'
@@ -180,38 +178,18 @@ class TestBlockingConnectionPool:
 
 
 class TestConnectionPoolURLParsing:
-    def test_defaults(self):
-        pool = redis.ConnectionPool.from_url('redis://localhost')
-        assert pool.connection_class == redis.Connection
-        assert pool.connection_kwargs == {
-            'host': 'localhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
-            'password': None,
-        }
-
     def test_hostname(self):
-        pool = redis.ConnectionPool.from_url('redis://myhost')
+        pool = redis.ConnectionPool.from_url('redis://my.host')
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
-            'host': 'myhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
-            'password': None,
+            'host': 'my.host',
         }
 
     def test_quoted_hostname(self):
-        pool = redis.ConnectionPool.from_url('redis://my %2F host %2B%3D+',
-                                             decode_components=True)
+        pool = redis.ConnectionPool.from_url('redis://my %2F host %2B%3D+')
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'my / host +=+',
-            'port': 6379,
-            'db': 0,
-            'username': None,
-            'password': None,
         }
 
     def test_port(self):
@@ -220,9 +198,6 @@ class TestConnectionPoolURLParsing:
         assert pool.connection_kwargs == {
             'host': 'localhost',
             'port': 6380,
-            'db': 0,
-            'username': None,
-            'password': None,
         }
 
     @skip_if_server_version_lt(REDIS_6_VERSION)
@@ -231,24 +206,17 @@ class TestConnectionPoolURLParsing:
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
-            'db': 0,
             'username': 'myuser',
-            'password': None,
         }
 
     @skip_if_server_version_lt(REDIS_6_VERSION)
     def test_quoted_username(self):
         pool = redis.ConnectionPool.from_url(
-            'redis://%2Fmyuser%2F%2B name%3D%24+:@localhost',
-            decode_components=True)
+            'redis://%2Fmyuser%2F%2B name%3D%24+:@localhost')
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
-            'db': 0,
             'username': '/myuser/+ name=$+',
-            'password': None,
         }
 
     def test_password(self):
@@ -256,22 +224,15 @@ class TestConnectionPoolURLParsing:
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
             'password': 'mypassword',
         }
 
     def test_quoted_password(self):
         pool = redis.ConnectionPool.from_url(
-            'redis://:%2Fmypass%2F%2B word%3D%24+@localhost',
-            decode_components=True)
+            'redis://:%2Fmypass%2F%2B word%3D%24+@localhost')
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
             'password': '/mypass/+ word=$+',
         }
 
@@ -281,44 +242,33 @@ class TestConnectionPoolURLParsing:
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
-            'db': 0,
             'username': 'myuser',
             'password': 'mypass',
         }
 
     def test_db_as_argument(self):
-        pool = redis.ConnectionPool.from_url('redis://localhost', db='1')
+        pool = redis.ConnectionPool.from_url('redis://localhost', db=1)
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
             'db': 1,
-            'username': None,
-            'password': None,
         }
 
     def test_db_in_path(self):
-        pool = redis.ConnectionPool.from_url('redis://localhost/2', db='1')
+        pool = redis.ConnectionPool.from_url('redis://localhost/2', db=1)
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
             'db': 2,
-            'username': None,
-            'password': None,
         }
 
     def test_db_in_querystring(self):
         pool = redis.ConnectionPool.from_url('redis://localhost/2?db=3',
-                                             db='1')
+                                             db=1)
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
             'db': 3,
-            'username': None,
-            'password': None,
         }
 
     def test_extra_typed_querystring_options(self):
@@ -330,13 +280,10 @@ class TestConnectionPoolURLParsing:
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
             'db': 2,
             'socket_timeout': 20.0,
             'socket_connect_timeout': 10.0,
             'retry_on_timeout': True,
-            'username': None,
-            'password': None,
         }
         assert pool.max_connections == 10
 
@@ -359,30 +306,17 @@ class TestConnectionPoolURLParsing:
         assert pool.connection_kwargs['client_name'] == 'test-client'
 
     def test_invalid_extra_typed_querystring_options(self):
-        import warnings
-        with warnings.catch_warnings(record=True) as warning_log:
+        with pytest.raises(ValueError):
             redis.ConnectionPool.from_url(
                 'redis://localhost/2?socket_timeout=_&'
                 'socket_connect_timeout=abc'
             )
-        # Compare the message values
-        assert [
-            str(m.message) for m in
-            sorted(warning_log, key=lambda l: str(l.message))
-        ] == [
-            'Invalid value for `socket_connect_timeout` in connection URL.',
-            'Invalid value for `socket_timeout` in connection URL.',
-        ]
 
     def test_extra_querystring_options(self):
         pool = redis.ConnectionPool.from_url('redis://localhost?a=1&b=2')
         assert pool.connection_class == redis.Connection
         assert pool.connection_kwargs == {
             'host': 'localhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
-            'password': None,
             'a': '1',
             'b': '2'
         }
@@ -396,10 +330,6 @@ class TestConnectionPoolURLParsing:
         assert r.connection_pool.connection_class == redis.Connection
         assert r.connection_pool.connection_kwargs == {
             'host': 'myhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
-            'password': None,
         }
 
     def test_invalid_scheme_raises_error(self):
@@ -417,9 +347,6 @@ class TestConnectionPoolUnixSocketURLParsing:
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/socket',
-            'db': 0,
-            'username': None,
-            'password': None,
         }
 
     @skip_if_server_version_lt(REDIS_6_VERSION)
@@ -428,22 +355,17 @@ class TestConnectionPoolUnixSocketURLParsing:
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/socket',
-            'db': 0,
             'username': 'myuser',
-            'password': None,
         }
 
     @skip_if_server_version_lt(REDIS_6_VERSION)
     def test_quoted_username(self):
         pool = redis.ConnectionPool.from_url(
-            'unix://%2Fmyuser%2F%2B name%3D%24+:@/socket',
-            decode_components=True)
+            'unix://%2Fmyuser%2F%2B name%3D%24+:@/socket')
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/socket',
-            'db': 0,
             'username': '/myuser/+ name=$+',
-            'password': None,
         }
 
     def test_password(self):
@@ -451,32 +373,24 @@ class TestConnectionPoolUnixSocketURLParsing:
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/socket',
-            'db': 0,
-            'username': None,
             'password': 'mypassword',
         }
 
     def test_quoted_password(self):
         pool = redis.ConnectionPool.from_url(
-            'unix://:%2Fmypass%2F%2B word%3D%24+@/socket',
-            decode_components=True)
+            'unix://:%2Fmypass%2F%2B word%3D%24+@/socket')
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/socket',
-            'db': 0,
-            'username': None,
             'password': '/mypass/+ word=$+',
         }
 
     def test_quoted_path(self):
         pool = redis.ConnectionPool.from_url(
-            'unix://:mypassword@/my%2Fpath%2Fto%2F..%2F+_%2B%3D%24ocket',
-            decode_components=True)
+            'unix://:mypassword@/my%2Fpath%2Fto%2F..%2F+_%2B%3D%24ocket')
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/my/path/to/../+_+=$ocket',
-            'db': 0,
-            'username': None,
             'password': 'mypassword',
         }
 
@@ -486,8 +400,6 @@ class TestConnectionPoolUnixSocketURLParsing:
         assert pool.connection_kwargs == {
             'path': '/socket',
             'db': 1,
-            'username': None,
-            'password': None,
         }
 
     def test_db_in_querystring(self):
@@ -496,8 +408,6 @@ class TestConnectionPoolUnixSocketURLParsing:
         assert pool.connection_kwargs == {
             'path': '/socket',
             'db': 2,
-            'username': None,
-            'password': None,
         }
 
     def test_client_name_in_querystring(self):
@@ -511,28 +421,20 @@ class TestConnectionPoolUnixSocketURLParsing:
         assert pool.connection_class == redis.UnixDomainSocketConnection
         assert pool.connection_kwargs == {
             'path': '/socket',
-            'db': 0,
-            'username': None,
-            'password': None,
             'a': '1',
             'b': '2'
         }
 
 
+@pytest.mark.skipif(not ssl_available, reason="SSL not installed")
 class TestSSLConnectionURLParsing:
-    @pytest.mark.skipif(not ssl_available, reason="SSL not installed")
-    def test_defaults(self):
-        pool = redis.ConnectionPool.from_url('rediss://localhost')
+    def test_host(self):
+        pool = redis.ConnectionPool.from_url('rediss://my.host')
         assert pool.connection_class == redis.SSLConnection
         assert pool.connection_kwargs == {
-            'host': 'localhost',
-            'port': 6379,
-            'db': 0,
-            'username': None,
-            'password': None,
+            'host': 'my.host',
         }
 
-    @pytest.mark.skipif(not ssl_available, reason="SSL not installed")
     def test_cert_reqs_options(self):
         import ssl