- adjustment, the spec says: "Within the user and password field, any ":",

   "@", or "/" must be encoded." - so re-apply encoding to both password
and username, don't encode spaces as plus signs, don't encode any chars
outside of :, @, / on stringification - but we still parse for any
%XX character (is that right?)

6 files changed, 46 insertions, 34 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 571f11b2f..d118684d3 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -19,10 +19,11 @@
         :tickets: 2873
 
         The :func:`.create_engine` routine and the related
-        :func:`.make_url` function **no longer URL encode the password**.
-        Database passwords that include characters like spaces, plus signs
-        and anything else should now represent these characters directly,
-        without any URL escaping.
+        :func:`.make_url` function no longer considers the ``+`` sign
+        to be a space within the password field.  The parsing has been
+        adjuted to match RFC 1738 exactly, in that both ``username``
+        and ``password`` expect only ``:``, ``@``, and ``/`` to be
+        encoded.
 
         .. seealso::
 
diff --git a/doc/build/changelog/migration_09.rst b/doc/build/changelog/migration_09.rst
index 2d490e912..e5e6e4612 100644
--- a/doc/build/changelog/migration_09.rst
+++ b/doc/build/changelog/migration_09.rst
@@ -474,31 +474,29 @@ Behavioral Changes - Core
 
 .. _migration_2873:
 
-The "password" portion of a ``create_engine()`` URL is no longer URL encoded
-----------------------------------------------------------------------------
+The "password" portion of a ``create_engine()`` no longer considers the ``+`` sign as an encoded space
+------------------------------------------------------------------------------------------------------
 
 For whatever reason, the Python function ``unquote_plus()`` was applied to the
-"password" field of a URL, likely as a means of allowing the usage of escapes
-(e.g. "%2F" or similar) to be used, and perhaps as some way of allowing spaces
-to be present.  However, this is not complaint with `RFC 1738 <http://www.ietf.org/rfc/rfc1738.txt>`_
-which has no reserved characters within the password field and does not specify
-URL quoting - so the quote_plus routines are **no longer applied** to the password
-field.
-
-Examples of URLs with characters such as colons, @ symbols, spaces, and plus signs
-include::
+"password" field of a URL, which is an incorrect application of the
+encoding rules described in `RFC 1738 <http://www.ietf.org/rfc/rfc1738.txt>`_
+in that it escaped spaces as plus signs.  The stringiciation of a URL
+now only encodes ":", "@", or "/" and nothing else, and is now applied to both the
+``username`` and ``password`` fields (previously it only applied to the
+password).   On parsing, encoded characters are converted, but plus signs and
+spaces are passed through as is::
 
     # password: "pass word + other:words"
-    dbtype://user:pass word + other:words@host/dbname
+    dbtype://user:pass word + other%3Awords@host/dbname
 
-    # password: "apples%2Foranges"
+    # password: "apples/oranges"
     dbtype://username:apples%2Foranges@hostspec/database
 
     # password: "apples@oranges@@"
-    dbtype://username:apples@oranges@@@hostspec/database
+    dbtype://username:apples%40oranges%40%40@hostspec/database
 
     # password: '', username is "username@"
-    dbtype://username@:@hostspec/database
+    dbtype://username%40:@hostspec/database
 
 
 :ticket:`2873`
diff --git a/lib/sqlalchemy/engine/url.py b/lib/sqlalchemy/engine/url.py
index 28c15299e..77fbe2346 100644
--- a/lib/sqlalchemy/engine/url.py
+++ b/lib/sqlalchemy/engine/url.py
@@ -65,9 +65,10 @@ class URL(object):
     def __to_string__(self, hide_password=True):
         s = self.drivername + "://"
         if self.username is not None:
-            s += self.username
+            s += _rfc_1738_quote(self.username)
             if self.password is not None:
-                s += ':' + ('***' if hide_password else self.password)
+                s += ':' + ('***' if hide_password
+                            else _rfc_1738_quote(self.password))
             s += "@"
         if self.host is not None:
             if ':' in self.host:
@@ -194,6 +195,12 @@ def _parse_rfc1738_args(name):
             query = None
         components['query'] = query
 
+        if components['username'] is not None:
+            components['username'] = _rfc_1738_unquote(components['username'])
+
+        if components['password'] is not None:
+            components['password'] = _rfc_1738_unquote(components['password'])
+
         ipv4host = components.pop('ipv4host')
         ipv6host = components.pop('ipv6host')
         components['host'] = ipv4host or ipv6host
@@ -204,6 +211,12 @@ def _parse_rfc1738_args(name):
             "Could not parse rfc1738 URL from string '%s'" % name)
 
 
+def _rfc_1738_quote(text):
+    return re.sub(r'[:@/]', lambda m: "%%%X" % ord(m.group(0)), text)
+
+def _rfc_1738_unquote(text):
+    return util.unquote(text)
+
 def _parse_keyvalue_args(name):
     m = re.match(r'(\w+)://(.*)', name)
     if m is not None:
diff --git a/lib/sqlalchemy/util/__init__.py b/lib/sqlalchemy/util/__init__.py
index c68a64866..77339e56a 100644
--- a/lib/sqlalchemy/util/__init__.py
+++ b/lib/sqlalchemy/util/__init__.py
@@ -9,7 +9,7 @@ from .compat import callable, cmp, reduce,  \
     pickle, dottedgetter, parse_qsl, namedtuple, next, reraise, \
     raise_from_cause, text_type, string_types, int_types, binary_type, \
     quote_plus, with_metaclass, print_, itertools_filterfalse, u, ue, b,\
-    unquote_plus, b64decode, b64encode, byte_buffer, itertools_filter,\
+    unquote_plus, unquote, b64decode, b64encode, byte_buffer, itertools_filter,\
     iterbytes, StringIO, inspect_getargspec
 
 from ._collections import KeyedTuple, ImmutableContainer, immutabledict, \
diff --git a/lib/sqlalchemy/util/compat.py b/lib/sqlalchemy/util/compat.py
index 7c2bc88d4..ff434df43 100644
--- a/lib/sqlalchemy/util/compat.py
+++ b/lib/sqlalchemy/util/compat.py
@@ -40,7 +40,7 @@ if py3k:
     import builtins
 
     from inspect import getfullargspec as inspect_getfullargspec
-    from urllib.parse import quote_plus, unquote_plus, parse_qsl
+    from urllib.parse import quote_plus, unquote_plus, parse_qsl, quote, unquote
     import configparser
     from io import StringIO
 
@@ -95,7 +95,7 @@ if py3k:
 else:
     from inspect import getargspec as inspect_getfullargspec
     inspect_getargspec = inspect_getfullargspec
-    from urllib import quote_plus, unquote_plus
+    from urllib import quote_plus, unquote_plus, quote, unquote
     from urlparse import parse_qsl
     import ConfigParser as configparser
     from StringIO import StringIO
diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py
index d1ffe426d..0ae747b9c 100644
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -31,7 +31,7 @@ class ParseConnectTest(fixtures.TestBase):
             'dbtype://',
             'dbtype://username:password@/database',
             'dbtype:////usr/local/_xtest@example.com/members.db',
-            'dbtype://username:apples/oranges@hostspec/database',
+            'dbtype://username:apples%2Foranges@hostspec/database',
             'dbtype://username:password@[2001:da8:2004:1000:202:116:160:90]/database?foo=bar',
             'dbtype://username:password@[2001:da8:2004:1000:202:116:160:90]:80/database?foo=bar'
             ):
@@ -50,26 +50,26 @@ class ParseConnectTest(fixtures.TestBase):
             eq_(str(u), text)
 
     def test_rfc1738_password(self):
-        u = url.make_url("dbtype://user:pass word + other:words@host/dbname")
+        u = url.make_url("dbtype://user:pass word + other%3Awords@host/dbname")
         eq_(u.password, "pass word + other:words")
-        eq_(str(u), "dbtype://user:pass word + other:words@host/dbname")
+        eq_(str(u), "dbtype://user:pass word + other%3Awords@host/dbname")
 
         u = url.make_url('dbtype://username:apples%2Foranges@hostspec/database')
-        eq_(u.password, "apples%2Foranges")
+        eq_(u.password, "apples/oranges")
         eq_(str(u), 'dbtype://username:apples%2Foranges@hostspec/database')
 
-        u = url.make_url('dbtype://username:apples@oranges@@@hostspec/database')
+        u = url.make_url('dbtype://username:apples%40oranges%40%40@hostspec/database')
         eq_(u.password, "apples@oranges@@")
-        eq_(str(u), 'dbtype://username:apples@oranges@@@hostspec/database')
+        eq_(str(u), 'dbtype://username:apples%40oranges%40%40@hostspec/database')
 
-        u = url.make_url('dbtype://username@:@hostspec/database')
+        u = url.make_url('dbtype://username%40:@hostspec/database')
         eq_(u.password, '')
         eq_(u.username, "username@")
-        eq_(str(u), 'dbtype://username@:@hostspec/database')
+        eq_(str(u), 'dbtype://username%40:@hostspec/database')
 
-        u = url.make_url('dbtype://username:pass/word@hostspec/database')
+        u = url.make_url('dbtype://username:pass%2Fword@hostspec/database')
         eq_(u.password, 'pass/word')
-        eq_(str(u), 'dbtype://username:pass/word@hostspec/database')
+        eq_(str(u), 'dbtype://username:pass%2Fword@hostspec/database')
 
 class DialectImportTest(fixtures.TestBase):
     def test_import_base_dialects(self):