Re: [ID 20010608.010] File::Find re-entrancy

Message-Id: <200106150923.f5F9NpG02725@wcl-l.bham.ac.uk>

p4raw-id: //depot/perl@10607

2 files changed, 43 insertions, 9 deletions

diff --git a/lib/File/Find.pm b/lib/File/Find.pm
index 9ae39ace5d..274c7b8d17 100644
--- a/lib/File/Find.pm
+++ b/lib/File/Find.pm
@@ -281,8 +281,11 @@ my $Is_MacOS;
 require File::Basename;
 require File::Spec;
 
-my %SLnkSeen;
-my ($wanted_callback, $avoid_nlink, $bydepth, $no_chdir, $follow,
+# Should ideally be my() not our() but local() currently
+# refuses to operate on lexicals
+
+our %SLnkSeen;
+our ($wanted_callback, $avoid_nlink, $bydepth, $no_chdir, $follow,
     $follow_skip, $full_check, $untaint, $untaint_skip, $untaint_pat,
     $pre_process, $post_process);
 
@@ -447,6 +450,15 @@ sub _find_opt {
     my $wanted = shift;
     die "invalid top directory" unless defined $_[0];
 
+    # This function must local()ize everything because callbacks may
+    # call find() or finddepth()
+
+    local %SLnkSeen;
+    local ($wanted_callback, $avoid_nlink, $bydepth, $no_chdir, $follow,
+	$follow_skip, $full_check, $untaint, $untaint_skip, $untaint_pat,
+	$pre_process, $post_process);
+    local($dir, $name, $fullname, $prune);
+
     my $cwd           = $wanted->{bydepth} ? Cwd::fastcwd() : Cwd::cwd();
     my $cwd_untainted = $cwd;
     my $check_t_cwd   = 1;
@@ -463,7 +475,7 @@ sub _find_opt {
     $untaint_skip     = $wanted->{untaint_skip};
 
     # for compatability reasons (find.pl, find2perl)
-    our ($topdir, $topdev, $topino, $topmode, $topnlink);
+    local our ($topdir, $topdev, $topino, $topmode, $topnlink);
 
     # a symbolic link to a directory doesn't increase the link count
     $avoid_nlink      = $follow || $File::Find::dont_use_nlink;
@@ -1028,17 +1040,13 @@ sub wrap_wanted {
 
 sub find {
     my $wanted = shift;
-    %SLnkSeen= (); # clear hash first
     _find_opt(wrap_wanted($wanted), @_);
-    %SLnkSeen= ();  # free memory
 }
 
 sub finddepth {
     my $wanted = wrap_wanted(shift);
-    %SLnkSeen= (); # clear hash first
     $wanted->{bydepth} = 1;
     _find_opt($wanted, @_);
-    %SLnkSeen= ();  # free memory
 }
 
 # default
diff --git a/t/lib/filefind.t b/t/lib/filefind.t
index d07948b0ea..3e73ffcadc 100755
--- a/t/lib/filefind.t
+++ b/t/lib/filefind.t
@@ -9,10 +9,10 @@ my $cwd_untainted;
 
 BEGIN {
     chdir 't' if -d 't';
-   @INC = '../lib';
+    unshift @INC => '../lib';
 
     for (keys %ENV) { # untaint ENV
-	($ENV{$_}) = keys %{{ map {$_ => 1} $ENV{$_} }};
+	($ENV{$_}) = $ENV{$_} =~ /(.*)/;
     }
 
     $SIG{'__WARN__'} = sub { $warn_msg = $_[0]; warn "# Warn: $_[0]"; }
@@ -215,6 +215,19 @@ if ($^O eq 'MacOS') {
     File::Find::find( {wanted => \&wanted, untaint => 1},':fa' );
     Check( scalar(keys %Expect) == 0 );
 
+    print "# check re-entancy\n";
+    %Expect = (':' => 1, 'fsl' => 1, 'fa_ord' => 1, 'fab' => 1, 'fab_ord' => 1,
+           'faba' => 1, 'faa' => 1, 'faa_ord' => 1);
+    delete $Expect{'fsl'} unless $symlink_exists;
+    %Expect_Dir = (':' => 1, 'fa' => 1, 'faa' => 1, 'fab' => 1, 'faba' => 1, 
+                   'fb' => 1, 'fba' => 1);
+    delete @Expect_Dir{'fb','fba'} unless $symlink_exists;
+    File::Find::find( {wanted => sub { 
+      wanted();
+      File::Find::find( {wanted => sub {} , untaint => 1 },':' );
+    }, untaint => 1 }, ':fa' );
+    Check( scalar(keys %Expect) == 0 );
+
     %Expect=(':fa' => 1, ':fa:fsl' => 1, ':fa:fa_ord' => 1, ':fa:fab' => 1,
          ':fa:fab:fab_ord' => 1, ':fa:fab:faba' => 1,
          ':fa:fab:faba:faba_ord' => 1, ':fa:faa' => 1, ':fa:faa:faa_ord' => 1);
@@ -465,6 +478,19 @@ if ($^O eq 'MacOS') {
     File::Find::find( {wanted => \&wanted, untaint => 1, untaint_pattern => qr|^(.+)$|},'fa' );
     Check( scalar(keys %Expect) == 0 );
 
+    print "# check re-entancy\n";
+    %Expect = ('.' => 1, 'fsl' => 1, 'fa_ord' => 1, 'fab' => 1, 'fab_ord' => 1,
+           'faba' => 1, 'faa' => 1, 'faa_ord' => 1);
+    delete $Expect{'fsl'} unless $symlink_exists;
+    %Expect_Dir = ('fa' => 1, 'faa' => 1, 'fab' => 1, 'faba' => 1, 
+                   'fb' => 1, 'fba' => 1);
+    delete @Expect_Dir{'fb','fba'} unless $symlink_exists;
+    File::Find::find( {wanted => sub { 
+      wanted();
+      File::Find::find( {wanted => sub {} , untaint => 1, untaint_pattern => qr|^(.+)$|},'.' );
+    }, untaint => 1, untaint_pattern => qr|^(.+)$|},'fa' );
+    Check( scalar(keys %Expect) == 0 );
+
     %Expect=('fa' => 1, 'fa/fsl' => 1, 'fa/fa_ord' => 1, 'fa/fab' => 1,
          'fa/fab/fab_ord' => 1, 'fa/fab/faba' => 1,
          'fa/fab/faba/faba_ord' => 1, 'fa/faa' => 1, 'fa/faa/faa_ord' => 1);