Attachments made against Wiki pages with colons in their names end up in the wrong place.

[alexander@…]

A file attached to the wiki page 'Foo:Bar', will end up being attached to 'Foo/Bar'. It looks like this bug is a side-effect of this patch:

http://trac.edgewall.org/changeset/3019

An easy way to recreate this is to just attempt to attach a file to a page with a colon in it. The 'Add Attachment ...' page shows the colon replaced with a forward slash.

[cboos]

Ok, thanks for the report.

[osimons]

Still a problem with current 0.11b1+. From my brief tests that [3019] changeset does look like the cause of it. It is there for other reasons though, so need to work out a solution that keeps all things working.

It is a bit unfortunate that Wiki and Attachment handle this differently, also as it involves storing files in a structure on disk that is wrong.

I'm rescheduling it, hopefully finding a fix in the 0.11.x line.

[osimons]

#7462 closed as duplicate.

[rblank]

Here's an update on the current situation:

• The fix in [3019] was intended for accepting InterTrac links generated with .compat = true in TracIni, which look like /attachment/wiki:WikiStart:file.txt. In that case, replacing ':' with '/' conveniently fixes the path to the attachment.

• This fix was broken in [4246], where the realm part of the regexp has been changed from (wiki|ticket) to ([^/]+), which in this case matches the complete path instead of only "wiki". The path ends up empty.

The latter breakage could be fixed by taking the realm to be the string up to the first '/' or ':'. But fixing the original problem is not possible, as there is an ambiguity in the URL.

OTOH, this also means that since [4246] (21 months ago, 0.10.2), support for InterTrac links in compatibility mode has been broken, and this has never been reported. This probably means that there are few 0.9 instances pointing to >=0.10 instances through InterTrac.

I propose dropping support for <0.10 attachment InterTrac links by reverting the fix in [3019] as follows:

trac/attachment.py

@@ -345 +345 @@
     # IRequestHandler methods
 
     def match_request(self, req):
-        match = re.match(r'^/(raw-)?attachment/([^/]+)(?:[/:](.*))?$',
+        match = re.match(r'^/(raw-)?attachment/([^/]+)(?:/(.*))?$',
                          req.path_info)
         if match:
             raw, realm, path = match.groups()
@@ -353 +353 @@
                 req.args['format'] = 'raw'
             req.args['realm'] = realm
             if path:
-                req.args['path'] = path.replace(':', '/')
+                req.args['path'] = path
             return True
 
     def process_request(self, req):

Comments?

[osimons]

Nice - tested the fix and it works well for the attachment side of things.

Personally I'd rather have this serious attachment bug fixed, than support a special case from the early days of InterTrac that do not seem to be extensively used. However, should really get feedback from cboos or other 'old-timers' before committing - especially as it modifies behaviour in a stable branch.

[osimons]

Edging this one step closer to making next release - pending review.

[rblank]

Christian, would you mind taking a quick look a this one (comment:5)?

[cboos]

Fix looks good, I also think we can safely drop support for the compatibility mode here for 0.11.2 and perhaps remove any code related to this compatibility mode for 0.12, in case you find some more.

In the unlikely case there would be a problem (a "source" Trac incorrectly targeting a recent Trac in compatibility mode), it's a simple setting to change on that source Trac (setting compatibility mode to false). Since r6151 (before pre-0.11b1), compatibility mode is turned off by default anyway.

[rblank]

Thanks for the review. I'll apply the patch shortly.

[rblank]

Patch committed in [7546].