Redmine

Any reason to do this in the mail handler and not in the Attachment model? If an image is uploaded via a browser or the API with an incorrect content type, thumbnails and inline images might be broken as well.

We have only observed this for emails, but this can be moved to the model without problem too. I will adapt the patch in consequence.

Or maybe just store the provided content type and guess a better one when a serving a file, just like it's already done when an attachment have a blank content type. No need for a migration each time we change the guess rule.

Index: app/controllers/attachments_controller.rb
===================================================================
--- app/controllers/attachments_controller.rb    (revision 13994)
+++ app/controllers/attachments_controller.rb    (working copy)
@@ -183,7 +183,7 @@

   def detect_content_type(attachment)
     content_type = attachment.content_type
-    if content_type.blank?
+    if content_type.blank? || content_type == "application/octet-stream" 
       content_type = Redmine::MimeType.of(attachment.filename)
     end
     content_type.to_s

This is in fact exactly what we have done for Planio until we had worked out a solution here. I had thought the create-time guessing better as this would have to be done only once for each attachment, but that works well too and avoids additional migrations.

OpenProject (GPL3) uses another approach of attachment detection.
https://github.com/opf/openproject/commit/fb096b0c86eec9bf49612ef6c8768000c3099841

Toshi MARUYAMA wrote:

OpenProject (GPL3) uses another approach of attachment detection.

Sounds a bit overkill to me.

Thanks!