"bzr send" fails if branch nick contains a slash

[Andrew Bennetts]

I have a branch with a "/" in the nick (co-incidentally part of a loom). A "bzr send" with no further arguments fails:

$ bzr nick
Fix remote_path_for_transport for http://
$ bzr send
Using saved location: /home/andrew/warthogs/bzr/devel
bzr: ERROR: [Errno 2] No such file or directory: u'/tmp/bzr-mail-bvZtnl/Fix remote_path_for_transport for http://-3319.patch'

Full traceback:

$ bzr -Derror send
Using saved location: /home/andrew/warthogs/bzr/devel
bzr: ERROR: exceptions.IOError: [Errno 2] No such file or directory: u'/tmp/bzr-mail-xEq2V7/Fix remote_path_for_transport for http://-3319.patch'

Traceback (most recent call last):
  File "/home/andrew/code/bzr/bzrlib/commands.py", line 834, in run_bzr_catch_errors
    return run_bzr(argv)
  File "/home/andrew/code/bzr/bzrlib/commands.py", line 790, in run_bzr
    ret = run(*run_argv)
  File "/home/andrew/code/bzr/bzrlib/commands.py", line 492, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/home/andrew/code/bzr/bzrlib/builtins.py", line 4130, in run
    kwargs.get('from', '.'), mail_to, message)
  File "/home/andrew/code/bzr/bzrlib/builtins.py", line 4231, in _run
    outfile.getvalue(), basename)
  File "/home/andrew/code/bzr/bzrlib/mail_client.py", line 73, in compose_merge_request
    'x-patch', '.patch', basename)
  File "/home/andrew/code/bzr/bzrlib/mail_client.py", line 135, in compose
    outfile = open(attach_path, 'wb')
IOError: [Errno 2] No such file or directory: u'/tmp/bzr-mail-xEq2V7/Fix remote_path_for_transport for http://-3319.patch'

bzr 1.4dev on python 2.5.1 (linux2)
arguments: ['/home/andrew/code/bzr/bzr', '-Derror', 'send']
encoding: 'UTF-8', fsenc: 'UTF-8', lang: 'en_AU.UTF-8'

In general, we really need a try/except here, because sanitising perfectly and portably is almost impossible. Other than "/", I'd expect NUL bytes to cause issues, also "\", possibly unicode characters (not sure if the branch nick is unicode?), and also on platforms like windows special file names like "NUL:" and "PRN:" can cause trouble. And names like "." and ".." are special everywhere. So maybe the best strategy is to simply try the attach_path unmodified, and then if that throws an exception run a very aggressive sanitiser over the filename (perhaps the URL-escaping-like one we use for knit files?).

[Andrew Bennetts (spiv)]

I guess a less ambitious fix would be to catch the IOError, and simply use the regular temp file name generation (i.e. some random ascii letters and digits). It's not perfect, but it would be a big improvement over giving a traceback.

[Robert Collins (lifeless)]

On Tue, 2008-04-01 at 12:35 +0000, Andrew Bennetts wrote:
> I guess a less ambitious fix would be to catch the IOError, and simply
> use the regular temp file name generation (i.e. some random ascii
> letters and digits). It's not perfect, but it would be a big
> improvement over giving a traceback.

I'd be inclined to mangle the nick first rather than catch an error;
nicks are not limited to file system chars.

-Rob

--
GPG key available at: <http://www.robertcollins.net/keys.txt>.