If you add the beforeAttachmentSaveHandler in any Plugin and attach a file, you get a file attached with zero bytes.

How to reproduce:

• In any existing enabled Plugin, such as TablePlugin, add this:
  sub beforeAttachmentSaveHandler{ return; }
• Attach a text file to any topic
• View the attached file: You get an empty file

This is on a Solaris machine.

I need this fixed urgently for the BlackListPlugin in order to fight the nasty TWiki:Codev.HtmlAttachmentSpam issue. The enhanced Plugin works properly on Cairo, but on TWiki 4 we have above mentioned issue.

This bugs is on 4.0.2 release and TWiki 4 branch.

-- PTh

There is something wierd on that platform, then, because it works perfectly on TWiki 4 on my platforms (Debian etch and IndigoPerl on Windows XP). Without access to the platform I can only suggest you instrument Store.pm (around where the handler is called)

CC

Some more info.

TWiki 4.0.2 returns the temp filename in {file} instead of {tmpFilename}; this has been fixed in the latest twiki 4 branch. That means, no plugin can work in 4.0.2 using {tmpFilename} in beforeAttachmentSaveHandler.

Store.pm in the latest TWiki 4 uses File::Temp. According to the doc, http://www.ayni.com/perldoc/perl5.8.0/lib/File/Temp.html, "If called in scalar context, only the filehandle is returned and the file will automatically be deleted when closed". That is probably platform dependent, on Solaris this seems to be the case, hence a zero file size file when you read it back after the beforeAttachmentSaveHandler call. On other platforms the temp file seems to live longer.

Also, Store.pm in the latest TWiki 4 uses File::Temp to create a temp file before calling beforeAttachmentSaveHandler. This is not a standard library according to Perl 5.005 doc, http://www.fnal.gov/docs/products/perl/pod.new/5.00503/pod/perlmodlib.html . Any way to avoid File::Temp? TWiki does not use it anywhere else. It is possible to build a temp file name, such as web.topic.attachment.user.[randomnumber].

-- PTh

There's no need to avoid File::Temp; in fact, it is to be strongly preferred over any made-up mechanism TWiki would provide. All that is required is to call it in a list context instead of a scalar context.

Note that the optimal solution is to pass the stream to the beforeAttachmentSaveHandler instead of requiring mucking about with a temporary file.

CC

Thanks for fixing, it works now nicely.

I understand that there is no need to avoid File::Temp to fix the bug. However, I am concerned that we introduce yet another CPAN dependency (installation hassle and possible performance impact). We need to (a) find another solution, or (b) document the new CPAN dependency (since this is not a standard module in the Perl version we support).

-- PTh

For those using the TWiki:Plugins.BlackListPlugin with TWiki 4.0.2 on Solaris: Change the the beforeAttachmentSaveHandler "if" section of sub saveAttachment in file twiki/lib/TWiki/Store.pm as follows:

            if( $plugins->haveHandlerFor( 'beforeAttachmentSaveHandler' )) {
                # SMELL: legacy spec of beforeAttachmentSaveHandler requires
                # a local copy of the stream. This could be a problem for
                # very big data files.
                use File::Temp;
                my $fh;
                # Note: do *not* rely on UNLINK => 1, because in a mod_perl
                # context the destructor may not be called for a *long* time.
                # Call tempfile in a list context so that file does not get
                # deleted when closed.
                ( $fh, $tmpFile ) = File::Temp::tempfile();
                binmode( $fh );
                # transfer 512KB blocks
                my $transfer;
                my $r;
                while( $r = sysread( $opts->{stream}, $transfer, 0x80000 )) {
                    syswrite( $fh, $transfer, $r );
                }
                close( $fh );
                $attrs->{tmpFilename} = $tmpFile;
                $plugins->beforeAttachmentSaveHandler( $attrs, $topic, $web );
                open( $opts->{stream}, "<$tmpFile" );
                binmode( $opts->{stream} );
            }

-- PTh

File::Temp is not a CPAN dependency; it's shipped with perl since 5.6.1.

CC

Sorry to be a pest, but TWiki 4.0 is advertized to work with 5.005_03 or higher, and File::Temp was not a standard lib at that time, hence the CPAN dependency.

-- PTh

As previously stated I will not, can not, claim support for any version of Perl i don't know to have been tested, which opens a whole other can of worms again. See http://twiki.org/cgi-bin/view/Codev/WhatVersionsOfPerlAreSupported. Follow up there.

CC

Re-opening this since action has not been taken on the CPAN dependency (either document the dependecy or remove the module)

-- PTh

updated TWiki/TWikiSystemRequirements

-- WN