Patch #27885

Empty email attachments are imported to Redmine, creating broken DB records

Added by Felix Schäfer 8 months ago. Updated 7 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Email receiving
Target version:3.3.6

Description

Redmine generally disallows empty or 0-byte-sized attachments, this is not checked when importing attachments from emails through the MailHandler though.

In the case of an empty email attachment, an Attachment record is created, though without disk_filename, disk_directory and digest. The attachment is then shown in the attachment list of the issue, but clicking on the attachment link to see the attachment or to download it leads to an error, as there is no file on disk for that attachment.

27885.patch Magnifier (3.42 KB) Felix Schäfer, 2018-01-03 14:28

Associated revisions

Revision 17147
Added by Jean-Philippe Lang 7 months ago

Empty email attachments are imported to Redmine, creating broken DB records (#27885).

Patch by Felix Schäfer.

Revision 17148
Added by Jean-Philippe Lang 7 months ago

Merged r17147 to 3.4-stable (#27885).

Revision 17149
Added by Jean-Philippe Lang 7 months ago

Merged r17147 to 3.3-stable (#27885).

History

#1 Updated by Felix Schäfer 8 months ago

The attached patch fixes this issues and adds a test for that case. The fixture email is copied from the ticket_with_attachment.eml fixture and adapted accordingly. It is probably also possible to merge this test into the general attachments for emails test in that same test file.

It might also be possible and a good idea to switch to ActsAsAttachable#save_attachments to save email attachments in the MailHandler, as in that case attachment handling in the MailHandler would not need to be manually synced with the logic of other places.

#2 Updated by Go MAEDA 8 months ago

  • Target version set to 3.3.6

LGTM.
The fix also can be applied to 3.4-stable and 3.3-stable. Setting target version to 3.3.6.

#3 Updated by Jean-Philippe Lang 7 months ago

  • Status changed from New to Closed

Committed, thanks.

#4 Updated by Felix Schäfer 7 months ago

Thank you!

Also available in: Atom PDF