From 9183275aa7e510f51dc63f6589cbfc4dbc60beeb Mon Sep 17 00:00:00 2001 From: Marcel de Rooy 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