[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)

Alexander Belchenko bialix at ukr.net
Wed Feb 27 15:27:46 GMT 2008


John Arbash Meinel пишет:
> Alexander Belchenko wrote:
> | This patch fixes problem with UnicodeEncodeError and subprocess module @
> | win32.
> | Actually I need this fix only for Thunderbird, default MapiClient is not
> | affected
> | by unicode bug. So if someone think that subject.encode(user_encdoing,
> | 'replace')
> | should be only in Thunderbird class, I'll move it there.
> |
> 
> +            if sys.platform == 'win32':
> +                user_encoding = osutils.get_user_encoding()
> +                if to:
> +                    to = to.encode(user_encoding, 'replace')
> +                if subject:
> +                    subject = subject.encode(user_encoding, 'replace')
> ~             cmdline.extend(self._get_compose_commandline(to, subject,
> ~                                                          attach_path))
> 
> 
> ^- It almost seems to make more sense to put this check in
> "_get_compose_commandline".

Currently we have 5 different mail clients, so you propose to duplicate
that code 5 times in different places across mail_client.py?

> I've never really been a fan of "encode(..., 'replace')", though I guess 
> it is
> necessary here. It just means that it is lossy, which means your subject 
> and
> destination address could easily end up garbled. I suppose that is 
> better than
> not getting sent at all...
> 
> +            message_options['attachment'] = 
> str(urlutils.local_path_to_url(
> +                attach_path))
> 
> ^- If local_path_to_url isn't returning a plain string, that indicates a 
> bug in
> local_path_to_url. Because URLs (for us) are defined as 7-bit strings.

So it's definitely the bug:

In [1]: import bzrlib

In [2]: from bzrlib import urlutils

In [3]: import os

In [4]: urlutils.local_path_to_url(os.getcwdu())
Out[4]: u'file:///C:/work/Bazaar/mydev.packs/bzr.dev'

> So I would rather fix local_path_to_url rather than put a bandage around 
> it.

OK.

> Otherwise I don't have a problem with doing the encoding ourselves.
> 
> BB:tweak



More information about the bazaar mailing list