From 9183275aa7e510f51dc63f6589cbfc4dbc60beeb Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Fri, 7 Aug 2015 19:42:44 +0200
Subject: [PATCH] Bug 14321: Add some error checking to Upload.pm
Content-Type: text/plain; charset=utf-8

This patch makes Upload return some error codes if a file cannot be read
or cannot be written.

Test plan:
[1] Upload a file with stage-marc-import.pl. This is a temporary file; it
    will be saved in your system's temporary dir (e.g. /tmp) within the
    folder koha_upload.
[2] Change the owner of this koha_upload dir and remove write permissions.
[3] Upload the file again. You should get: Upload status: Failed.
[4] Restore owner and permissions.
---
 Koha/Upload.pm       |   57 ++++++++++++++++++++++++++++++++++++++------------
 tools/upload-file.pl |    2 +-
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/Koha/Upload.pm b/Koha/Upload.pm
index c3a7e47..5b1ca8b 100644
--- a/Koha/Upload.pm
+++ b/Koha/Upload.pm
@@ -59,8 +59,7 @@ use base qw(Class::Accessor);
 
 use C4::Context;
 
-__PACKAGE__->mk_ro_accessors( qw| name
-|);
+__PACKAGE__->mk_ro_accessors( qw||);
 
 =head2 new
 
@@ -107,6 +106,23 @@ sub result {
     return join ',', @a;
 }
 
+=head2 err
+
+    Returns hash with errors in format { file => err, ... }
+    Undefined if there are no errors.
+
+=cut
+
+sub err {
+    my ( $self ) = @_;
+    my $err;
+    foreach my $f ( keys $self->{files} ) {
+        my $e = $self->{files}->{$f}->{errcode};
+        $err->{ $f } = $e if $e;
+    }
+    return $err;
+}
+
 =head2 get
 
     Returns array
@@ -117,13 +133,18 @@ sub result {
 sub get {
     my ( $self, $params ) = @_;
     my $temp= $self->_lookup( $params );
-    my ( @rv, $res );
+    my ( @rv, $res);
     foreach my $r ( @$temp ) {
+        undef $res;
         $res->{name} = $r->{filename};
         $res->{path}= $self->_full_fname($r);
-        $res->{fh} = IO::File->new( $res->{path}, "r" )
-            if $params->{filehandle};
-        push @rv, $res;
+        if( $res->{path} && -r $res->{path} ) {
+            $res->{fh} = IO::File->new( $res->{path}, "r" )
+                if $params->{filehandle};
+            push @rv, $res;
+        } else {
+            $self->{files}->{ $r->{filename} }->{errcode}=5; #not readable
+        }
         last if !wantarray;
     }
     return wantarray? @rv: $res;
@@ -146,8 +167,6 @@ sub _init {
     } else {
         $self->{category} = $params->{category};
     }
-
-    $self->{errors} = [];
     $self->{files} = {};
     $self->{uid} = C4::Context->userenv->{number} if C4::Context->userenv;
 }
@@ -165,18 +184,23 @@ sub _create_file {
     if( $self->{files}->{$filename} &&
             $self->{files}->{$filename}->{errcode} ) {
         #skip
+    } elsif( !$self->{temporary} && !$self->{rootdir} ) {
+        $self->{files}->{$filename}->{errcode} = 3; #no rootdir
     } elsif( $self->{temporary} && !$self->{tmpdir} ) {
-        $self->{files}->{$filename}->{errcode} = 2;
+        $self->{files}->{$filename}->{errcode} = 4; #no tempdir
     } else {
         my $dir = $self->_dir;
         my $fn = $self->{files}->{$filename}->{hash}. '_'. $filename;
+        if( -e "$dir/$fn" ) {
+            $self->{files}->{$filename}->{errcode} = 1; #already exists
+            return;
+        }
         $fh = IO::File->new( "$dir/$fn", "w");
         if( $fh ) {
             $fh->binmode;
             $self->{files}->{$filename}->{fh}= $fh;
         } else {
-            $self->{files}->{$filename}->{errcode} = 1;
-            # push @{$self->{errors}}, [ 1, $filename ];
+            $self->{files}->{$filename}->{errcode} = 2; #not writable
         }
     }
     return $fh;
@@ -192,10 +216,17 @@ sub _dir {
 
 sub _full_fname {
     my ( $self, $rec ) = @_;
+    my $p;
     if( ref $rec ) {
-        return ( $rec->{category}? $self->{rootdir}: $self->{tmpdir} ).
-            '/'. $rec->{dir}. '/'. $rec->{hashvalue}. '_'. $rec->{filename};
+        if( $rec->{category} && !$self->{rootdir} ) {
+        } elsif( !$rec->{category} && !$self->{tmpdir} ) {
+        } else {
+            $p= $rec->{category}? $self->{rootdir}: $self->{tmpdir};
+            $p.= '/';
+            $p.= $rec->{dir}. '/'. $rec->{hashvalue}. '_'. $rec->{filename};
+        }
     }
+    return $p;
 }
 
 sub _hook {
diff --git a/tools/upload-file.pl b/tools/upload-file.pl
index ec0313d..a1a2b65 100755
--- a/tools/upload-file.pl
+++ b/tools/upload-file.pl
@@ -58,7 +58,7 @@ if ($auth_failure) {
 }
 
 my $upload = Koha::Upload->new({ });
-if( !$upload || ! $upload->cgi ) {
+if( !$upload || !$upload->cgi || $upload->err ) {
     send_reply( 'failed' );
 } else {
     send_reply( 'done', $upload->result );
-- 
1.7.10.4