* Bug in mbox.rb?
@ 2014-11-17 23:08 Ruthard Baudach
2014-11-19 9:03 ` [sup] " Gaute Hope
0 siblings, 1 reply; 7+ messages in thread
From: Ruthard Baudach @ 2014-11-17 23:08 UTC (permalink / raw)
To: supmua
[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]
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????
Yours,
Ruthard
--
Please encrypt and sign emails.
My PGP-Id: AC5AC6C2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [sup] Bug in mbox.rb?
2014-11-17 23:08 Bug in mbox.rb? Ruthard Baudach
@ 2014-11-19 9:03 ` Gaute Hope
2014-11-19 16:53 ` Ruthard Baudach
0 siblings, 1 reply; 7+ messages in thread
From: Gaute Hope @ 2014-11-19 9:03 UTC (permalink / raw)
To: supmua, Ruthard Baudach
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [sup] Bug in mbox.rb?
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
0 siblings, 2 replies; 7+ messages in thread
From: Ruthard Baudach @ 2014-11-19 16:53 UTC (permalink / raw)
To: Gaute Hope; +Cc: supmua
[-- Attachment #1: Type: text/plain, Size: 3119 bytes --]
>== 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.
> >
> > 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?
Yours, Ruthard
--
Dr. Ruthard Baudach
Speckertsweg 36a
97209 Veitshöchheim
----
Emails bitte verschlüsseln und signieren.
Meine PGP-Id: AC5AC6C2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [sup] Bug in mbox.rb?
2014-11-19 16:53 ` Ruthard Baudach
@ 2014-11-19 20:54 ` Ruthard Baudach
2014-11-20 17:48 ` Gaute Hope
1 sibling, 0 replies; 7+ messages in thread
From: Ruthard Baudach @ 2014-11-19 20:54 UTC (permalink / raw)
To: Gaute Hope, supmua
[-- Attachment #1: Type: text/plain, Size: 607 bytes --]
>== Auszüge aus der Nachricht von Ruthard Baudach vom 2014-11-19 17:53:
> 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?
I found minitest documentation and tutorials and had rr gem not
installed, and ruby 2 dropped the current working directory from the
search path, thus 'file.rb' has to be changed to './file.rb' – now I see
the sense in the test classes :-)
Ruthard
Please encrypt and sign emails.
My PGP-Id: AC5AC6C2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [sup] Bug in mbox.rb?
2014-11-19 16:53 ` Ruthard Baudach
2014-11-19 20:54 ` Ruthard Baudach
@ 2014-11-20 17:48 ` Gaute Hope
2014-11-21 19:29 ` Ruthard Baudach
1 sibling, 1 reply; 7+ messages in thread
From: Gaute Hope @ 2014-11-20 17:48 UTC (permalink / raw)
To: Ruthard Baudach; +Cc: supmua
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?
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [sup] Bug in mbox.rb?
2014-11-20 17:48 ` Gaute Hope
@ 2014-11-21 19:29 ` Ruthard Baudach
2014-11-23 9:58 ` Gaute Hope
0 siblings, 1 reply; 7+ messages in thread
From: Ruthard Baudach @ 2014-11-21 19:29 UTC (permalink / raw)
To: supmua
[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]
>== Auszüge aus der Nachricht von Gaute Hope vom 2014-11-20 18:48:
>
> 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're right, as long as message reading is stopped by /From email
ascdate\/ rather than /\nFrom email ascdate \n/.
>
> 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.
Well, as there is no real authoritative specification, it is at least a
valid sup mbox, isn't it?
>
> 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..
I used mbox for years, and never encountered any problems -- well,
that's not true. sup sometimes had trouble to show messages several
years old, they were still in the mbox and I can view them now after I
converted the mbox to maildir (using python's mbox and maildir modules,
I'm afraid). But that should be another issue, and I don't see any way
nor need nor resources to address it.
Let's close this issue.
Regards, Ruthard
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [sup] Bug in mbox.rb?
2014-11-21 19:29 ` Ruthard Baudach
@ 2014-11-23 9:58 ` Gaute Hope
0 siblings, 0 replies; 7+ messages in thread
From: Gaute Hope @ 2014-11-23 9:58 UTC (permalink / raw)
To: supmua, Ruthard Baudach
Excerpts from Ruthard Baudach's message of November 21, 2014 20:29:
> Let's close this issue.
Ok!
- gaute
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-11-23 9:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-17 23:08 Bug in mbox.rb? 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
2014-11-21 19:29 ` Ruthard Baudach
2014-11-23 9:58 ` Gaute Hope
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox