[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [sup-devel] rmail gem is faulty



On Fri, Nov 18, 2011 at 8:17 PM, Matthieu Rakotojaona
<matthieu.rakotojaona@gmail.com> 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 <matthieu.rakotojaona@gmail.com>
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: #{@m.header['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 #{@m.header['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
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel