You are here: Foswiki>Tasks Web>Item13541 (03 Aug 2015, GeorgeClark)Edit Attach

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

pencil
Priority: Urgent
Current State: Closed
Released In: 2.0.1
Target Release: patch
Applies To: Extension
Component: PlainFileStoreContrib
Branches: master Item13525
Reported By: GeorgeClark
Waiting For:
Last Change By: GeorgeClark
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
 
Topic revision: r5 - 03 Aug 2015, GeorgeClark - This page was cached on 25 May 2016 - 09:19.

The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License