From d7cdb6e15eb5488554e030a98f028d9e874f9b8b Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Wed, 27 May 2015 13:52:25 +0200
Subject: [PATCH] Bug 6874: [QA Follow-up] Adjustments for opac-retrieve

Adds httpheaders sub to UploadedFiles; this will be extended on a new
report. (Idea is to add configurable headers for file extensions.)
Trivial unit test added.
Small cosmetic changes to opac-retrieve-file.

Test plan:
Run test UploadedFiles.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
---
 C4/UploadedFiles.pm            | 14 ++++++++++++++
 opac/opac-retrieve-file.pl     | 22 ++++++----------------
 t/db_dependent/UploadedFiles.t |  6 +++++-
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/C4/UploadedFiles.pm b/C4/UploadedFiles.pm
index 341b5b6966..c03acbfe18 100644
--- a/C4/UploadedFiles.pm
+++ b/C4/UploadedFiles.pm
@@ -290,4 +290,18 @@ sub DelUploadedFile {
     return $retval;
 }
 
+=head2 httpheaders
+
+    httpheaders returns http headers for a retrievable upload
+    Will be extended by report 14282
+
+=cut
+
+sub httpheaders {
+    my $file= shift;
+    return
+        ( '-type' => 'application/octet-stream',
+          '-attachment' => $file, );
+}
+
 1;
diff --git a/opac/opac-retrieve-file.pl b/opac/opac-retrieve-file.pl
index 4c122d2ff9..419f2bdd5c 100755
--- a/opac/opac-retrieve-file.pl
+++ b/opac/opac-retrieve-file.pl
@@ -27,21 +27,11 @@ my $input = new CGI;
 
 my $id = $input->param('id');
 my $file = C4::UploadedFiles::GetUploadedFile($id);
-exit 1 if not $file;
+exit 1 if !$file || !-f $file->{filepath};
 
-my $file_path = $file->{filepath};
-
-if( -f $file_path ) {
-    open my $fh, '<', $file_path or die "Can't open file: $!";
-    print $input->header(
-        -type => "application/octet-stream",
-        -attachment => $file->{filename}
-    );
-    while(<$fh>) {
-        print $_;
-    }
-} else {
-    exit 1;
+open my $fh, '<', $file->{filepath} or die "Can't open file: $!";
+print $input->header( C4::UploadedFiles::httpheaders( $file->{filename} ));
+while(<$fh>) {
+    print $_;
 }
-
-exit 0;
+close $fh;
diff --git a/t/db_dependent/UploadedFiles.t b/t/db_dependent/UploadedFiles.t
index dfa1c47004..908f9f930b 100644
--- a/t/db_dependent/UploadedFiles.t
+++ b/t/db_dependent/UploadedFiles.t
@@ -3,7 +3,7 @@
 use Modern::Perl;
 use File::Temp qw/ tempdir /;
 use Test::CGI::Multipart;
-use Test::More tests => 17;
+use Test::More tests => 18;
 use Test::Warn;
 
 use t::lib::Mocks;
@@ -55,3 +55,7 @@ my $UploadResult;
 warning_like { $UploadResult=C4::UploadedFiles::UploadFile($testfilename,'../',$testfile_fh->handle); } qr/^Filename or dirname contains '..'. Aborting upload/, "Expected warning for bad file upload.";
 is($UploadResult, undef, "UploadFile with dir containing \"..\" return undef");
 is(C4::UploadedFiles::GetUploadedFile(), undef, 'GetUploadedFile without parameters returns undef');
+
+#trivial test for httpheaders
+my @hdrs = C4::UploadedFiles::httpheaders('does_not_matter_yet');
+is( @hdrs == 4 && $hdrs[1] =~ /application\/octet-stream/, 1, 'Simple test for httpheaders'); #TODO Will be extended on report 14282
-- 
2.39.5