be a bit safer about files

don't use two-arg open, don't use File::Temp::tempfile

2 files changed, 49 insertions, 76 deletions

diff --git a/lib/Spreadsheet/ParseXLSX.pm b/lib/Spreadsheet/ParseXLSX.pm
index 3d1ebd6..6e84e3e 100644
--- a/lib/Spreadsheet/ParseXLSX.pm
+++ b/lib/Spreadsheet/ParseXLSX.pm
@@ -63,41 +63,36 @@ sub parse {
     my $self = shift;
     my ($file, $formatter) = @_;
 
+    my $zip = Archive::Zip->new;
     my $workbook = Spreadsheet::ParseExcel::Workbook->new;
 
-    my $tempfile;
     if ($self->_check_signature($file)) {
-        $tempfile = $file = Spreadsheet::ParseXLSX::Decryptor->open(
+        $file = Spreadsheet::ParseXLSX::Decryptor->open(
             $file,
             $self->{Password}
         );
     }
 
-    eval {
-        my $zip = Archive::Zip->new;
-        if (openhandle($file)) {
-            bless $file, 'IO::File' if ref($file) eq 'GLOB'; # sigh
-            $zip->readFromFileHandle($file) == Archive::Zip::AZ_OK
-                or die "Can't open filehandle as a zip file";
-            $workbook->{File} = undef;
-        }
-        elsif (!ref($file)) {
-            $zip->read($file) == Archive::Zip::AZ_OK
-                or die "Can't open file '$file' as a zip file";
-            $workbook->{File} = $file;
-        }
-        else {
-            die "Argument to 'new' must be a filename or open filehandle";
-        }
-
-        $self->_parse_workbook($zip, $workbook, $formatter);
-    };
-    if ($tempfile) {
-        unlink $tempfile;
-    };
-    die $@ if $@;
+    if (openhandle($file)) {
+        bless $file, 'IO::File' if ref($file) eq 'GLOB'; # sigh
+        my $fh = ref($file) eq 'File::Temp'
+            ? IO::File->new("<&=" . fileno($file))
+            : $file;
+        $zip->readFromFileHandle($fh) == Archive::Zip::AZ_OK
+            or die "Can't open filehandle as a zip file";
+        $workbook->{File} = undef;
+        $workbook->{__tempfile} = $file;
+    }
+    elsif (!ref($file)) {
+        $zip->read($file) == Archive::Zip::AZ_OK
+            or die "Can't open file '$file' as a zip file";
+        $workbook->{File} = $file;
+    }
+    else {
+        die "Argument to 'new' must be a filename or open filehandle";
+    }
 
-    return $workbook;
+    return $self->_parse_workbook($zip, $workbook, $formatter);
 }
 
 sub _check_signature {
diff --git a/lib/Spreadsheet/ParseXLSX/Decryptor.pm b/lib/Spreadsheet/ParseXLSX/Decryptor.pm
index e98425f..6fa777f 100644
--- a/lib/Spreadsheet/ParseXLSX/Decryptor.pm
+++ b/lib/Spreadsheet/ParseXLSX/Decryptor.pm
@@ -6,7 +6,7 @@ use Crypt::Mode::CBC;
 use Crypt::Mode::ECB;
 use Digest::SHA ();
 use Encode ();
-use File::Temp 'tempfile';
+use File::Temp ();
 use MIME::Base64 ();
 use OLE::Storage_Lite;
 
@@ -20,28 +20,18 @@ sub open {
 
     $password = $password || 'VelvetSweatshop';
 
-    my ($infoFile, $packageFile) = _getCompoundData($filename, ['EncryptionInfo', 'EncryptedPackage']);
+    my ($infoFH, $packageFH) = _getCompoundData($filename, ['EncryptionInfo', 'EncryptedPackage']);
 
-    my $xlsx;
-
-    eval {
-        my $infoFH = IO::File->new();
-        $infoFH->open($infoFile);
-        $infoFH->binmode();
-
-        my $buffer;
-        $infoFH->read($buffer, 8);
-        my ($majorVers, $minorVers) = unpack('SS', $buffer);
+    my $buffer;
+    $infoFH->read($buffer, 8);
+    my ($majorVers, $minorVers) = unpack('SS', $buffer);
 
-        if ($majorVers == 4 && $minorVers == 4) {
-            $xlsx = agileDecryption($infoFH, $packageFile, $password);
-        } else {
-            $xlsx = standardDecryption($infoFH, $packageFile, $password);
-        }
-        $infoFH->close();
-    };
-    unlink $infoFile, $packageFile;
-    die $@ if $@;
+    my $xlsx;
+    if ($majorVers == 4 && $minorVers == 4) {
+        $xlsx = agileDecryption($infoFH, $packageFH, $password);
+    } else {
+        $xlsx = standardDecryption($infoFH, $packageFH, $password);
+    }
 
     return $xlsx;
 }
@@ -59,11 +49,11 @@ sub _getCompoundData {
         if ($#data < 0) {
             push @files, undef;
         } else {
-            my ($fh, $filename) = File::Temp::tempfile();
-            my $out = IO::Handle->new_from_fd($fh, 'w') || die "TempFile error!";
-            $out->write($data[0]->{Data});
-            $out->close();
-            push @files, $filename;
+            my $fh = File::Temp->new;
+            binmode($fh);
+            $fh->write($data[0]->{Data});
+            $fh->seek(0, 0);
+            push @files, $fh;
         }
     }
 
@@ -71,7 +61,7 @@ sub _getCompoundData {
 }
 
 sub standardDecryption {
-    my ($infoFH, $packageFile, $password) = @_;
+    my ($infoFH, $packageFH, $password) = @_;
 
     my $buffer;
     my $n = $infoFH->read($buffer, 24);
@@ -121,28 +111,22 @@ sub standardDecryption {
 
     $decryptor->verifyPassword($encryptedVerifier, $encryptedVerifierHash);
 
-    my $in = new IO::File;
-    $in->open("<$packageFile") || die 'File/handle opening error';
-    $in->binmode();
-
-    my ($fh, $filename) = File::Temp::tempfile();
+    my $fh = File::Temp->new;
     binmode($fh);
-    my $out = IO::Handle->new_from_fd($fh, 'w') || die "TempFile error!";
 
     my $inbuf;
-    $in->read($inbuf, 8);
+    $packageFH->read($inbuf, 8);
     my $fileSize = unpack('L', $inbuf);
 
-    $decryptor->decryptFile($in, $out, 1024, $fileSize);
+    $decryptor->decryptFile($packageFH, $fh, 1024, $fileSize);
 
-    $in->close();
-    $out->close();
+    $fh->seek(0, 0);
 
-    return $filename;
+    return $fh;
 }
 
 sub agileDecryption {
-    my ($infoFH, $packageFile, $password) = @_;
+    my ($infoFH, $packageFH, $password) = @_;
 
     my $xml = XML::Twig->new;
     $xml->parse($infoFH);
@@ -180,24 +164,18 @@ sub agileDecryption {
                   blockSize       => 0 + $info->att('blockSize')
               });
 
-    my $in = new IO::File;
-    $in->open("<$packageFile") || die 'File/handle opening error';
-    $in->binmode();
-
-    my ($fh, $filename) = File::Temp::tempfile();
+    my $fh = File::Temp->new;
     binmode($fh);
-    my $out = IO::Handle->new_from_fd($fh, 'w') || die "TempFile error!";
 
     my $inbuf;
-    $in->read($inbuf, 8);
+    $packageFH->read($inbuf, 8);
     my $fileSize = unpack('L', $inbuf);
 
-    $fileDecryptor->decryptFile($in, $out, 4096, $key, $fileSize);
+    $fileDecryptor->decryptFile($packageFH, $fh, 4096, $key, $fileSize);
 
-    $in->close();
-    $out->close();
+    $fh->seek(0, 0);
 
-    return $filename;
+    return $fh;
 }
 
 sub new {