Discussions of development and use of the Sup email client
 help / color / mirror / Atom feed
From: Gaute Hope <eg@gaute.vetsj.com>
To: supmua <supmua@googlegroups.com>,
	Ruthard Baudach <ruthard.baudach@web.de>
Subject: Re: [sup] Bug in mbox.rb?
Date: Wed, 19 Nov 2014 09:03:06 +0000	[thread overview]
Message-ID: <1416387515-astroid-1-m2ak1advzy-2022@strange> (raw)
In-Reply-To: <1416264666-sup-9223@ruthard-lappi>

Excerpts from Ruthard Baudach's message of November 18, 2014 0:08:
> If I am not mistaken, both RFC4155 and
> http://www.qmail.org/man/man5/mbox.html
> specify that an mbox entry is *terminated* by a blank line.
> 
> line 117 -124 of lib/sup/mbox.rb reads:
> 
>   def store_message date, from_email, &block
>     need_blank = File.exists?(@path) && !File.zero?(@path)
>     File.open(@path, "ab") do |f|
>       f.puts if need_blank
>       f.puts "From #{from_email} #{date.asctime}"
>       yield f
>     end
>   end
> 
> As far as I understand this, sup puts a blank line *before* the
> separator line except for the first message.
> 
> My old sent.mbox is terminated by the last line of the last message, my
> old inbox.mbox is terminated by a blank line.
> 
> If I'm not mistaken, this could lead to problems, if the same mbox would
> be fed by sup and another MDA, e.g. if someone would use the same mbox
> for in- and outgoing mail (as I did when I had to setup sup anew after
> upgrading to 0.15. Changing to maildir simultaneously safed me from this
> bug. Huh!)
> 
> Changing the lines 117 - 124 of lib/sup/mbox.rb to
> 
>   def store_message date, from_email, &block
>     File.open(@path, "ab") do |f|
>       f.puts "From #{from_email} #{date.asctime}"
>       yield f
>       f.puts
>     end
>   end
> 
> would take care of this.
> 
> Do you agree?
> 
> Shall I submit a patch?
> 
> O – of course this would break existing setups with wrong formatted
> mboxes.
> 
> Hmmmmmm???? 

haven't looked at the details yet, but nice catch.

oh.. that would be baad. breaking existing setups is a
out-of-the-question though. Consequently; we need tests for whatever
change fixing this introduces (have a look at the test folder).

I think a fix should; 
- fix existing mboxes (on next write)
- don't break on any existing mboxes
- conform to standards
- include tests of all these points
- RMail has a mbox parser (i think) which perhaps could be used for
  testing, i think sup included its own for performance reasons.


gaute


  reply	other threads:[~2014-11-19  9:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 23:08 Ruthard Baudach
2014-11-19  9:03 ` Gaute Hope [this message]
2014-11-19 16:53   ` [sup] " Ruthard Baudach
2014-11-19 20:54     ` Ruthard Baudach
2014-11-20 17:48     ` Gaute Hope
2014-11-21 19:29       ` Ruthard Baudach
2014-11-23  9:58         ` Gaute Hope

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1416387515-astroid-1-m2ak1advzy-2022@strange \
    --to=eg@gaute.vetsj.com \
    --cc=ruthard.baudach@web.de \
    --cc=supmua@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox