[MERGE][#139318] bzr send @ win32: ensure that command line to invoking Thunderbird is 8-bit string, not unicode (because subprocess does not understand non-ascii unicode)

[Aaron Bentley]

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

Alexander Belchenko wrote:
> Test case provided.

Thanks.

> I restrict the test for Thunderbird client for now,
> because it uses url instead of abs path to attachment. I think I should not
> touch paths (neither path to executable nor attachment).

I think this isn't right.  If the path to an executable or attachment is
unicode, then on Windows, won't Python raise a UnicodeEncodeError?

> Additionally I fix hidden bug in win32_path_to_url function that leads
> to force unicode URL for local paths. Test cases provided for posix and
> win32 implementations of *_path_to_url.

Thank you.

>> Actually, it looks like we need to encode all arguments that may be
>> provided, so I think the right place would be in
>> ExternalMailClient._compose, doing
>>
>> cmdline = [a.encode(user_encoding, 'replace') for a in cmdline]
>>
>> If you do it there, it's pretty hard to test, so I wouldn't require it.
> 
> This approach actually is unsafe because
> unicode_path.encode(user_encoding, 'replace')
> could break absolute unicode paths to executable or attachment.

I believe an absolute unicode path to an executable or attachment would
be unusable.  These are 8-bit APIs, and it looks like on Windows, Python
isn't encoding properly.  So guess is that on Windows, all unicode
commandline parameters must be encoded.

> +    * Encode recipient and subject line from unicode to user_encoding before
> +      invoking external mail client (e.g. Thunderbird) in `bzr send` command
> +      on Windows. (#139318, Alexander Belchenko)

No longer only on Windows.

> --- bzrlib/mail_client.py	2007-11-28 17:27:10 +0000
> +++ bzrlib/mail_client.py	2008-02-27 18:52:57 +0000
> @@ -144,9 +144,8 @@
>              the attachment type.
>          """
>          for name in self._get_client_commands():
> -            cmdline = [name]
> -            cmdline.extend(self._get_compose_commandline(to, subject,
> -                                                         attach_path))
> +            cmdline = _get_compose_8bit_commandline(name, to, subject,
> +                attach_path)

Does this actually work?  It looks like it should be
self._get_compose_8bit...

> === modified file 'bzrlib/urlutils.py'
> --- bzrlib/urlutils.py	2007-12-21 17:26:37 +0000
> +++ bzrlib/urlutils.py	2008-02-27 18:24:39 +0000
> @@ -274,7 +274,8 @@
>      # check for UNC path \\HOST\path
>      if win32_path.startswith('//'):
>          return 'file:' + escape(win32_path)
> -    return 'file:///' + win32_path[0].upper() + ':' + escape(win32_path[2:])
> +    return ('file:///' + str(win32_path[0].upper()) + ':' +
> +        escape(win32_path[2:]))

I suppose drive letters can't be non-ascii, so this probably doesn't
matter, but it would be nice to do escape(win32_path[0].upper()) for
consistency.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHxiHO0F+nu1YWqI0RAjXqAKCJo+bWoFDe2xM1CsPgYc/OtMyf1gCePgTz
6BcVlDyDgZeSTJU+xZ1bc0E=
=Iw+s
-----END PGP SIGNATURE-----