Discussions of development and use of the Sup email client
 help / color / mirror / Atom feed

* 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