Merge commit 'origin/various-mbox-fixes'

Conflicts:

	lib/sup/mbox.rb
	test/test_mbox_parsing.rb

diff --git a/lib/sup/mbox.rb b/lib/sup/mbox.rb
index 5dd89b707d4709678d346a9f4490e2a3a672f101..0d941b1d8ae341b86bcf6303e752c3c4bb1c36e5 100644 (file)
--- a/lib/sup/mbox.rb
+++ b/lib/sup/mbox.rb
@@ -6,6 +6,20 @@ require "sup/rfc2047"
 module Redwood
 
 module MBox
-  BREAK_RE = /^From \S+/ ######### TODO REMOVE ME
+  BREAK_RE = /^From \S+ (.+)$/
+
+  def is_break_line? l
+    l =~ BREAK_RE or return false
+    time = $1
+    begin
+      ## hack -- make Time.parse fail when trying to substitute values from Time.now
+      Time.parse time, 0
+      true
+    rescue NoMethodError
+      Redwood::log "found invalid date in potential mbox split line, not splitting: #{l.inspect}"
+      false
+    end
+  end
+  module_function :is_break_line?
 end
 end

diff --git a/lib/sup/mbox/loader.rb b/lib/sup/mbox/loader.rb
index ad52dfb1087fae4d3d794fbe8a9d1e46bdbbad52..ebb2aed675e171397b99e877023e24611cf119c7 100644 (file)
--- a/lib/sup/mbox/loader.rb
+++ b/lib/sup/mbox/loader.rb
@@ -9,7 +9,7 @@ class Loader < Source
   attr_accessor :labels
 
   ## uri_or_fp is horrific. need to refactor.
-  def initialize uri_or_fp, start_offset=nil, usual=true, archived=false, id=nil, labels=[]
+  def initialize uri_or_fp, start_offset=0, usual=true, archived=false, id=nil, labels=[]
     @mutex = Mutex.new
     @labels = ((labels || []) - LabelManager::RESERVED_LABELS).uniq.freeze
 
@@ -56,7 +56,7 @@ class Loader < Source
     @mutex.synchronize do
       @f.seek offset
       l = @f.gets
-      unless l =~ BREAK_RE
+      unless MBox::is_break_line? l
         raise OutOfSyncSourceError, "mismatch in mbox file offset #{offset.inspect}: #{l.inspect}." 
       end
       header = parse_raw_email_header @f
@@ -68,13 +68,12 @@ class Loader < Source
     @mutex.synchronize do
       @f.seek offset
       begin
-        RMail::Mailbox::MBoxReader.new(@f).each_message do |input|
-          m = RMail::Parser.read(input)
-          if m.body && m.body.is_a?(String)
-            m.body.gsub!(/^>From /, "From ")
-          end
-          return m
-        end
+        ## don't use RMail::Mailbox::MBoxReader because it doesn't properly ignore
+        ## "From" at the start of a message body line.
+        string = ""
+        l = @f.gets
+        string << l until @f.eof? || MBox::is_break_line?(l = @f.gets)
+        RMail::Parser.read string
       rescue RMail::Parser::Error => e
         raise FatalSourceError, "error parsing mbox file: #{e.message}"
       end
@@ -120,7 +119,7 @@ class Loader < Source
     @mutex.synchronize do
       @f.seek offset
       yield @f.gets
-      until @f.eof? || (l = @f.gets) =~ BREAK_RE
+      until @f.eof? || MBox::is_break_line?(l = @f.gets)
         yield l
       end
     end
@@ -141,7 +140,7 @@ class Loader < Source
         ## 2. at the beginning of an mbox separator (in all other
         ##    cases).
 
-        l = @f.gets or raise "next while at EOF"
+        l = @f.gets or return nil
         if l =~ /^\s*$/ # case 1
           returned_offset = @f.tell
           @f.gets # now we're at a BREAK_RE, so skip past it
@@ -151,7 +150,7 @@ class Loader < Source
         end
 
         while(line = @f.gets)
-          break if line =~ BREAK_RE
+          break if MBox::is_break_line? line
           next_offset = @f.tell
         end
       end

diff --git a/lib/sup/message.rb b/lib/sup/message.rb
index 3532b180c17031eac0df8ca6b4aef2e84fcebcd8..6dd1f7dfa3fa67ed9dd70a39460fa0d08ec73537 100644 (file)
--- a/lib/sup/message.rb
+++ b/lib/sup/message.rb
@@ -190,7 +190,7 @@ class Message
   ## this is called when the message body needs to actually be loaded.
   def load_from_source!
     @chunks ||=
-      if @source.has_errors?
+      if @source.respond_to?(:has_errors?) && @source.has_errors?
         [Chunk::Text.new(error_message(@source.error.message).split("\n"))]
       else
         begin
@@ -364,6 +364,7 @@ private
     [notice, sig, children].flatten.compact
   end
 
+  ## takes a RMail::Message, breaks it into Chunk:: classes.
   def message_to_chunks m, encrypted=false, sibling_types=[]
     if m.multipart?
       chunks =

diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb
index 5cd1043f6b7859d5feb5ae1332d5d0ba0f39f6f3..fb4abb2526771422579020407bb0ba671391485a 100644 (file)
--- a/lib/sup/poll.rb
+++ b/lib/sup/poll.rb
@@ -86,7 +86,7 @@ EOS
       Index.usual_sources.each do |source|
 #        yield "source #{source} is done? #{source.done?} (cur_offset #{source.cur_offset} >= #{source.end_offset})"
         begin
-          yield "Loading from #{source}... " unless source.done? || source.has_errors?
+          yield "Loading from #{source}... " unless source.done? || (source.respond_to?(:has_errors?) && source.has_errors?)
         rescue SourceError => e
           Redwood::log "problem getting messages from #{source}: #{e.message}"
           Redwood::report_broken_sources :force_to_top => true

diff --git a/test/dummy_source.rb b/test/dummy_source.rb
index b84e64e8da0cf1a6b2b45a10be192cd44d3233c6..83790c5dcaeb26ff716bb7b87537373eb8a79def 100644 (file)
--- a/test/dummy_source.rb
+++ b/test/dummy_source.rb
@@ -53,13 +53,6 @@ class DummySource < Source
       yield f.gets
     end
   end
-
-  # FIXME: this one was not mentioned in the source documentation, but
-  # it's still required
-  def has_errors?
-
-  end
-
 end
 
 end

diff --git a/test/test_header_parsing.rb b/test/test_header_parsing.rb
index 7368d81ff7bed7423472391450555f24ae887fa3..91cf7c76f66b020874a5e2bedece3c8fc83f626c 100644 (file)
--- a/test/test_header_parsing.rb
+++ b/test/test_header_parsing.rb
@@ -104,4 +104,54 @@ EOS
   assert_equal "Bob <bob@bob.com>", h["from"]
   assert_nil h["to"]
   end
+
+  def test_from_line_splitting
+    l = MBox::Loader.new StringIO.new(<<EOS)
+From sup-talk-bounces@rubyforge.org Mon Apr 27 12:56:18 2009
+From: Bob <bob@bob.com>
+To: a dear friend
+
+Hello there friend. How are you?
+
+From sea to shining sea
+
+From bob@bob.com I get only spam.
+
+From bob@bob.com   
+
+From bob@bob.com
+
+(that second one has spaces at the endj
+
+This is the end of the email.
+EOS
+    offset, labels = l.next
+    assert_equal 0, offset
+    offset, labels = l.next
+    assert_nil offset
+  end
+
+  def test_more_from_line_splitting
+    l = MBox::Loader.new StringIO.new(<<EOS)
+From sup-talk-bounces@rubyforge.org Mon Apr 27 12:56:18 2009
+From: Bob <bob@bob.com>
+To: a dear friend
+
+Hello there friend. How are you?
+
+From bob@bob.com Mon Apr 27 12:56:19 2009
+From: Bob <bob@bob.com>
+To: a dear friend
+
+Hello again! Would you like to buy my products?
+EOS
+    offset, labels = l.next
+    assert_not_nil offset
+
+    offset, labels = l.next
+    assert_not_nil offset
+
+    offset, labels = l.next
+    assert_nil offset
+  end
 end