Merge pull request #200 from jamadam/upload-file-improve3

Upload file improve to handle '..' in file paths

cpanfile

@@ -12,7 +12,7 @@ requires "File::Which" => "0";
 requires "HTTP::Headers" => "0";
 requires "HTTP::Request" => "0";
 requires "HTTP::Response" => "0";
-requires "IO::Compress::Zip" => "0";
+requires "IO::String" => "0";
 requires "IO::Socket" => "0";
 requires "IO::Socket::INET" => "0";
 requires "IO::Uncompress::Unzip" => "0";

lib/Selenium/Remote/Driver.pm

@@ -15,15 +15,15 @@ use v5.10.0;    # Before 5.006, v5.10.0 would not be understood.
 use Carp;
 our @CARP_NOT;
 
-use IO::Compress::Zip qw(zip $ZipError);
+use IO::String;
+use Archive::Zip qw( :ERROR_CODES );
 use Scalar::Util;
 use Selenium::Remote::RemoteConnection;
 use Selenium::Remote::Commands;
 use Selenium::Remote::WebElement;
 use File::Spec::Functions ();
-use File::Basename ();
+use File::Basename qw(basename);
 use Sub::Install ();
-use Cwd ();
 use MIME::Base64 ();
 
 use constant FINDERS => {
@@ -2449,7 +2449,7 @@ sub button_up {
 
     When passing raw data, be advised that it expects a zipped
     and then base64 encoded version of a single file.
-    Multiple files are not supported by the remote server.
+    Multiple files and/or directories are not supported by the remote server.
 
  Usage:
     my $remote_fname = $driver->upload_file( $fname );
@@ -2479,27 +2479,19 @@ sub upload_file {
     my $res = { 'command' => 'uploadFile' };    # /session/:SessionId/file
     my $ret = $self->_execute_command( $res, $params );
 
-    #WORKAROUND: Since this is undocumented selenium functionality,
-    #work around a bug.
-    my ($drive, $path, $file) = File::Spec::Functions::splitpath($ret);
-    if ($file ne $filename) {
-        $ret = File::Spec::Functions::catpath($drive,$path,$filename);
-    }
-
     return $ret;
 }
 
 sub _prepare_file {
     my ($self,$filename) = @_;
 
-    #Apparently zip chokes on non-canonical paths, creating double
-    #submissions sometimes
-    $filename = Cwd::abs_path($filename);
-
     if ( not -r $filename ) { die "upload_file: no such file: $filename"; }
     my $string = "";    # buffer
-    zip $filename => \$string
-      or die "zip failed: $ZipError\n";    # compress the file into string
+    my $zip = Archive::Zip->new();
+    $zip->addFile($filename, basename($filename));
+    if ($zip->writeToFileHandle(IO::String->new($string)) != AZ_OK) {
+        die 'zip failed';
+    }
 
     return {
         file => MIME::Base64::encode_base64($string)          # base64-encoded string

t/01-driver.t

@@ -504,8 +504,8 @@ UPLOAD: {
     my $testFile = "UEsDBBQACAAIAFtuNEYAAAAAAAAAAAAAAAAKABUAdXBsb2FkVGVzdFVUCQADjbG+VJ6xvlRVeAQA\n6APoAytJLS4BAFBLBwgMfn/YBgAAAAQAAABQSwECFAMUAAgACABbbjRGDH5/2AYAAAAEAAAACgAN\nAAAAAAAAAAAApIEAAAAAdXBsb2FkVGVzdFVUBQABjbG+VFV4AABQSwUGAAAAAAEAAQBFAAAAUwAA\nAAAA\n";
     my $otherTestFile = "UEsDBBQACAAIAFtuNEYAAAAAAAAAAAAAAAAMABUAdC91cGxvYWRUZXN0VVQJAAOesb5UnrG+VFV4\nBADoA+gDK0ktLgEAUEsHCAx+f9gGAAAABAAAAFBLAQIUAxQACAAIAFtuNEYMfn/YBgAAAAQAAAAM\nAA0AAAAAAAAAAACkgQAAAAB0L3VwbG9hZFRlc3RVVAUAAZ6xvlRVeAAAUEsFBgAAAAABAAEARwAA\nAFUAAAAAAA==\n";
 
-    like( $driver->upload_file('uploadTest',$testFile),qr/uploadTest$/,'upload_file returns FULL path to the file: cwd');
-    like( $driver->upload_file('t/uploadTest',$otherTestFile),qr/uploadTest$/,'upload_file returns FULL path to the file: subdir');
+    like( my $path1 = $driver->upload_file('uploadTest',$testFile),qr/uploadTest$/,'upload_file returns FULL path to the file: cwd');
+    ok(-f $path1) if $harness->record;
 
     my $fake_driver;
     if ($harness->record) {
@@ -523,8 +523,11 @@ UPLOAD: {
     }
 
     #In the case this is not mocked, it tests a real issue (.. in paths), if not, it makes sure the zip/base64 bits at least don't make us explode.
-    like( $fake_driver->upload_file('t/uploadTest'),qr/uploadTest$/,'upload_file: zip/base64 branch' );
-    like( $fake_driver->upload_file('t/../t/uploadTest'),qr/uploadTest$/,'upload_file: zip/base64 branch with .. in path' );
+    like( my $path2 = $fake_driver->upload_file('t/uploadTest'),qr/uploadTest$/,'upload_file: zip/base64 branch' );
+    like( my $path3 = $fake_driver->upload_file('t/../t/uploadTest'),qr/uploadTest$/,'upload_file: zip/base64 branch with .. in path' );
+    unlike( $path3,qr/\.\./,'upload_file: zip/base64 branch with .. in path' );
+    ok(-f $path2) if $harness->record;
+    ok(-f $path3) if $harness->record;
 
     #Negative tests to verify that our expected behavior codepath is travelled by tests
     like( exception { $driver->upload_file('@@@SomeFileThatDoesNotExist@@@')},qr/no such file/i,"Passing missing file terminates program");