Subject: Re: [sup-devel] rmail gem is faulty

From: Matthieu Rakotojaona <[email-masked]>
Date: Fri, 18 Nov 2011 20:19:23 +0100

On Fri, Nov 18, 2011 at 8:17 PM, Matthieu Rakotojaona
<[email-masked]> wrote:
> The result is the patch enclosed. All I did was in
> lib/heliotrope/message.rb, so everything else works (mainly).

Hem.

-- 
Matthieu RAKOTOJAONA
From 62772e81b01169803eb9647342523e8149e2175d Mon Sep 17 00:00:00 2001
From: rakoo <[email-masked]>
Date: Fri, 18 Nov 2011 19:23:23 +0100
Subject: [PATCH] modified message.rb to use 'mail' gem

Before we used rmail, but it is not maintained anymore and faulty
---
 lib/heliotrope/message.rb |  110 ++++++++++++++++++++++++++++-----------------
 1 files changed, 69 insertions(+), 41 deletions(-)

diff --git a/lib/heliotrope/message.rb b/lib/heliotrope/message.rb
index fd83cee..f7cf0e7 100644
--- a/lib/heliotrope/message.rb
+++ b/lib/heliotrope/message.rb
@@ -1,10 +1,23 @@
 # encoding: UTF-8
 
-require 'rmail'
+require 'mail'
 require 'digest/md5'
 require 'json'
 require 'timeout'
 
+module Mail
+class Message
+	def fetch value
+		if self[value].nil?
+			return nil
+		else
+			return self[value].decoded.to_s
+		end
+	end
+end
+end
+
+
 module Heliotrope
 class InvalidMessageError < StandardError; end
 class Message
@@ -14,42 +27,61 @@ class Message
   end
 
   def parse!
-    @m = RMail::Parser.read @rawbody
+    @m = Mail.read_from_string @rawbody
 
-    @msgid = find_msgids(decode_header(validate_field(:message_id, @m.header["message-id"]))).first
-    ## this next error happens if we have a field, but we can't find a <something> in it
-    raise InvalidMessageError, "can't parse msgid: [email-masked]['message-id']}" unless @msgid
+		@msgid = @m[:message_id].message_id
+		## this next error happens if we have a field, but we can't find a <something> in it
+    raise InvalidMessageError, "can't parse msgid: #{@m[:message_id].message_id}" unless @msgid
     @safe_msgid = munge_msgid @msgid
 
-    @from = Person.from_string decode_header(validate_field(:from, @m.header["from"]))
+		# From can contain multiple mailboxes. If it does, it MUST contain a
+		# Sender: field, which we will use. If it does not, it doesn't respect
+		# RFC5322, but we will use the first email address of the From: header
+		@from = 
+			if @m[:from].addresses.size > 1
+				if @m[:sender].nil?
+					Person.from_string validate_field(:from, @m[:from].formatted.first.to_s)
+				else
+					Person.from_string validate_field(:sender, @m[:sender].formatted.first.to_s)
+				end
+			else
+				Person.from_string validate_field(:from, @m[:from].decoded.to_s)
+			end
+		
+		@sender = begin 
+			Person.from_string validate_field(:sender, @m[:sender].formatted.first.to_s) unless @m[:sender].nil?
+		rescue InvalidMessageError
+			""
+		end
+
     @date = begin
-      Time.parse(validate_field(:date, @m.header["date"])).to_i
+			Time.parse(@m.date.to_s).to_i
     rescue ArgumentError
       #puts "warning: invalid date field [email-masked]['date']}"
       0
     end
 
-    @to = Person.many_from_string decode_header(@m.header["to"])
-    @cc = Person.many_from_string decode_header(@m.header["cc"])
-    @bcc = Person.many_from_string decode_header(@m.header["bcc"])
-    @subject = decode_header @m.header["subject"]
-    @reply_to = Person.from_string @m.header["reply-to"]
+		@to = @m[:to].nil? ? [] : Person.many_from_string(@m[:to].decoded.to_s)
+		@cc = @m[:cc].nil? ? [] : Person.many_from_string(m[:cc].decoded.to_s)
+		@bcc = @m[:bcc].nil? ? [] : Person.many_from_string(@m[:bcc].decoded.to_s)
+
+		@subject = @m.subject || "" #we can store and retrieve UTF-8 ...
 
-    @refs = find_msgids decode_header(@m.header["references"] || "")
-    in_reply_to = find_msgids decode_header(@m.header["in-reply-to"] || "")
-    @refs += in_reply_to unless @refs.member? in_reply_to.first
-    @safe_refs = @refs.map { |r| munge_msgid(r) }
+    @refs = @m[:references].nil? ? [] : @m[:references].message_ids.map{ |m| decode_header m} 
+		in_reply_to = @m[:in_reply_to].nil? ? [] : @m[:in_reply_to].message_ids{ |m| decode_header m}
+		@refs += in_reply_to unless @refs.member?(in_reply_to.first)
+		@safe_refs = @refs.nil? ? [] : @refs.map { |r| munge_msgid(r) }
 
     ## various other headers that you don't think we will need until we
     ## actually need them.
 
     ## this is sometimes useful for determining who was the actual target of
     ## the email, in the case that someone has aliases
-    @recipient_email = @m.header["envelope-to"] || @m.header["x-original-to"] || @m.header["delivered-to"]
+    @recipient_email = @m.fetch("envelope-to") || @m.fetch("x-original-to") || @m.fetch("delivered-to")
 
-    @list_subscribe = @m.header["list-subscribe"]
-    @list_unsubscribe = @m.header["list-unsubscribe"]
-    @list_post = @m.header["list-post"] || @m.header["x-mailing-list"]
+    @list_subscribe = @m.fetch("list-subscribe")
+    @list_unsubscribe = @m.fetch("list-unsubscribe")
+    @list_post = @m.fetch("list-post") || @m.fetch("x-mailing-list")
 
     self
   end
@@ -90,8 +122,8 @@ class Message
   end
 
   def direct_recipients; to end
-  def indirect_recipients; cc + bcc end
-  def recipients; direct_recipients + indirect_recipients end
+  def indirect_recipients; (cc || []) + (bcc || []) end
+  def recipients; (direct_recipients || []) + (indirect_recipients || []) end
 
   def indexable_text
     @indexable_text ||= begin
@@ -128,12 +160,9 @@ class Message
     ""
   end
 
-  def has_attachment?
-    @has_attachment ||=
-      mime_parts("text/plain").any? do |type, fn, id, content|
-        fn && (type !~ SIGNATURE_ATTACHMENT_TYPE)
-    end
-  end
+	def has_attachment?
+		@m.has_attachments? # defined in the mail gem
+	end
 
   def signed?
     @signed ||= mime_part_types.any? { |t| t =~ SIGNED_MIME_TYPE }
@@ -148,7 +177,7 @@ class Message
   end
 
 private
-
+	
   ## hash the fuck out of all message ids. trust me, you want this.
   def munge_msgid msgid
     Digest::MD5.hexdigest msgid
@@ -159,8 +188,8 @@ private
   end
 
   def mime_part_types part=@m
-    ptype = part.header["content-type"] || ""
-    [ptype] + (part.multipart? ? part.body.map { |sub| mime_part_types sub } : [])
+    ptype = part.fetch("content-type") || ""
+    [ptype] + (part.multipart? ? part.body.parts.map { |sub| mime_part_types sub } : [])
   end
 
   ## unnests all the mime stuff and returns a list of [type, filename, content]
@@ -171,14 +200,14 @@ private
   def decode_mime_parts part, preferred_type, level=0
     if part.multipart?
       if mime_type_for(part) =~ /multipart\/alternative/
-        target = part.body.find { |p| mime_type_for(p).index(preferred_type) } || part.body.first
+        target = part.body.parts.find { |p| mime_type_for(p).index(preferred_type) } || part.body.first
         if target # this can be nil
           decode_mime_parts target, preferred_type, level + 1
         else
           []
         end
       else # decode 'em all
-        part.body.compact.map { |subpart| decode_mime_parts subpart, preferred_type, level + 1 }.flatten 1
+        part.body.parts.compact.map { |subpart| decode_mime_parts subpart, preferred_type, level + 1 }.flatten 1
       end
     else
       type = mime_type_for part
@@ -199,11 +228,11 @@ private
   end
 
   def mime_type_for part
-    (part.header["content-type"] || "text/plain").gsub(/\s+/, " ").strip.downcase
+    (part.fetch("content-type") || "text/plain").gsub(/\s+/, " ").strip.downcase
   end
 
   def mime_id_for part
-    header = part.header["content-id"]
+    header = part.fetch("content-id")
     case header
       when /<(.+?)>/; $1
       else header
@@ -212,8 +241,8 @@ private
 
   ## a filename, or nil
   def mime_filename_for part
-    cd = part.header["Content-Disposition"]
-    ct = part.header["Content-Type"]
+    cd = part.fetch("Content-Disposition")
+    ct = part.fetch("Content-Type")
 
     ## RFC 2183 (Content-Disposition) specifies that disposition-parms are
     ## separated by ";". So, we match everything up to " and ; (if present).
@@ -250,11 +279,10 @@ private
   def mime_content_for mime_part, preferred_type
     return "" unless mime_part.body # sometimes this happens. not sure why.
 
-    mt = mime_type_for(mime_part) || "text/plain" # i guess
-    content_type = if mt =~ /^(.+);/ then $1.downcase else mt end
-    source_charset = if mt =~ /charset="?(.*?)"?(;|$)/i then $1 else "US-ASCII" end
+		content_type = mime_part.header[:content_type].nil? ? "text/plain" : mime_part.header[:content_type].string
+		source_charset = mime_part.charset || "US-ASCII"
 
-    content = mime_part.decode
+    content = mime_part.decoded
     converted_content, converted_charset = if(converter = CONVERSIONS[[content_type, preferred_type]])
       send converter, content, source_charset
     else
-- 
1.7.7.3

_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: Matthieu Rakotojaona <[email-masked]>
Date: Fri, 18 Nov 2011 20:17:35 +0100

Hello everyone,

I have seen that 'rmail' doesn't parse some of my mails correctly.
Specifically, it includes the body in the header fields (I don't  know
how it manages to do that).
Moreover, last version dates from 2008. So I replaced it with the
'mail' gem [0], more up-to-date and updated, with some useful methods
that we don't have to define specifically in heliotrope.

[0] https://github.com/mikel/mail