Merge lp:~gz/bzr-explorer/repo_view_non_ascii_branch_open_777860 into lp:bzr-explorer

[Martin Packman]

Opening branches with non-ascii names works in general, but from the repository view you get an error about invalid urls. Seems the code is a little confused about what the path is. Both osutils and urlutils are used in the file. Simply escaping the relpath (which will always be one directory as far as I can see, so we don't need to worry about slashes) in _get_abspath appears to resolve the issue at least.

I've tested this manually, and would like to add an automated test. I don't see anything likely in the tests folder to base one off however.

[Alexander Belchenko (bialix)]

Many thanks for your quick patch!

Re tests: testing GUI is not very easy, at least I haven't found my way into these lands yet.

review: Approve

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/05/2011 06:19 PM, Martin [gz] wrote:
> Martin [gz] has proposed merging lp:~gz/bzr-explorer/repo_view_non_ascii_branch_open_777860 into lp:bzr-explorer.
>
> Requested reviews:
> Bazaar Explorer Developers (bzr-explorer-dev)
> Related bugs:
> Bug #777860 in Bazaar Explorer: "InvalidURL opening branch with non-ascii characters from repository view"
> https://bugs.launchpad.net/bzr-explorer/+bug/777860
>
> For more details, see:
> https://code.launchpad.net/~gz/bzr-explorer/repo_view_non_ascii_branch_open_777860/+merge/60090
>
> Opening branches with non-ascii names works in general, but from the repository view you get an error about invalid urls. Seems the code is a little confused about what the path is. Both osutils and urlutils are used in the file. Simply escaping the relpath (which will always be one directory as far as I can see, so we don't need to worry about slashes) in _get_abspath appears to resolve the issue at least.
>
> I've tested this manually, and would like to add an automated test. I don't see anything likely in the tests folder to base one off however.

URLs passed to transports (Transport.get()) should be URLs, and not
simple paths.

bzrlib.transport.get_transport() tries to be DWIM friendly, and will try
to figure out what kind of string is passed, and whether it should be
treated as a pure URL or as a "Unicode" URL (starts with HTTP:// but
uses u'\xb5' rather than %B5, etc.)

URLs are always 7-bit ascii.

So yes, abspath() be getting an escaped relpath. I don't know whether
all the code in that file is passing in a Unicode path string or whether
it is usually/sometimes/always a URL.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3D/0EACgkQJdeBCYSNAAOzCACfYvolXU0FfhDNCsA80YT+g0RH
8yoAnixEEdhblUQ3THlqzlwqulrX2tTc
=Q1tp
-----END PGP SIGNATURE-----

=== modified file 'NEWS'
--- NEWS	2010-11-26 14:42:06 +0000
+++ NEWS	2011-05-05 16:19:28 +0000
@@ -8,6 +8,11 @@
 1.1.3 dd-mmm-2010
 -----------------
 
+Bug Fixes:
+
+* Rather than displaying an error saying "Invalid url supplied to transport"
+  branches with non-ascii characters in the directory name can now be opened.
+ (Martin [gz], #777860)
 
 1.1.2 26-Nov-2010
 -----------------

=== modified file 'lib/view_repository.py'
--- lib/view_repository.py	2010-02-21 05:34:30 +0000
+++ lib/view_repository.py	2011-05-05 16:19:28 +0000
@@ -240,7 +240,7 @@
         return unicode(model.data(id_index).toString())
 
     def _get_abspath(self, relpath):
-        return self._get_root_transport().abspath(relpath)
+        return self._get_root_transport().abspath(urlutils.escape(relpath))
 
     def do_double_clicked(self, model_index):
         self._relpath = self._get_relpath_for_index(model_index)