Merge lp:~jelmer/bzr/urlutils-quote into lp:bzr

[Jelmer Vernooij]

Include quote() and unquote() directly in bzrlib.urlutils.

Include quote() and unquote() directly in bzrlib.urlutils.

This avoids having to import urllib for non-http operations. urllib imports
socket, which import ssl. This adds overhead to the bzr startup time
for local operations.

[Martin Pool (mbp)]

very nice. thanks for adding an import tariff.

  vote approve

review: Approve

[Jelmer Vernooij (jelmer)]

sent to pqm by email

[Jelmer Vernooij (jelmer)]

sent to pqm by email

=== modified file 'bzrlib/mail_client.py'
--- bzrlib/mail_client.py	2010-04-23 08:51:52 +0000
+++ bzrlib/mail_client.py	2011-12-19 01:33:35 +0000
@@ -19,7 +19,6 @@
 import subprocess
 import sys
 import tempfile
-import urllib
 
 import bzrlib
 from bzrlib import (
@@ -310,7 +309,7 @@
             message_options['attachment'] = urlutils.local_path_to_url(
                 attach_path)
         if body is not None:
-            options_list = ['body=%s' % urllib.quote(self._encode_safe(body))]
+            options_list = ['body=%s' % urlutils.quote(self._encode_safe(body))]
         else:
             options_list = []
         options_list.extend(["%s='%s'" % (k, v) for k, v in
@@ -352,15 +351,15 @@
         """See ExternalMailClient._get_compose_commandline"""
         compose_url = []
         if from_ is not None:
-            compose_url.append('from=' + urllib.quote(from_))
+            compose_url.append('from=' + urlutils.quote(from_))
         if subject is not None:
-            # Don't use urllib.quote_plus because Claws doesn't seem
+            # Don't use urlutils.quote_plus because Claws doesn't seem
             # to recognise spaces encoded as "+".
             compose_url.append(
-                'subject=' + urllib.quote(self._encode_safe(subject)))
+                'subject=' + urlutils.quote(self._encode_safe(subject)))
         if body is not None:
             compose_url.append(
-                'body=' + urllib.quote(self._encode_safe(body)))
+                'body=' + urlutils.quote(self._encode_safe(body)))
         # to must be supplied for the claws-mail --compose syntax to work.
         if to is None:
             raise errors.NoMailAddressSpecified()

=== modified file 'bzrlib/plugins/launchpad/lp_registration.py'
--- bzrlib/plugins/launchpad/lp_registration.py	2011-03-24 11:41:42 +0000
+++ bzrlib/plugins/launchpad/lp_registration.py	2011-12-19 01:33:35 +0000
@@ -166,8 +166,8 @@
             # the url?  perhaps a bit more secure against accidentally
             # revealing it.  std66 s3.2.1 discourages putting the
             # password in the url.
-            hostinfo = '%s:%s@%s' % (urllib.quote(self.registrant_email),
-                                     urllib.quote(self.registrant_password),
+            hostinfo = '%s:%s@%s' % (urlutils.quote(self.registrant_email),
+                                     urlutils.quote(self.registrant_password),
                                      hostinfo)
             url = urlunsplit((scheme, hostinfo, path, '', ''))
         else:

=== modified file 'bzrlib/plugins/weave_fmt/repository.py'
--- bzrlib/plugins/weave_fmt/repository.py	2011-12-07 14:03:01 +0000
+++ bzrlib/plugins/weave_fmt/repository.py	2011-12-19 01:33:35 +0000
@@ -23,7 +23,6 @@
 import gzip
 import os
 from cStringIO import StringIO
-import urllib
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
@@ -708,7 +707,7 @@
             raise errors.ObjectNotLocked(self)
         relpaths = set()
         for quoted_relpath in self._transport.iter_files_recursive():
-            relpath = urllib.unquote(quoted_relpath)
+            relpath = urlutils.unquote(quoted_relpath)
             path, ext = os.path.splitext(relpath)
             if ext == '.gz':
                 relpath = path
@@ -748,7 +747,7 @@
             raise errors.ObjectNotLocked(self)
         relpaths = set()
         for quoted_relpath in self._transport.iter_files_recursive():
-            relpath = urllib.unquote(quoted_relpath)
+            relpath = urlutils.unquote(quoted_relpath)
             path, ext = os.path.splitext(relpath)
             if ext == '.gz':
                 relpath = path

=== modified file 'bzrlib/smart/medium.py'
--- bzrlib/smart/medium.py	2011-11-25 17:54:52 +0000
+++ bzrlib/smart/medium.py	2011-12-19 01:33:35 +0000
@@ -28,7 +28,6 @@
 import os
 import sys
 import time
-import urllib
 
 import bzrlib
 from bzrlib.lazy_import import lazy_import
@@ -840,7 +839,7 @@
         """
         medium_base = urlutils.join(self.base, '/')
         rel_url = urlutils.relative_url(medium_base, transport.base)
-        return urllib.unquote(rel_url)
+        return urlutils.unquote(rel_url)
 
 
 class SmartClientStreamMedium(SmartClientMedium):

=== modified file 'bzrlib/tests/http_server.py'
--- bzrlib/tests/http_server.py	2011-03-08 16:00:55 +0000
+++ bzrlib/tests/http_server.py	2011-12-19 01:33:35 +0000
@@ -22,9 +22,9 @@
 import re
 import SimpleHTTPServer
 import socket
-import urllib
 import urlparse
 
+from bzrlib import urlutils
 from bzrlib.tests import test_server
 
 
@@ -336,7 +336,7 @@
         """
         # abandon query parameters
         path = urlparse.urlparse(path)[2]
-        path = posixpath.normpath(urllib.unquote(path))
+        path = posixpath.normpath(urlutils.unquote(path))
         path = path.decode('utf-8')
         words = path.split('/')
         words = filter(None, words)

=== modified file 'bzrlib/tests/test_ftp_transport.py'
--- bzrlib/tests/test_ftp_transport.py	2011-08-19 22:34:02 +0000
+++ bzrlib/tests/test_ftp_transport.py	2011-12-19 01:33:35 +0000
@@ -16,7 +16,6 @@
 
 import ftplib
 import getpass
-import urllib
 
 from bzrlib import (
     config,
@@ -24,6 +23,7 @@
     tests,
     transport,
     ui,
+    urlutils,
     )
 
 from bzrlib.transport import ftp
@@ -85,9 +85,9 @@
         parsed_url = transport.ConnectedTransport._split_url(base)
         new_url = parsed_url.clone()
         new_url.user = self.user
-        new_url.quoted_user = urllib.quote(self.user)
+        new_url.quoted_user = urlutils.quote(self.user)
         new_url.password = self.password
-        new_url.quoted_password = urllib.quote(self.password)
+        new_url.quoted_password = urlutils.quote(self.password)
         return str(new_url)
 
     def test_no_prompt_for_username(self):

=== modified file 'bzrlib/tests/test_import_tariff.py'
--- bzrlib/tests/test_import_tariff.py	2011-12-18 15:49:48 +0000
+++ bzrlib/tests/test_import_tariff.py	2011-12-19 01:33:35 +0000
@@ -190,11 +190,14 @@
             'bzrlib.xml8',
             'getpass',
             'kerberos',
+            'ssl',
+            'socket',
             'smtplib',
             'tarfile',
             'tempfile',
             'termios',
             'tty',
+            'urllib',
             ] + old_format_modules)
         # TODO: similar test for repository-only operations, checking we avoid
         # loading wt-specific stuff

=== modified file 'bzrlib/tests/test_mail_client.py'
--- bzrlib/tests/test_mail_client.py	2009-09-02 08:26:27 +0000
+++ bzrlib/tests/test_mail_client.py	2011-12-19 01:33:35 +0000
@@ -14,8 +14,6 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-import urllib
-
 from bzrlib import (
     errors,
     mail_client,
@@ -212,7 +210,7 @@
         claws = mail_client.Claws(None)
         cmdline = claws._get_compose_commandline(
             u'[e-mail]', u'\xb5cosm of fun!', u'file%')
-        subject_string = urllib.quote(
+        subject_string = urlutils.quote(
             u'\xb5cosm of fun!'.encode(osutils.get_user_encoding(), 'replace'))
         self.assertEqual(
             ['--compose',

=== modified file 'bzrlib/tests/test_permissions.py'
--- bzrlib/tests/test_permissions.py	2011-02-25 15:24:22 +0000
+++ bzrlib/tests/test_permissions.py	2011-12-19 01:33:35 +0000
@@ -32,9 +32,8 @@
 
 import os
 import sys
-import urllib
 
-from bzrlib import transport
+from bzrlib import urlutils
 from bzrlib.branch import Branch
 from bzrlib.bzrdir import BzrDir
 from bzrlib.tests import TestCaseWithTransport, TestSkipped
@@ -68,11 +67,11 @@
         test.assertTransportMode(t, base, dir_mode)
     for root, dirs, files in os.walk(base):
         for d in dirs:
-            p = '/'.join([urllib.quote(x) for x in root.split('/\\') + [d]])
+            p = '/'.join([urlutils.quote(x) for x in root.split('/\\') + [d]])
             test.assertTransportMode(t, p, dir_mode)
         for f in files:
             p = os.path.join(root, f)
-            p = '/'.join([urllib.quote(x) for x in root.split('/\\') + [f]])
+            p = '/'.join([urlutils.quote(x) for x in root.split('/\\') + [f]])
             test.assertTransportMode(t, p, file_mode)
 
 

=== modified file 'bzrlib/tests/test_urlutils.py'
--- bzrlib/tests/test_urlutils.py	2011-11-24 13:15:51 +0000
+++ bzrlib/tests/test_urlutils.py	2011-12-19 01:33:35 +0000
@@ -1026,3 +1026,20 @@
             urlutils.file_relpath, "file:///A:/b", "file:///A:/")
         self.assertRaises(PathNotChild,
             urlutils.file_relpath, "file:///A:/b/c", "file:///A:/b")
+
+
+class QuoteTests(TestCase):
+
+    def test_quote(self):
+        self.assertEqual('abc%20def', urlutils.quote('abc def'))
+        self.assertEqual('abc%2Fdef', urlutils.quote('abc/def', safe=''))
+        self.assertEqual('abc/def', urlutils.quote('abc/def', safe='/'))
+
+    def test_quote_tildes(self):
+        self.assertEqual('%7Efoo', urlutils.quote('~foo'))
+        self.assertEqual('~foo', urlutils.quote('~foo', safe='/~'))
+
+    def test_unquote(self):
+        self.assertEqual('%', urlutils.unquote('%25'))
+        self.assertEqual('\xc3\xa5', urlutils.unquote('%C3%A5'))
+        self.assertEqual(u"\xe5", urlutils.unquote(u'\xe5'))

=== modified file 'bzrlib/transport/__init__.py'
--- bzrlib/transport/__init__.py	2011-11-25 17:49:44 +0000
+++ bzrlib/transport/__init__.py	2011-12-19 01:33:35 +0000
@@ -33,7 +33,6 @@
 lazy_import(globals(), """
 import errno
 from stat import S_ISDIR
-import urllib
 import urlparse
 
 from bzrlib import (
@@ -1415,12 +1414,12 @@
 
         :return: The corresponding URL.
         """
-        netloc = urllib.quote(host)
+        netloc = urlutils.quote(host)
         if user is not None:
             # Note that we don't put the password back even if we
             # have one so that it doesn't get accidentally
             # exposed.
-            netloc = '%s@%s' % (urllib.quote(user), netloc)
+            netloc = '%s@%s' % (urlutils.quote(user), netloc)
         if port is not None:
             netloc = '%s:%d' % (netloc, port)
         path = urlutils.escape(path)

=== modified file 'bzrlib/transport/http/__init__.py'
--- bzrlib/transport/http/__init__.py	2011-09-19 13:02:42 +0000
+++ bzrlib/transport/http/__init__.py	2011-12-19 01:33:35 +0000
@@ -22,7 +22,6 @@
 from cStringIO import StringIO
 import re
 import urlparse
-import urllib
 import sys
 import weakref
 
@@ -63,9 +62,9 @@
             host = netloc.split(':', 1)[0]
         else:
             host = netloc
-        username = urllib.unquote(username)
+        username = urlutils.unquote(username)
         if password is not None:
-            password = urllib.unquote(password)
+            password = urlutils.unquote(password)
         else:
             password = ui.ui_factory.get_password(
                 prompt=u'HTTP %(user)s@%(host)s password',
@@ -589,7 +588,7 @@
         if transport_base.startswith('bzr+'):
             transport_base = transport_base[4:]
         rel_url = urlutils.relative_url(self.base, transport_base)
-        return urllib.unquote(rel_url)
+        return urlutils.unquote(rel_url)
 
     def send_http_smart_request(self, bytes):
         try:

=== modified file 'bzrlib/urlutils.py'
--- bzrlib/urlutils.py	2011-11-24 13:15:51 +0000
+++ bzrlib/urlutils.py	2011-12-19 01:33:35 +0000
@@ -23,7 +23,6 @@
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
 from posixpath import split as _posix_split
-import urllib
 import urlparse
 
 from bzrlib import (
@@ -60,13 +59,87 @@
     return split(url, exclude_trailing_slash=exclude_trailing_slash)[0]
 
 
+# Private copies of quote and unquote, copied from Python's
+# urllib module because urllib unconditionally imports socket, which imports
+# ssl.
+
+always_safe = ('ABCDEFGHIJKLMNOPQRSTUVWXYZ'
+               'abcdefghijklmnopqrstuvwxyz'
+               '0123456789' '_.-')
+_safe_map = {}
+for i, c in zip(xrange(256), str(bytearray(xrange(256)))):
+    _safe_map[c] = c if (i < 128 and c in always_safe) else '%{0:02X}'.format(i)
+_safe_quoters = {}
+
+
+def quote(s, safe='/'):
+    """quote('abc def') -> 'abc%20def'
+
+    Each part of a URL, e.g. the path info, the query, etc., has a
+    different set of reserved characters that must be quoted.
+
+    RFC 2396 Uniform Resource Identifiers (URI): Generic Syntax lists
+    the following reserved characters.
+
+    reserved    = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" |
+                  "$" | ","
+
+    Each of these characters is reserved in some component of a URL,
+    but not necessarily in all of them.
+
+    By default, the quote function is intended for quoting the path
+    section of a URL.  Thus, it will not encode '/'.  This character
+    is reserved, but in typical usage the quote function is being
+    called on a path where the existing slash characters are used as
+    reserved characters.
+    """
+    # fastpath
+    if not s:
+        if s is None:
+            raise TypeError('None object cannot be quoted')
+        return s
+    cachekey = (safe, always_safe)
+    try:
+        (quoter, safe) = _safe_quoters[cachekey]
+    except KeyError:
+        safe_map = _safe_map.copy()
+        safe_map.update([(c, c) for c in safe])
+        quoter = safe_map.__getitem__
+        safe = always_safe + safe
+        _safe_quoters[cachekey] = (quoter, safe)
+    if not s.rstrip(safe):
+        return s
+    return ''.join(map(quoter, s))
+
+
+_hexdig = '0123456789ABCDEFabcdef'
+_hextochr = dict((a + b, chr(int(a + b, 16)))
+                 for a in _hexdig for b in _hexdig)
+
+def unquote(s):
+    """unquote('abc%20def') -> 'abc def'."""
+    res = s.split('%')
+    # fastpath
+    if len(res) == 1:
+        return s
+    s = res[0]
+    for item in res[1:]:
+        try:
+            s += _hextochr[item[:2]] + item[2:]
+        except KeyError:
+            s += '%' + item
+        except UnicodeDecodeError:
+            s += unichr(int(item[:2], 16)) + item[2:]
+    return s
+
+
 def escape(relpath):
     """Escape relpath to be a valid url."""
     if isinstance(relpath, unicode):
         relpath = relpath.encode('utf-8')
     # After quoting and encoding, the path should be perfectly
     # safe as a plain ASCII string, str() just enforces this
-    return str(urllib.quote(relpath, safe='/~'))
+    return str(quote(relpath, safe='/~'))
 
 
 def file_relpath(base, path):
@@ -566,7 +639,7 @@
     This returns a Unicode path from a URL
     """
     # jam 20060427 URLs are supposed to be ASCII only strings
-    #       If they are passed in as unicode, urllib.unquote
+    #       If they are passed in as unicode, unquote
     #       will return a UNICODE string, which actually contains
     #       utf-8 bytes. So we have to ensure that they are
     #       plain ASCII strings, or the final .decode will
@@ -577,7 +650,7 @@
     except UnicodeError, e:
         raise errors.InvalidURL(url, 'URL was not a plain ASCII url: %s' % (e,))
 
-    unquoted = urllib.unquote(url)
+    unquoted = unquote(url)
     try:
         unicode_path = unquoted.decode('utf-8')
     except UnicodeError, e:
@@ -742,20 +815,20 @@
             port, quoted_path):
         self.scheme = scheme
         self.quoted_host = quoted_host
-        self.host = urllib.unquote(self.quoted_host)
+        self.host = unquote(self.quoted_host)
         self.quoted_user = quoted_user
         if self.quoted_user is not None:
-            self.user = urllib.unquote(self.quoted_user)
+            self.user = unquote(self.quoted_user)
         else:
             self.user = None
         self.quoted_password = quoted_password
         if self.quoted_password is not None:
-            self.password = urllib.unquote(self.quoted_password)
+            self.password = unquote(self.quoted_password)
         else:
             self.password = None
         self.port = port
         self.quoted_path = _url_hex_escapes_re.sub(_unescape_safe_chars, quoted_path)
-        self.path = urllib.unquote(self.quoted_path)
+        self.path = unquote(self.quoted_path)
 
     def __eq__(self, other):
         return (isinstance(other, self.__class__) and
@@ -871,7 +944,7 @@
         if offset is not None:
             relative = unescape(offset).encode('utf-8')
             path = self._combine_paths(self.path, relative)
-            path = urllib.quote(path, safe="/~")
+            path = quote(path, safe="/~")
         else:
             path = self.quoted_path
         return self.__class__(self.scheme, self.quoted_user,

=== modified file 'bzrlib/versionedfile.py'
--- bzrlib/versionedfile.py	2011-05-16 10:08:01 +0000
+++ bzrlib/versionedfile.py	2011-12-19 01:33:35 +0000
@@ -24,8 +24,6 @@
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
-import urllib
-
 from bzrlib import (
     annotate,
     bencode,
@@ -38,6 +36,7 @@
     multiparent,
     tsort,
     revision,
+    urlutils,
     )
 """)
 from bzrlib.registry import Registry
@@ -821,11 +820,11 @@
 
     def map(self, key):
         """See KeyMapper.map()."""
-        return urllib.quote(self._map(key))
+        return urlutils.quote(self._map(key))
 
     def unmap(self, partition_id):
         """See KeyMapper.unmap()."""
-        return self._unmap(urllib.unquote(partition_id))
+        return self._unmap(urlutils.unquote(partition_id))
 
 
 class PrefixMapper(URLEscapeMapper):
@@ -878,7 +877,7 @@
     def _escape(self, prefix):
         """Turn a key element into a filesystem safe string.
 
-        This is similar to a plain urllib.quote, except
+        This is similar to a plain urlutils.quote, except
         it uses specific safe characters, so that it doesn't
         have to translate a lot of valid file ids.
         """
@@ -891,7 +890,7 @@
 
     def _unescape(self, basename):
         """Escaped names are easily unescaped by urlutils."""
-        return urllib.unquote(basename)
+        return urlutils.unquote(basename)
 
 
 def make_versioned_files_factory(versioned_file_factory, mapper):

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-12-15 14:47:22 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-12-19 01:33:35 +0000
@@ -116,6 +116,11 @@
   speeding up various commands including ``bzr export``,
   ``bzr checkout`` and ``bzr cat``. (Jelmer Vernooij, #608640)
 
+* ``bzrlib.urlutils`` now includes ``quote`` and ``unquote`` functions,
+  rather than importing them from ``urllib``. This prevents loading
+  of the ``socket``, ``ssl`` and ``urllib`` modules for
+  local bzr operations. (Jelmer Vernooij)
+
 Testing
 *******