From: Gaute Hope <eg@gaute.vetsj.com>
To: Ruthard Baudach <ruthard.baudach@web.de>
Cc: supmua <supmua@googlegroups.com>
Subject: Re: [sup] Bug in mbox.rb?
Date: Thu, 20 Nov 2014 17:48:53 +0000 [thread overview]
Message-ID: <1416505196-astroid-8-i6copfrsyz-3226@strange> (raw)
In-Reply-To: <1416415410-sup-6292@ruthard-lappi>
Excerpts from Ruthard Baudach's message of November 19, 2014 17:53:
>>== Auszüge aus der Nachricht von Gaute Hope vom 2014-11-19 10:03:
>> 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.
I am not sure this is a problem after all, as stated in qmails 'How a
message is read', message reading is stopped either at the next From
line or EOF.
The blank line does not play a huge role, but it is in effect added to
all messages except the last one (which is terminated by EOF anyway).
You could do a f.puts as you suggest below in stead, but it would not
make much of a difference as far as I can see.
I think the mbox is valid as it is.
Now; mbox.rb does escape From lines in the body, but I cannot see how
they are un-escaped when reading. Does RMail::Parser.read unescape
these? Also, does mbox.rb skip the extra blank line or do messages get
an extra blank line after being re-read? probably not critical, but
tests should be run on this..
cheers, gaute
>> >
>> > 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
>
> I think
>
> 1) It's a minor bug, it would only cause problems if one would
> intermingle in- and outgoing mail. Does it has to be fixed?
>
> 2) I could either just fix it, write a script to fix wrongly formatted
> mboxes, and the next release would need a WARNING RUN SUP_FIX_MBOXES
> prior to using it after update.
>
> 3) I could check the mbox everytime a message is added and add a empty
> line as needed. Would increase disk usage.
>
> I couldn't make any sense of the test dir in the source code, probably for
> lack of knowledge, not because of lack of sense;-)
>
> Could you point me to some documentation on testing the sup team is
> using?
>
next prev parent reply other threads:[~2014-11-20 17:48 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 ` [sup] " Gaute Hope
2014-11-19 16:53 ` Ruthard Baudach
2014-11-19 20:54 ` Ruthard Baudach
2014-11-20 17:48 ` Gaute Hope [this message]
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=1416505196-astroid-8-i6copfrsyz-3226@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