Re: Transaction.pm body_fh() badly broken [was: qpsmtpd-v0.84 seems to ignore size_threshold]

This was changed in

commit a52660a646012691f993cca821c00fe05cff08bb
Author: jaredj <[email protected]>
Date:   Wed Mar 25 07:38:05 2009 -0500

    Spool body when $transaction->body_fh() is called
    
    Qpsmtpd::Transaction::body_filename() calls $self->body_spool() if
    the message body has not already been spool to disk.  This adds
    the same check to Qpsmtpd::Transaction::body_fh()

but I don't remember the background behind it.  Jared, do you remember
why you changed this?

David Favor wrote:
> 
> Since no one responded to my previous email I've
> combed through the code and found the problem.
> 
> root@hive1:# git-describe
> v0.84-36-g0b0e4e9
> 
> perldoc lib/Qpsmtpd/Transaction.pm says...
> 
>     body_fh( )
>         Returns the file handle to the temporary file
>         of the email. This will return undef if the file
>         is not opened (yet). In hook_data( ) or later you can
>         force spooling to disk by calling $transaction->body_filename.
> 
> This use to be true and at some point the body_fh() code changed to:
> 
>     sub body_fh {
>         my ($self) = @_;
>         # Spool to disk if we weren't already doing so
>         $self->body_spool() unless $self->{_filename};
>         return $self->{_body_file};
>     }
> 
> Per the documentation this subroutine should be a single line:
> 
>         return $self->{_body_fh};
> 
> Lord knows when this code changed or what other code written
> depends on this change.

See above.

> 
> Suggested fixes:
> 
>     1) Fix the documentation to correctly reflect calling body_fh()
>        is the same as calling body_spool()

This seems straightforward.

> 
>     2) Add a subroutine spooled() which returns 1 if the data is
>        spooled to a file or 0 if in memory

Patch welcome.

-R