Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Regexp instead of string comparison for message/rfc822 #684

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

vongruenigen commented Mar 5, 2014

Some mail clients (e.g. Thunderbird) add an extra name field to the Content-Type header when attaching an .eml file.

In my case, the header then was something like this..

Content-Type: message/rfc822;
  name="ForwardedMessage.eml";

..instead of

Content-Type: message/rfc822;

This led to problems when trying to get all attachments recurisvely through the Mail::AttachmentList class.

Luckily, the solution was pretty simple: Instead of doing a hard string comparison when checking the Content-Type header of a part, I changed it to use a regular expression instead. Also adjusted the fixture mail to ensure that its actually tested.

@nisanth074 nisanth074 commented on the diff Apr 3, 2014

lib/mail/attachments_list.rb
@@ -5,7 +5,7 @@ def initialize(parts_list)
@parts_list = parts_list
@content_disposition_type = 'attachment'
parts_list.map { |p|
- if p.content_type == "message/rfc822"
+ if p.content_type =~ %r/message\/rfc822/
@nisanth074

nisanth074 Apr 3, 2014

Could be

if p.mime_type == 'message/rfc822'
@vongruenigen

vongruenigen Apr 4, 2014

Contributor

Yes, that's also working. But IMO it's more clear to match a regexp against a "real" header instead of using a method where it's not 100% obvious where the value is coming from.

Contributor

grosser commented Jul 21, 2014

FYI I'm using split(";").first == 'message/rfc822'

@jeremy jeremy added this to the 2.7.0 milestone May 16, 2017

@jeremy jeremy closed this in efc735f May 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment