Item13541: eachAttachment returns directory names with PlainFileStore, breaks bulk_copy.pl

Discovered that bulk_copy.pl was failing with empty attachments, which turned out to be directory names.

The following patch filters out the directory names from within the topic attachment directory. Note that this does not implement the RCS hack incDir API extension for the caller to request directory names. bulk_copy.pl won't be able to use that hack because it's using Meta to get the attachments. I'll open a separate task to revert that hack.

diff --git a/PlainFileStoreContrib/lib/Foswiki/Store/PlainFile.pm b/PlainFileStoreContrib/lib/Foswiki/Store/PlainFile.pm
index 03fcddb..4b1cbb3 100644
--- a/PlainFileStoreContrib/lib/Foswiki/Store/PlainFile.pm
+++ b/PlainFileStoreContrib/lib/Foswiki/Store/PlainFile.pm
@@ -728,9 +728,10 @@ sub eachAttachment {
    my ( $this, $meta ) = @_;
 
    my $dh;
-    opendir( $dh, _encode( _getPub($meta) ) )
+    my $dn = _encode( _getPub($meta));
+    opendir( $dh, $dn )
      or return new Foswiki::ListIterator( [] );
-    my @list = grep { !/^[.*_]/ && !/,pfv$/ } _readdir($dh);
+    my @list = grep { !/^[.*_]/ && !/,pfv$/ && ( -f $dn.'/'._encode($_)) } _readdir($dh);
    closedir($dh);
 
    require Foswiki::ListIterator;

This bug exposes a separate issue in bulk_copy.pl. When the attachment is empty, it causes a failure due to empty variable. Here is a fix, though it probably ought to report the issue rather than just avoiding it.

diff --git a/core/tools/bulk_copy.pl b/core/tools/bulk_copy.pl
index d7e3b6e..cfe7785 100755
--- a/core/tools/bulk_copy.pl
+++ b/core/tools/bulk_copy.pl
@@ -376,7 +376,7 @@ sub copy_attachment_version {
    binmode $tfh;
    local $/ = undef;
    my $data = <$fh>;
-    print $tfh $data;
+    print $tfh $data if length($data);
    close($tfh);
    my $tfn = $tfh->filename();

-- GeorgeClark - 20 Jul 2015

Both patches look fine, please go ahead and commit them. On the 'incDir hack', subdirectories in topics in pub have never been part of the attachment spec and must be ignored by Foswiki. We cannot allow them to be added to the spec by stealth. If you are determined that they should be added, then you're going to need a full feature spec. At the moment the use of these subdirs is leveraging a massive (and IMHO dangerous) hole in the file-based store implementation.

-- Main.CrawfordCurrie - 21 Jul 2015 - 06:49

Commenting is disabled while not logged in