Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 12 months ago

#10850 closed defect (fixed)

When attaching a file that contains colons, only part after colon is displayed

Reported by: info@… Owned by: Jun Omae
Priority: low Milestone: 0.12.5
Component: attachment Version:
Severity: normal Keywords: attachment filename
Cc: Branch:
Release Notes:

Correctly handle a attachment file named with colon and backslash characters

API Changes:

Description

I have files created by a script that contain a timestamp in their filename. E.g.

2012-09-11_15:36:40-test.tbz2

When I add this to a wiki entry as an attachment, it gets renamed to

40-test.tbz2

which looks like some internal quoting glitch to me.

Attachments (1)

40-test.tbz2 (5 bytes ) - added by Christian Boos 6 years ago.
testing…

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Christian Boos

Attachment: 40-test.tbz2 added

testing…

comment:1 Changed 6 years ago by Christian Boos

Component: generalattachment
Milestone: next-stable-1.0.x
Priority: normallow

Reproduced. If the fix is not too disruptive, it could apply to milestone:0.12.5 as well.

comment:2 Changed 6 years ago by Jun Omae

The issue is caused by replacing a colon character with a slash character at branches/0.12-stable/trac/attachment.py@10837#L660.

That code is probably for Internet Explorer which uploads with full path as uploading file, however, I think we can remove that code.

And, we should use posixpath.basename instead of os.path.basename to determine the base name of the uploaded file. os.path.basename on Windows separates with slash, backslash and colon characters.

[9d8c6e5a/jomae.git]

comment:3 Changed 6 years ago by Christian Boos

I confirm that the proposed change fixes the issue, and that besides upload still works well from IE.

However I see now that we'll get a similar issue with \ in the filename. Like for ":", we might have those in filenames on Unix.

comment:4 Changed 6 years ago by Jun Omae

Right. However we should replace backslashes when a browser sends Windows full path as the uploaded file. It depends on the security settings in Internet Explorer.

  • trac/attachment.py

    diff --git a/trac/attachment.py b/trac/attachment.py
    index f0731b5..29f6f75 100644
    a b class AttachmentModule(Component):  
    658658        # Files uploaded from OS X might be in NFD.
    659659        filename = unicodedata.normalize('NFC', unicode(upload.filename,
    660660                                                        'utf-8'))
    661         filename = filename.strip().replace('\\', '/')
     661        filename = filename.strip()
     662        # Replace backslashes with slashes if filename is Windows full path
     663        if filename.startswith('\\') or re.match(r'[A-Za-z]:\\', filename):
     664            filename = filename.replace('\\', '/')
    662665        # We want basename to be delimited by only slashes on all platforms
    663666        filename = posixpath.basename(filename)
    664667        if not filename:

comment:5 Changed 6 years ago by Jun Omae

[9d8c6e5a/jomae.git] is to keep colons and [39f4193b/jomae.git] is to keep backslashes (the patch in comment:4 with unit tests). I'll apply to 0.12-stable tonight.

comment:6 Changed 6 years ago by Christian Boos

Fine by me!

The last problem with attachments containing a : in their name is that we can't link to them with the usual TracLinks syntax: e.g. attachment:the:filename:wiki:WikiStart won't work.

Given how rare accessing such filenames is, I'm not sure it's worth trying to "fix" that though, given one could always use the explicit URL [/attachment/wiki/WikiStart/A%3AFILE].

comment:7 Changed 6 years ago by Jun Omae

Milestone: next-stable-1.0.x0.12.5
Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

Given how rare accessing such filenames is, I'm not sure it's worth trying to "fix" that though, given one could always use the explicit URL [/attachment/wiki/WikiStart/A%3AFILE].

Agreed. But if did, I think I would add attachment:"the:filename.txt" to TracLinks syntax….

The patches are applied in [11365-11367].

comment:8 Changed 6 years ago by Jun Omae

Owner: set to Jun Omae

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.