[patch] Fix displaying attachments with spaces etc.

[patch] Fix displaying attachments with spaces etc.

[Hrvoje Niksic]

For years now it has annoyed me that Gnus can't seem to show
attachments whose file name contain spaces.  It was all the weirder
because Gnus does try to dilligently quote file names.  Today I
tracked down the problem.

My Debian system tries to be smart and has system mailcap entries that
look like this:

    image/jpeg; display '%s'

Gnus also tries to be smart and change every occurrence of %s to a
shell-quoted string.  This is presumably so that simple-minded entries
like this:

    image/jpeg; display %s

keep working.  So for the file name "Amateur Pussy(37).jpg" Gnus ends
up calling the display command like this:

    (start-process ... "sh" "-c" "display 'Amateur\\ Pussy\\(37\\).jpg'")

which fails for obvious reasons.  This is annoying because once in a
while I receive a spam that I actually want to read.  (The above
example is, of course, completely fabricated with no connection
whatsoever to real events or spams.)

Here is a fix that special-cases '%s' and "%s" by simply treating them
the same as %s.  This workaround will fail to perform in more
convoluted cases such as display 'foo%sbar', but we won't be any worse
off than we were without it.  I haven't found any use of %s other than
as a complete string in my 271-line system mailcap file.

Here is the patch which I intend to commit unless someone yells
(please Cc me) or offers a better one:

2002-08-13  Hrvoje Niksic  <hniksic@xemacs.org>

	* mm-decode.el (mm-mailcap-command): Remove the quotes around '%s'
	and "%s" so we don't overquote them.

--- lisp/mm-decode.el.orig	Tue Aug 13 17:41:02 2002
+++ lisp/mm-decode.el	Tue Aug 13 17:44:40 2002
@@ -774,7 +774,7 @@
 	(beg 0)
 	(uses-stdin t)
 	out sub total)
-    (while (string-match "%{\\([^}]+\\)}\\|%s\\|%t\\|%%" method beg)
+    (while (string-match "%{\\([^}]+\\)}\\|'%s'\\|\"%s\"\\|%s\\|%t\\|%%" method beg)
       (push (substring method beg (match-beginning 0)) out)
       (setq beg (match-end 0)
 	    total (match-string 0 method)
@@ -782,7 +782,10 @@
       (cond
        ((string= total "%%")
 	(push "%" out))
-       ((string= total "%s")
+       ((or (string= total "%s")
+	    ;; we do our own quoting
+	    (string= total "'%s'")
+	    (string= total "\"%s\""))
 	(setq uses-stdin nil)
 	(push (mm-quote-arg
 	       (gnus-map-function mm-path-name-rewrite-functions file)) out))

Re: [patch] Fix displaying attachments with spaces etc.

[Simon Josefsson]

Hrvoje Niksic <hniksic@xemacs.org> writes:

> My Debian system tries to be smart and has system mailcap entries that
> look like this:
>
>     image/jpeg; display '%s'

Has anyone talked to the Debian people that maintain the file? [1]

IMHO mailcap application must quote things internally, and mailcap
files should not quote things. The Debian approach doesn't work when
the filename contains '.

FWIW, I like the patch.

[1] Which package creates mailcap, btw?  dpkg isn't very helpful (the
file exists of course, and I didn't create it):

bash-2.05b$ dpkg -S /etc/mailcap
dpkg: /etc/mailcap not found.
bash-2.05b$

Re: [patch] Fix displaying attachments with spaces etc.

[Alan Shutko]

Simon Josefsson <jas@extundo.com> writes:

> Has anyone talked to the Debian people that maintain the file? [1]

Yes, and they're not changing anything.

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=90483
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=122279

> IMHO mailcap application must quote things internally, and mailcap
> files should not quote things. The Debian approach doesn't work when
> the filename contains '.

They'll say you shouldn't put ' in filenames.

-- 
Alan Shutko <ats@acm.org> - In a variety of flavors!
Smith's Law: No real problem has a solution.

Re: [patch] Fix displaying attachments with spaces etc.

[Simon Josefsson]

Alan Shutko <ats@acm.org> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> Has anyone talked to the Debian people that maintain the file? [1]
>
> Yes, and they're not changing anything.

Oops, then I sent another one in vain.

>> IMHO mailcap application must quote things internally, and mailcap
>> files should not quote things. The Debian approach doesn't work when
>> the filename contains '.
>
> They'll say you shouldn't put ' in filenames.

D'oh.

Re: [patch] Fix displaying attachments with spaces etc.

[Hrvoje Niksic]

Simon Josefsson <jas@extundo.com> writes:

> Hrvoje Niksic <hniksic@xemacs.org> writes:
>
>> My Debian system tries to be smart and has system mailcap entries that
>> look like this:
>>
>>     image/jpeg; display '%s'
>
> Has anyone talked to the Debian people that maintain the file? [1]

I haven't.  I can only assume that they did that to fix the broken
and simple-minded applications that simply do the printf-like
replacement of %s and handle the rest to sh -c.  I have a hunch that
there is a large number of those around.

Maybe they can be persuaded to change that on the grounds of
correctness.  Maybe not.  I don't know.  Be that as it may, Gnus
should at least handle that format on the ground of all those broken
Debian mailcaps around.  :-(

> IMHO mailcap application must quote things internally, and mailcap
> files should not quote things. The Debian approach doesn't work when
> the filename contains '.

With my patch it does.  :-)  But yes, I see your point.

> FWIW, I like the patch.

Thanks.  I think it's "safe" in the sense that it fixes an important
class of cases, while making Gnus behave no worse than it did before.

> [1] Which package creates mailcap, btw?  dpkg isn't very helpful (the
> file exists of course, and I didn't create it):
>
> bash-2.05b$ dpkg -S /etc/mailcap
> dpkg: /etc/mailcap not found.
> bash-2.05b$

`man update-mime'.  The idea is that every package stores its MIME
types in its own file, and runs `update-mime' from PACKAGE.postinst.
That way a package can safely update its MIME types without touching
other packages.  This kind of split is very frequent in Debian.

The comment at the beginning of `update-mime' is encouraging:

#  Update-MIME:  Install programs into "/etc/mailcap", resolve conflicts,
#                                auto-uninstall, make dinner, and wash dishes.

Re: [patch] Fix displaying attachments with spaces etc.

[Paul Jarc]

Simon Josefsson <jas@extundo.com> wrote:
> IMHO mailcap application must quote things internally, and mailcap
> files should not quote things. The Debian approach doesn't work when
> the filename contains '.

If mailcap weren't so badly designed, it wouldn't be necessary to
quote anything.
image/jpeg: display "$filename"
Then the application would set $filename in the environment.  Gnus
could do this substitution in a similar way to this patch, although I
don't know whether it's worthwhile since the quoting code is already
written.


paul

Re: Fix displaying attachments with spaces etc.

[Toby Speight]

I wonder if it's sufficient to (when (search-forward "'%s'" nil t)
(replace-match "%s" t t command)) to take care of this particular
case.  Personally, I'd feel a lot more comfortable if Gnus ignored the
sender's suggested filename except when saving to a permanent file,
and used a randomly-generated name for viewing, in the same way that
W3 does.  I don't like the idea of trusting the author, no matter how
careful Gnus is.


0> In article <m3d6smtx2a.fsf@multivac.cwru.edu>,
0> Paul R. Jarc <URL:mailto:prj@po.cwru.edu> ("Paul") wrote:

Paul> If mailcap weren't so badly designed, it wouldn't be necessary to
Paul> quote anything.
Paul> image/jpeg: display "$filename"
Paul> Then the application would set $filename in the environment.  Gnus
Paul> could do this substitution in a similar way to this patch, although I
Paul> don't know whether it's worthwhile since the quoting code is already
Paul> written.

Yeah.  I think it's safer (from silly mistakes in the quoting code,
perhaps) to use environmental substitution.  This would still require
removal of the single quotes as above, though.

Re: Fix displaying attachments with spaces etc.

[Simon Josefsson]

> I wonder if it's sufficient to (when (search-forward "'%s'" nil t)
> (replace-match "%s" t t command)) to take care of this particular
> case.

I think this is conceptually what Hrvoje's patch does.

> Personally, I'd feel a lot more comfortable if Gnus ignored the
> sender's suggested filename except when saving to a permanent file, and
> used a randomly-generated name for viewing, in the same way that W3
> does.  I don't like the idea of trusting the author, no matter how
> careful Gnus is.

Send a patch. :-)   The suggested filename contains some information
though, and Gnus doesn't display it normally, I think, so you'll lose
information.  Gnus could display the filename below the MIME button or in
the minibuffer though.

Re: Fix displaying attachments with spaces etc.

[Kai Großjohann]

Toby Speight <streapadair@gmx.net> writes:

> I wonder if it's sufficient to (when (search-forward "'%s'" nil t)
> (replace-match "%s" t t command)) to take care of this particular
> case.  Personally, I'd feel a lot more comfortable if Gnus ignored the
> sender's suggested filename except when saving to a permanent file,
> and used a randomly-generated name for viewing, in the same way that
> W3 does.  I don't like the idea of trusting the author, no matter how
> careful Gnus is.

I guess the filename is needed for some programs.  Maybe some
programs look at the filename to decide what to do with it?

But Gnus could mangle the filename to be somewhat harmless.  For
example, it could replace spaces with underscores and ticks of all
kinds and sizes, too.  That would keep the extension intact which is
probably what those programs are looking at...

kai
-- 
A large number of young women don't trust men with beards.  (BFBS Radio)

Re: Fix displaying attachments with spaces etc.

[Toby Speight]

Please don't send me separate copies of list articles (did my
"Mail-Copies-To: never" get dropped somewhere?)


0> In article <vafofc03qy5.fsf@INBOX.auto.gnus.tok.lucy.cs.uni-dortmund.de>,
0> Kai Großjohann <URL:mailto:Kai.Grossjohann@CS.Uni-Dortmund.DE> ("Kai") wrote:

Kai> I guess the filename is needed for some programs.  Maybe some
Kai> programs look at the filename to decide what to do with it?

Well, that's broken on their part, but if we are to support filename-based
processing, shouldn't we be using the established local filename convention
(as described in /etc/mime.types or equivalent) rather than the *sender's*
local filename convention?

I see too many exploit attempts from the Windows world using MIME
parts with a declared type that's different to the receiver's implied
(from the filename) type.  I don't want to open up GNU systems to that
sort of abuse.


Kai> But Gnus could mangle the filename to be somewhat harmless.  For
Kai> example, it could replace spaces with underscores and ticks of
Kai> all kinds and sizes, too.  That would keep the extension intact
Kai> which is probably what those programs are looking at...

Possibly all non-alphamerics (except '.') could safely be stripped or
replaced?

list copies (was Re: Fix displaying attachments with spaces etc.)

[Sean Neakums]

commence  Toby Speight quotation:

> Please don't send me separate copies of list articles (did my
> "Mail-Copies-To: never" get dropped somewhere?)

I don't see it.  (I'm subscribed via mail, not NNTP.)  For mailing
lists, you should set Mail-Followup-To.

-- 
 /                          |
[|] Sean Neakums            |  Questions are a burden to others;
[|] <sneakums@zork.net>     |      answers a prison for oneself.
 \                          |

Re: list copies

[Toby Speight]

0> In article <6u8z32u9mg.fsf_-_@zork.zork.net>,
0> Sean Neakums <URL:mailto:sneakums@zork.net> ("Sean") wrote:

Sean> commence  Toby Speight quotation:

>> Please don't send me separate copies of list articles (did my
>> "Mail-Copies-To: never" get dropped somewhere?)

Sean> I don't see it.  (I'm subscribed via mail, not NNTP.)  For mailing
Sean> lists, you should set Mail-Followup-To.

Hmm, my bad:

    (setq message-required-mail-headers
        (append common-headers
                '(To
                  (optional . In-Reply-To)
                  (X-Message-Flag . fortune-short)
                  (optional . X-Mailer))))
    (setq message-required-news-headers
          (append common-headers
                  '(Newsgroups
                    (optional . X-Newsreader)
                    (Mail-Copies-To . "never"))))

I need to find a good way to add it in to mailing lists as well as
news.  And find a good way of generating Mail-Followup-To - I probably
ought to grab an Oort, or even try again at getting CVS to work
through our proxy (poxy) firewall.

Re: [patch] Fix displaying attachments with spaces etc.

[Björn Torkelsson]

Simon Josefsson <jas@extundo.com> writes:

> [1] Which package creates mailcap, btw?  dpkg isn't very helpful (the
> file exists of course, and I didn't create it):
>
> bash-2.05b$ dpkg -S /etc/mailcap
> dpkg: /etc/mailcap not found.
> bash-2.05b$

Probably mime-support

modesty[torkel]$ dlocate mailcap
...
mime-support: /etc/mailcap.order
mime-support: /usr/lib/mime/mailcap
mime-support: /usr/share/man/man1/run-mailcap.1.gz
mime-support: /usr/share/man/man5/mailcap.5.gz
mime-support: /usr/share/man/man5/mailcap.order.5.gz
mime-support: /usr/bin/run-mailcap
...

/torkel

Re: [patch] Fix displaying attachments with spaces etc.

[Lars Magne Ingebrigtsen]

Hrvoje Niksic <hniksic@xemacs.org> writes:

> 	* mm-decode.el (mm-mailcap-command): Remove the quotes around '%s'
> 	and "%s" so we don't overquote them.

I applied the bits that hadn't been applied.  (De-quoting """.) 

-- 
(domestic pets only, the antidote for overdose, milk.)
   larsi@gnus.org * Lars Magne Ingebrigtsen