Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes for File::chdir #4

Merged
merged 5 commits into from

2 participants

@jberger

These of course need to be tested on many platforms, but I believe from looking through the alias mapping in Cwd that Cwd::getcwd is going to be safer than Cwd::abs_path.

jberger added some commits
@jberger jberger rely on Cwd::getcwd which should be safer on cygwin (and other platfo…
…rms?) than abs_path.

Some platforms implement abs_path via fast_abs_path which is a more dangerous function. getcwd doesn't seem to suffer that failing.
I filed a bug on Cwd (https://rt.perl.org/rt3/Public/Bug/Display.html?id=115962) but this fix for File::chdir should work immediately.
Addresses GitHub #3
ecaf12f
@jberger jberger Fix another tainted variable so that tests now pass under `prove -lT` fac8f8a
@jberger jberger Need to fix newline.t or else the test will still fail on platforms w…
…hen *Cwd::abs_path = \&Cwd::fast_abs_path
f208289
@jberger jberger remove use of Cwd::abs_path without argument preferring Cwd::getcwd i…
…nstead
35592d5
@jberger jberger remove use of Cwd::abs_path from documentation (preferring Cwd::getcw…
…d). README.pod will be rebuilt before release.
b2b4f13
@jberger

It is now my assessment that Cwd::abs_path should never be used without argument to get the working directory. Cwd::getcwd exists for this purpose and doesn't (appear) to suffer from die-ing (with a misleading message) when in a directory with a newline in it.

@jberger jberger referenced this pull request
Closed

test broken on cygwin #3

@dagolden dagolden merged commit 7338fa6 into dagolden:master
@dagolden
Owner

Thank you very much. Released to CPAN as 0.1008

@jberger

Glad to help. It was a rather fun find actually! I took some pleasure in documenting it too :-) http://blogs.perl.org/users/joel_berger/2012/12/a-question-of-location.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 2, 2012
  1. @jberger

    rely on Cwd::getcwd which should be safer on cygwin (and other platfo…

    jberger authored
    …rms?) than abs_path.
    
    Some platforms implement abs_path via fast_abs_path which is a more dangerous function. getcwd doesn't seem to suffer that failing.
    I filed a bug on Cwd (https://rt.perl.org/rt3/Public/Bug/Display.html?id=115962) but this fix for File::chdir should work immediately.
    Addresses GitHub #3
  2. @jberger
  3. @jberger

    Need to fix newline.t or else the test will still fail on platforms w…

    jberger authored
    …hen *Cwd::abs_path = \&Cwd::fast_abs_path
  4. @jberger
  5. @jberger

    remove use of Cwd::abs_path from documentation (preferring Cwd::getcw…

    jberger authored
    …d). README.pod will be rebuilt before release.
This page is out of date. Refresh to see the latest.
View
7 lib/File/chdir.pm
@@ -18,7 +18,7 @@ tie @CWD, 'File::chdir::ARRAY' or die "Can't tie \@CWD";
sub _abs_path {
# Otherwise we'll never work under taint mode.
- my($cwd) = Cwd::abs_path =~ /(.*)/s;
+ my($cwd) = Cwd::getcwd =~ /(.*)/s;
# Run through File::Spec, since everything else uses it
return canonpath($cwd);
}
@@ -39,7 +39,8 @@ sub _catpath {
}
sub _chdir {
- my($new_dir) = @_;
+ # Untaint target directory
+ my ($new_dir) = $_[0] =~ /(.*)/s;
local $Carp::CarpLevel = $Carp::CarpLevel + 1;
if ( ! CORE::chdir($new_dir) ) {
@@ -295,7 +296,7 @@ which is much simpler than the equivalent:
sub foo {
use Cwd;
- my $orig_dir = Cwd::abs_path;
+ my $orig_dir = Cwd::getcwd;
chdir('some/other/dir');
...do your work...
View
6 t/array.t
@@ -3,7 +3,7 @@
use strict;
use Test::More tests => 55;
use File::Spec::Functions qw/canonpath splitdir catdir splitpath catpath/;
-use Cwd qw/getcwd abs_path/;
+use Cwd qw/getcwd/;
BEGIN { use_ok('File::chdir') }
@@ -12,7 +12,7 @@ BEGIN { use_ok('File::chdir') }
#--------------------------------------------------------------------------#-
# _catdir has OS-specific path separators so do the same for getcwd
-sub _getcwd { canonpath( abs_path ) }
+sub _getcwd { canonpath( getcwd ) }
# reassemble
sub _catpath {
@@ -21,7 +21,7 @@ sub _catpath {
}
# get $vol here and use it later
-my ($vol,$cwd) = splitpath(canonpath(abs_path),1);
+my ($vol,$cwd) = splitpath(canonpath(getcwd),1);
# get directory list the way a user would use it -- without empty leading dir
# as returned by splitdir;
View
4 t/chdir.t
@@ -3,12 +3,12 @@
use strict;
use Test::More tests => 6;
use File::Spec::Functions qw/canonpath catdir/;
-use Cwd qw/getcwd abs_path/;
+use Cwd qw/getcwd/;
BEGIN { use_ok('File::chdir') }
# _catdir has OS-specific path separators so do the same for getcwd
-sub _getcwd { canonpath( abs_path ) }
+sub _getcwd { canonpath( getcwd ) }
my($cwd) = _getcwd =~ /(.*)/; # detaint otherwise nothing's gonna work
View
6 t/delete-array.t
@@ -14,7 +14,7 @@ BEGIN {
}
use File::Spec::Functions qw/canonpath splitdir catdir splitpath catpath/;
-use Cwd qw/getcwd abs_path/;
+use Cwd qw/getcwd/;
BEGIN { use_ok('File::chdir') }
@@ -23,7 +23,7 @@ BEGIN { use_ok('File::chdir') }
#--------------------------------------------------------------------------#-
# _catdir has OS-specific path separators so do the same for getcwd
-sub _getcwd { canonpath( abs_path ) }
+sub _getcwd { canonpath( getcwd ) }
# reassemble
sub _catpath {
@@ -32,7 +32,7 @@ sub _catpath {
}
# get $vol here and use it later
-my ($vol,$cwd) = splitpath(canonpath(abs_path),1);
+my ($vol,$cwd) = splitpath(canonpath(getcwd),1);
# get directory list the way a user would use it -- without empty leading dir
# as returned by splitdir;
View
6 t/newline.t
@@ -6,7 +6,7 @@ use warnings;
use Test::More;
use File::chdir;
-use Cwd qw(abs_path);
+use Cwd qw(getcwd);
my $Orig_Cwd = $CWD;
@@ -17,11 +17,11 @@ plan skip_all => "Can't make a directory with a newline in it" unless $Can_mkdir
{
local $CWD = $Test_Dir;
- is $CWD, abs_path;
+ is $CWD, getcwd;
}
is $CWD, $Orig_Cwd;
-is abs_path, $Orig_Cwd;
+is getcwd, $Orig_Cwd;
END {
chdir $Orig_Cwd; # just in case
View
4 t/var.t
@@ -3,12 +3,12 @@
use strict;
use Test::More tests => 13;
use File::Spec::Functions qw/canonpath catdir/;
-use Cwd qw/getcwd abs_path/;
+use Cwd qw/getcwd/;
BEGIN { use_ok('File::chdir') }
# _catdir has OS-specific path separators so do the same for getcwd
-sub _getcwd { canonpath( abs_path ) }
+sub _getcwd { canonpath( getcwd ) }
my $cwd = _getcwd;
Something went wrong with that request. Please try again.