Merge lp:~spiv/bzr/escape-smart-server-requested-paths-458762 into lp:bzr

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

This fixes inconsistencies in server-side HPSS path decoding that was breaking unicode paths. The client has always been consistent in how it was transmitting requests (VFS requests use URL escaped paths, others use UTF-8), but the server was assuming URL escaped everywhere. This branch fixes the server to match reality, and adds some tests (and fixes an existing one!). Thanks very much to Michael Hudson for investigating this issue and writing the first version of this patch.

[Martin Pool (mbp)]

review: Approve

=== modified file 'NEWS'
--- NEWS	2009-10-28 00:12:03 +0000
+++ NEWS	2009-10-29 00:35:22 +0000
@@ -42,6 +42,9 @@
 * TreeTransform.adjust_path updates the limbo paths of descendants of adjusted
   files.  (Aaron Bentley)
 
+* Unicode paths are now handled correctly and consistently by the smart
+  server.  (Andrew Bennetts, Michael Hudson, #458762)
+
 Improvements
 ************
 
 
=== modified file 'bzrlib/smart/request.py'
--- bzrlib/smart/request.py	2009-10-22 06:53:08 +0000
+++ bzrlib/smart/request.py	2009-10-29 00:35:22 +0000
@@ -188,7 +188,7 @@
             relpath = urlutils.joinpath('/', path)
             if not relpath.startswith('/'):
                 raise ValueError(relpath)
-            return '.' + relpath
+            return urlutils.escape('.' + relpath)
         else:
             raise errors.PathNotChild(client_path, self._root_client_path)
 
 
=== modified file 'bzrlib/smart/vfs.py'
--- bzrlib/smart/vfs.py	2009-03-23 14:59:43 +0000
+++ bzrlib/smart/vfs.py	2009-10-29 00:35:22 +0000
@@ -27,6 +27,7 @@
 import os
 
 from bzrlib import errors
+from bzrlib import urlutils
 from bzrlib.smart import request
 
 
@@ -59,6 +60,14 @@
         if not vfs_enabled():
             raise errors.DisabledMethod(self.__class__.__name__)
 
+    def translate_client_path(self, relpath):
+        # VFS requests are made with escaped paths so the escaping done in
+        # SmartServerRequest.translate_client_path leads to double escaping.
+        # Remove it here -- the fact that the result is still escaped means
+        # that the str() will not fail on valid input.
+        x = request.SmartServerRequest.translate_client_path(self, relpath)
+        return str(urlutils.unescape(x))
+
 
 class HasRequest(VfsRequest):
 
 
=== modified file 'bzrlib/tests/test_smart.py'
--- bzrlib/tests/test_smart.py	2009-09-24 05:31:23 +0000
+++ bzrlib/tests/test_smart.py	2009-10-29 00:35:22 +0000
@@ -43,6 +43,7 @@
 import bzrlib.smart.bzrdir, bzrlib.smart.bzrdir as smart_dir
 import bzrlib.smart.packrepository
 import bzrlib.smart.repository
+import bzrlib.smart.vfs
 from bzrlib.smart.request import (
     FailedSmartServerResponse,
     SmartServerRequest,
@@ -170,6 +171,18 @@
         self.assertRaises(
             errors.PathNotChild, request.translate_client_path, 'bar/')
         self.assertEqual('./baz', request.translate_client_path('foo/baz'))
+        e_acute = u'\N{LATIN SMALL LETTER E WITH ACUTE}'.encode('utf-8')
+        self.assertEqual('./' + urlutils.escape(e_acute),
+                         request.translate_client_path('foo/' + e_acute))
+
+    def test_translate_client_path_vfs(self):
+        """VfsRequests receive escaped paths rather than raw UTF-8."""
+        transport = self.get_transport()
+        request = smart.vfs.VfsRequest(transport, 'foo/')
+        e_acute = u'\N{LATIN SMALL LETTER E WITH ACUTE}'.encode('utf-8')
+        escaped = urlutils.escape('foo/' + e_acute)
+        self.assertEqual('./' + urlutils.escape(e_acute),
+                         request.translate_client_path(escaped))
 
     def test_transport_from_client_path(self):
         transport = self.get_transport()
@@ -1690,6 +1703,19 @@
         self.assertEqual(SmartServerResponse(('ok',)), response)
 
 
+class TestSmartServerVfsGet(tests.TestCaseWithMemoryTransport):
+
+    def test_unicode_path(self):
+        """VFS requests expect unicode paths to be escaped."""
+        filename = u'foo\N{INTERROBANG}'
+        filename_escaped = urlutils.escape(filename)
+        backing = self.get_transport()
+        request = smart.vfs.GetRequest(backing)
+        backing.put_bytes_non_atomic(filename_escaped, 'contents')
+        self.assertEqual(SmartServerResponse(('ok', ), 'contents'),
+            request.execute(filename_escaped))
+
+
 class TestHandlers(tests.TestCase):
     """Tests for the request.request_handlers object."""
 
 
=== modified file 'bzrlib/tests/test_smart_transport.py'
--- bzrlib/tests/test_smart_transport.py	2009-09-24 05:31:23 +0000
+++ bzrlib/tests/test_smart_transport.py	2009-10-29 00:35:22 +0000
@@ -657,8 +657,10 @@
         # wire-to-wire, using the whole stack, with a UTF-8 filename.
         transport = memory.MemoryTransport('memory:///')
         utf8_filename = u'testfile\N{INTERROBANG}'.encode('utf-8')
+        # VFS requests use filenames, not raw UTF-8.
+        hpss_path = urlutils.escape(utf8_filename)
         transport.put_bytes(utf8_filename, 'contents\nof\nfile\n')
-        to_server = StringIO('get\001' + utf8_filename + '\n')
+        to_server = StringIO('get\001' + hpss_path + '\n')
         from_server = StringIO()
         server = medium.SmartServerPipeStreamMedium(
             to_server, from_server, transport)
 
=== modified file 'doc/developers/network-protocol.txt'
--- doc/developers/network-protocol.txt	2009-09-17 03:21:14 +0000
+++ doc/developers/network-protocol.txt	2009-10-29 00:35:22 +0000
@@ -436,6 +436,9 @@
 using `bzrlib.transport.pathfilter` and `os.path.expanduser`, taking care
 to respect the virtual root.
 
+Paths in request arguments are UTF-8 encoded, except for the legacy VFS
+requests which expect escaped (`bzrlib.urlutils.escape`) paths.
+
 
 Requests
 ========