Attachments page missing ticket number

[Thijs Triemstra]

When you visit the root of an attachment page (​example) a confusing page is displayed with the title 'Ticket # - Attachments' + the project name. This also happens for a ​ticket with no attachments at all.

I would expect the ticket number to be included and display a list of available attachments.

[Christian Boos]

Actually the problem happens when the trailing slash is missing, so:

• ​http://trac.edgewall.org/attachment/ticket/698/ is OK (your example link above), but ​http://trac.edgewall.org/attachment/ticket/698 is not
• ​http://trac.edgewall.org/attachment/ticket/4500/ would be OK as well

[Remy Blank]

PatchWelcome.

[Ryan J Ollos <ryan.j.ollos@…>]

This issue has come up in several plugins: ​th:#10352, ​th:#9785, ​th:#9787.

[Christian Boos]

With the following modification, a request to /attachment/ticket/698 becomes the same as one for /attachment/ticket/698/.

trac/attachment.py

@@ -435 +435 @@
         filename = None
 
         if not parent_realm or not path:
-            raise HTTPBadRequest(_('Bad request'))
+            raise HTTPBadRequest(_("Improper attachment request"))
 
         parent_realm = Resource(parent_realm)
         action = req.args.get('action', 'view')
@@ -443 +443 @@
             parent_id = path.rstrip('/')
         else:
             segments = path.split('/')
-            parent_id = '/'.join(segments[:-1])
-            filename = len(segments) > 1 and segments[-1]
+            if len(segments) > 1:
+                parent_id, filename = '/'.join(segments[:-1]), segments[-1]
+            else:
+                parent_id, filename = segments[0], ''
 
         parent = parent_realm(id=parent_id)

There's one subtelty: /attachment/wiki/TracDev/Proposals/ would show the attachments for the wiki page TracDev/Proposals, whereas /attachment/wiki/TracDev/Proposals would show the attachment named Proposals for the page TracDev. I think that's OK.

[Christian Boos]

…and it's perhaps not worth splitting path for joining moments later:

trac/attachment.py

@@ -442 +442 @@
         if action == 'new':
             parent_id = path.rstrip('/')
         else:
-            segments = path.split('/')
-            parent_id = '/'.join(segments[:-1])
-            filename = len(segments) > 1 and segments[-1]
+            last_slash = path.rfind('/')
+            if last_slash == -1:
+                parent_id, filename = path, ''
+            else:
+                parent_id, filename = path[:last_slash], path[last_slash + 1:]
 
         parent = parent_realm(id=parent_id)

[Christian Boos]

Last patch applied in r11356.