Some inline images / attachments can not be viewed due to %0D%0A in URL

[anonymous]

One email had an inline image (in HTML source <img src="cid:part1.6D8E6A6C.5AFB627A@gmail.com" alt="">).

It is shown in the plugin as an attachment (part1.6D8E6A6C.5AFB627A@gmail.com.png; name=nkdjpgmpnpoepckd.png​ (52.4 KB) - added by (none) 7 minutes ago.).

But clicking the link (https://.../trac/attachment/mailarchive/881/part1.6D8E6A6C.5AFB627A%40gmail.com.png%3B%0D%0A%20name%3Dnkdjpgmpnpoepckd.png) leads to an error:

Bad Request - Invalid URL

HTTP Error 400. The request URL is invalid.

The file is on disk in the tracenv/files/... folder.

The problem seems to be the %0D%0A part in the URL. (Maybe it's IIS which does not allow that?)

[anonymous]

Looks like we are here​ with the following variable values:

Variable	Value
cid	<part1.6D8E6A6C.5AFB627A@gmail.com>
mimetype	image/png;\n name="nkdjpgmpnpoepckd.png"
ext	png;\n name="nkdjpgmpnpoepckd.png"
filename (before translate)	<part1.6D8E6A6C.5AFB627A@gmail.com>.png;\n name="nkdjpgmpnpoepckd.png"

So something like this could help:

import re
# ...
m = re.match("^[\w/]+;\r\n name=\"([\w\d\.]+)\"$", mimetype)
if m:
    filename = str(m.group(1))
else:
    # ...

[anonymous]

Or better use something like this:

def parse_mime_type(mime_type):
    """Parse a mime-type into its parts.

    Example: 'image/png; name="a.png"'
    becomes: ('image/png', {'name': '"a.png"'})
    """
    parts = mime_type.split(';')
    type = parts[0].strip()
    params = dict([tuple([s.strip()
                          for s in param.split('=', 1)])
                   for param in parts[1:]])
    return (type, params)

and then:

mimetype, mimetype_params = parse_mime_type(mimetype)
if "name" in mimetype_params:
    filename = mimetype_params["name"]
else:
    # ...

[anonymous]

And add % to the filtered characters?

[Jun Omae]

We could simply use part.get_content_type() rather than mimetype = part.get('Content-Type') and parsing.

>>> from email import message_from_string
>>> m = message_from_string("""From: <x>
... Content-Type: image/png;\n name="nkdjpgmpnpoepckd.png"
...
... body
... """)
>>> m.get('Content-Type')
'image/png;\n name="nkdjpgmpnpoepckd.png"'
>>> m.get_content_type()
'image/png'

Also, filename of attachment cannot have control characters. Those characters would be trimmed by trac.attachment module.

[anonymous]

Thanks, I was not aware. So we can use this to get the intended filename (nkdjpgmpnpoepckd.png):

filename = part.get_param('name')
if not filename:
    mimetype = part.get_content_type()
    ext = EXT_MAP.get(mimetype) # if ... else ...
    filename = cid + '.' + ext

Also, filename of attachment cannot have control characters. Those characters would be trimmed by trac.attachment module.

Are you sure? Unfortunately it seems to me filename is not trimmed automatically, but used as-is, leading to this problem.

[Jun Omae]

Replying to anonymous:

Thanks, I was not aware. So we can use this to get the intended filename (nkdjpgmpnpoepckd.png):

filename = part.get_param('name')
if not filename:
    mimetype = part.get_content_type()
    ext = EXT_MAP.get(mimetype) # if ... else ...
    filename = cid + '.' + ext

We could use ​get_filename() to retrieve attached filename.

>>> m1 = message_from_string("""\
... Content-Type: image/png; name="file-1.png"
... Content-Transfer-Encoding: base64
... Content-ID: <image-0.png>
... Content-Disposition: inline; filename="file-1.png"
... """)
>>> m1.get_filename()
'file-1.png'
>>>
>>> m2 = message_from_string("""\
... Content-Transfer-Encoding: base64
... Content-ID: <image-0.png>
... Content-Disposition: inline; filename="file-2.png"
... """)
>>> m2.get_filename()
'file-2.png'
>>>
>>> m3 = message_from_string("""\
... Content-Type: image/png; name="file-3.png"
... Content-Transfer-Encoding: base64
... Content-ID: <image-0.png>
... """)
>>> m3.get_filename()
'file-3.png'
>>>
>>> m4 = message_from_string("""\
... Content-Type: image/png
... Content-Transfer-Encoding: base64
... Content-ID: <image-0.png>
... """)
>>> m4.get_filename() is None
True

Also, filename of attachment cannot have control characters. Those characters would be trimmed by trac.attachment module.

Are you sure? Unfortunately it seems to me filename is not trimmed automatically, but used as-is, leading to this problem.

Sorry for my misunderstanding. Adding an attachment via trac-admin and web ui, Trac replaces control characters with white spaces since 1.2. See ​trac:r13603.

Root cause of unable to view attachment named with LF characters is that . in regular expression doesn't match LF in req.path_info.

        match = re.match(r'/(raw-|zip-)?attachment/([^/]+)(?:/(.*))?$',
                         req.path_info)

See also:

• ​https://groups.google.com/d/topic/trac-dev/fl-36eCmuYQ/discussion
• ​trac:source:/tags/trac-1.2.2/trac/attachment.py@:164-165#L163

I think this plugin should normalize filename with the same logic in Trac. See ​trac:source:/tags/trac-1.2.2/trac/attachment.py@:1127-1141#L1127.

[anonymous]

Thanks for all the research! So maybe something like this:

mailarchive/model.py

@@ -5 +5 @@
 from email.header import decode_header
 from email.utils import parsedate_tz, mktime_tz
 from tempfile import TemporaryFile
+import unicodedata
+import re
 
 from trac.attachment import Attachment
 from trac.db import Table, Column, Index
 from trac.mimeview.api import KNOWN_MIME_TYPES
 from trac.resource import Resource
 from trac.util.datefmt import from_utimestamp, to_utimestamp, utc
+from trac.util.text import stripws
 
 
 SCHEMA = [
@@ -34 +37 @@
 EXT_MAP['image/tiff'] = 'tiff'
 EXT_MAP['image/svg+xml'] = 'svg'
 
+DELETE_CHARS_RE = re.compile(
+    '[' +
+    ''.join(filter(lambda c: unicodedata.category(c) == 'Cc',
+                   map(unichr, xrange(0x10000)))) +
+    '\\/:*?"<>|' +
+    ']')
+
+def normalized_filename(filename):
+    filename = DELETE_CHARS_RE.sub(' ', filename)
+    filename = stripws(filename)
+    return filename
+
 def terms_to_clauses(terms):
     """Split list of search terms and the 'or' keyword into list of lists of search terms."""
     clauses = [[]]
@@ -125 +140 @@
     @classmethod
     def storeattachments(cls, env, mail, msg):
         def add_attachment(payload, filename):
+            filename = normalized_filename(filename)
             with TemporaryFile('w+b') as file:
                 file.write(payload)
                 size = file.tell()
@@ -143 +159 @@
                         add_attachment('Invalid attachment: message/rfc822 parts can not be base64 encoded!', part.get_filename())
                         continue
                     add_attachment(part.get_payload(decode=True), part.get_filename())
+                    continue
 
             cid = part.get('Content-ID')
             if cid:
-                mimetype = part.get('Content-Type')
-                ext = EXT_MAP.get(mimetype) if mimetype in EXT_MAP else \
-                    mimetype[mimetype.rindex('/')+1:] if '/' in mimetype else \
-                    mimetype
-                filename = cid + '.' + ext
-                deletechars = '\/:*?"<>|'
-                filename = filename.translate(None, deletechars)
+                filename = part.get_filename()
+                if not filename:
+                    mimetype = part.get_content_type()
+                    ext = EXT_MAP.get(mimetype) if mimetype in EXT_MAP else \
+                        mimetype[mimetype.rindex('/')+1:] if '/' in mimetype else \
+                        mimetype
+                    filename = cid + '.' + ext
+                
                 add_attachment(part.get_payload(decode=True), filename)
 
     @classmethod

[lucid]

In 16717:

MailArchivePlugin: Fix attachment filenames.
(see #13243)

[lucid]

Thanks.

Use trac-admin <path/to/tracenv/> mailarchive fix-attachment-filenames to fix old filenames somewhat.

(See ​trac:ticket:12870 for renaming attachment API.)