[PATCH] Download/save all attachments as .zip file

[mark@…]

We often have several attachments to each ticket. Downloading each of them separately can be quite tedious, and to ease this process I added a new "Save all as ZIP-file" button to the attachments list.

Attached is a patch file for patching Trac 0.10.4. Feel free to use it or possibly include it in a future version of Trac if you find it useful.

[cboos]

Interesting, thanks for the patch!

[mark@…]

Patch adapted for trac 0.11.0

[cboos]

Milestone 1.0 deleted

[rblank]

Allow downloading all attachments of a resource as a ZIP file.

[rblank]

7078-download-all-r10322.patch​ adds "Download all" links next to the "Attachments" title, and allows downloading all attachments of a resource as a ZIP file.

I'm not happy with the placement of the "Download all" link. It's nice on the ticket page, but rather ugly on wiki pages. I haven't found a way to place it right next to the "Attachments" title (i.e. not at the far right) without interfering with folding.

The links are marked with rel="nofollow, in the same way as the alternate format links. Should we also add that attribute to the raw download icons for attachment and source links? Ideally, we should prevent following the links (which is not what rel="nofollow" is about, unfortunately), but this doesn't seem to be possible on a per-link basis.

Also, there is currently no limit on the size of the resulting ZIP, which is built in-memory. Should I add an [attachment] max_download_all_size option for that, and remove the link if the total size of the attachments exceeds that limit?

Comments and suggestions welcome (as always).

[cboos]

Replying to rblank:

I'm not happy with the placement of the "Download all" link. It's nice on the ticket page, but rather ugly on wiki pages. I haven't found a way to place it right next to the "Attachments" title (i.e. not at the far right) without interfering with folding.

I haven't tried the patch yet, but I would expect to see the Download all at the bottom of the list (same idea as for normal pages "Download in alternate formats:" at the bottom).

e.g.

Attachments

• PyQt-Py2.6-gpl-4.6-1.exe (17.6 MB) - added by cboos 16 months ago.
• PyQt-Py2.6-gpl-4.6.1-1.exe (17.3 MB) - added by cboos 15 months ago.
• Download all attachments as: .zip

Also, rather than ?action=download we should use /zip-attachment/... as this is more robots.txt friendly.

[rblank]

All good ideas, I'll implement them.

[rblank]

Updated patch.

[rblank]

7078-download-all-r10500.patch​ implements the ideas from comment:6.

[rblank]

Last patch applied in [10551].

One last question: currently, no checks are done as to the size of the resulting .zip. We limit the maximum size of a single attachment with [attachment] max_size, but we don't limit the number of attachments, so theoretically a malicious user could create lots of attachments, and try to download the .zip, in the hope of generating a DoS.

Should I add a [attachment] zip_max_attachments option to limit the number of attachments allowed for a .zip? Or maybe rather a [attachment] max_zip_size option to limit the maximum total size of attachments downloadable as a .zip? I tend to prefer the latter, suggested patch coming shortly.

[rblank]

Limit the total size of attachments to be downloadable as a .zip.

[rblank]

7078-limit-zip-download-r10551.patch​ adds an option [attachment] max_zip_size to specify the maximum total size of a list of attachments to be downloadable as a .zip. It can be set to -1 to disable the functionality.

[cboos]

I've read the patch, it's perfect (well, except for the nitpick that I personally try to use "..." strings for messages and '...' for string constants, but I'm probably the only one to find that useful ;-) ).

[rblank]

Replying to cboos:

except for the nitpick that I personally try to use "..." strings for messages and '...' for string constants, but I'm probably the only one to find that useful ;-)

You know, it's funny you should mention that. I started doing exactly that in my plugin. In core, there's no clear tendency about that, both are equally present.

[rblank]

Patch applied in [10553]. Thanks for reviewing!

[bsheridan@…]

I believe the code added for the total_size check breaks the functionality. Since Attachment.select is implemented as a generator, I believe the line

total_size = sum(attachment.size for attachment in attachments)

consumes it and iterating it later...

for attachment in attachments:
    ...

is empty.

Collapsing the generator into a list first seems to correct the problem for us.

attachment.py

@@ -719 +721 @@
 
     def _download_as_zip(self, req, parent, attachments=None):
         if attachments is None:
-            attachments = Attachment.select(self.env, parent.realm, parent.id)
+            attachments = [a for a in Attachment.select(self.env, parent.realm, parent.id)]
         total_size = sum(attachment.size for attachment in attachments)
         if total_size > self.max_zip_size:
             raise TracError(_("Maximum total attachment size: %(num)s bytes",

[rblank]

Replying to bsheridan@…:

I believe the code added for the total_size check breaks the functionality.

This has been fixed already by Christian in [10588]. Please update!