Subject: [sup-devel] Mysterious nil errors

From: "Edward Z. Yang" <[email-masked]>
Date: Tue, 09 Mar 2010 23:50:23 -0500

Here are some patches that correct some mysterious Nil errors.  I don't
know what messages caused them or if they're correct.  I don't really want
them to go in, but I would like to know further directions.

diff --git a/lib/sup/message.rb b/lib/sup/message.rb
index ebc73fc..ec12039 100644
--- a/lib/sup/message.rb
+++ b/lib/sup/message.rb
@@ -436,12 +436,12 @@ private
         end
 
       unless chunks
-        sibling_types = m.body.map { |p| p.header.content_type }
+        sibling_types = m.body.map { |p| p.header["content_type"] ? p.header.content_type : "application/unknown" }
         chunks = m.body.map { |p| message_to_chunks p, encrypted, sibling_types }.flatten.compact
       end
 
       chunks
-    elsif m.header.content_type && m.header.content_type.downcase == "message/rfc822"
+    elsif m.header["content_type"] && m.header.content_type && m.header.content_type.downcase == "message/rfc822"
       if m.body
         payload = RMail::Parser.read(m.body)
         from = payload.header.from.first ? payload.header.from.first.format : ""
@@ -458,7 +458,7 @@ private
         debug "no body for message/rfc822 enclosure; skipping"
         []
       end
-    elsif m.header.content_type && m.header.content_type.downcase == "application/pgp" && m.body
+    elsif m.header["content_type"] && m.header.content_type && m.header.content_type.downcase == "application/pgp" && m.body
       ## apparently some versions of Thunderbird generate encryped email that
       ## does not follow RFC3156, e.g. messages with X-Enigmail-Version: 0.95.0
       ## they have no MIME multipart and just set the body content type to
@@ -503,7 +503,7 @@ private
         # Lowercase the filename because searches are easier that way 
         @attachments.push filename.downcase unless filename =~ /^sup-attachment-/
         add_label :attachment unless filename =~ /^sup-attachment-/
-        content_type = (m.header.content_type || "application/unknown").downcase # sometimes RubyMail gives us nil
+        content_type = (m.header["content_type"] && m.header.content_type || "application/unknown").downcase # sometimes RubyMail gives us nil
         [Chunk::Attachment.new(content_type, filename, m, sibling_types)]
 
       ## otherwise, it's body text
diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
index f53001f..84399d3 100644
--- a/lib/sup/modes/thread-index-mode.rb
+++ b/lib/sup/modes/thread-index-mode.rb
@@ -231,7 +231,7 @@ EOS
     old_cursor_thread = cursor_thread
     @mutex.synchronize do
       ## let's see you do THIS in python
-      @threads = @ts.threads.select { |t| !@hidden_threads[t] }.sort_by { |t| [t.date, t.first.id] }.reverse
+      @threads = @ts.threads.select { |t| !@hidden_threads[t] && t.first }.sort_by { |t| [t.date, t.first.id] }.reverse
       @size_widgets = @threads.map { |t| size_widget_for_thread t }
       @size_widget_width = @size_widgets.max_of { |w| w.display_length }
       @date_widgets = @threads.map { |t| date_widget_for_thread t }
_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: "Edward Z. Yang" <[email-masked]>
Date: Wed, 10 Mar 2010 13:06:19 -0500

Excerpts from Rich Lane's message of Wed Mar 10 12:59:48 -0500 2010:
> If header['content-type'] == "" then it counts as true in ruby. Maybe
> you wanted to check empty? instead? This is an rmail bug, so I think
> instead of adding workarounds to our code we should monkeypatch rmail to
> fix it.

Heh, my ruby-fu isn't quite there yet; I just hacked around until I made all of
the errors go away. :-)  Monkeypatching the rmail code would not be a bad
proposition.

Cheers,
Edward

---

From: Rich Lane <[email-masked]>
Date: Wed, 10 Mar 2010 12:24:12 -0500

> +        content_type = (m.header["content_type"] && m.header.content_type || "application/unknown").downcase # sometimes RubyMail gives us nil

Could you explain why these changes help? What's the return value of the
[] call versus the method call?
_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: "Edward Z. Yang" <[email-masked]>
Date: Tue, 09 Mar 2010 23:52:45 -0500

Actually, I know precisely where this is from:

Content-Type: 
Content-Disposition: inline
Content-Transfer-Encoding: base64
RT-Attachment: 1176783/16113169/7868621

Which translates into an empty string and thus the following
trace:

--- NoMethodError from thread: poll after loading inbox
undefined method `downcase' for nil:NilClass
/usr/lib/ruby/gems/1.8/gems/rmail-1.0.0/lib/rmail/header.rb:537:in `content_type'
/home/ezyang/Dev/sup/lib/sup/message.rb:439:in `message_to_chunks'
/home/ezyang/Dev/sup/lib/sup/message.rb:439:in `map'
/home/ezyang/Dev/sup/lib/sup/message.rb:439:in `message_to_chunks'
/home/ezyang/Dev/sup/lib/sup/message.rb:244:in `load_from_source!'
/home/ezyang/Dev/sup/lib/sup/message.rb:340:in `build_from_source'
/home/ezyang/Dev/sup/lib/sup/poll.rb:163:in `each_message_from'
/home/ezyang/Dev/sup/lib/sup/maildir.rb:160:in `each'
[snip]

I suppose this is something that should be fixed on the rmail side...

Cheers,
Edward

---

From: Rich Lane <[email-masked]>
Date: Wed, 10 Mar 2010 12:59:48 -0500

---

From: Tero Tilus <[email-masked]>
Date: Wed, 10 Mar 2010 08:43:12 +0200

> I suppose this is something that should be fixed on the rmail

And this is definitely not the only one.  Unfortunately RubyMail
<http://rubyforge.org/projects/rubymail/> is pretty much dead.

It looks like the state of art ruby mail library is currently Mail
<http://github.com/mikel/mail>.  I don't know how well it would serve
sup, but rmail definitely has a long list of issues (already worked
around in sup) and switching mail library would be a lot of work.

Thoughts?

-- 
Tero Tilus ## 050 3635 235 ## http://tero.tilus.net/
_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: "Edward Z. Yang" <[email-masked]>
Date: Wed, 10 Mar 2010 12:40:19 -0500

> [] call versus the method call?

Sure!  We turn our heads to exhibit 1: The rmail code that implements m.header.content_type

    def content_type(default = nil)
      if value = self['content-type']
        value.strip.split(/\s*;\s*/)[0].downcase
      else
        if block_given?
          yield
        else
          default
        end
      end
    end

This code takes "text/plain; charset=utf-8" and ensures that only "text/plain"
is returned.  But what if self['content-type'] is empty?  Then we call split
on an empty string...

irb(main):001:0> "".strip.split(/\s*;\s*/)
=> []

Oops; the zero indexing fails.

Actually, the patch is buggy; I should be testing for content-type instead. :-)

Cheers,
Edward