Skip to content

Commit 414f14d

Browse files
nanisxenu
authored andcommitted
File::Find: fix "follow => 1" on Windows
File::Find's code expects unix-style paths and it manipulates them using basic string operations. That code is very fragile, and ideally we should make it use File::Spec, but that would involve rewriting almost the whole module. Instead, we made it convert backslashes to slashes and handle drive letters. Note from xenu: this commit was adapted from the PR linked in this blogpost[1]. I have squashed it, written the commit message and slightly modified the code. [1] - https://www.nu42.com/2021/09/canonical-paths-file-find-way-forward.html Fixes #19995
1 parent 2fcaad3 commit 414f14d

File tree

7 files changed

+97
-27
lines changed

7 files changed

+97
-27
lines changed

MANIFEST

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4265,6 +4265,7 @@ ext/File-DosGlob/DosGlob.xs Win32 DOS-globbing module
42654265
ext/File-DosGlob/lib/File/DosGlob.pm Win32 DOS-globbing module
42664266
ext/File-DosGlob/t/DosGlob.t See if File::DosGlob works
42674267
ext/File-Find/lib/File/Find.pm Routines to do a find
4268+
ext/File-Find/t/correct-absolute-path-with-follow.t
42684269
ext/File-Find/t/find.t See if File::Find works
42694270
ext/File-Find/t/lib/Testing.pm Functions used in testing File-find
42704271
ext/File-Find/t/taint.t See if File::Find works with taint

Porting/Maintainers.pl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ package Maintainers;
172172
'DISTRIBUTION' => 'SMUELLER/AutoLoader-5.74.tar.gz',
173173
'FILES' => q[cpan/AutoLoader],
174174
'EXCLUDED' => ['t/00pod.t'],
175+
'CUSTOMIZED' => ['t/02AutoSplit.t'],
175176
},
176177

177178
'autouse' => {

cpan/AutoLoader/t/02AutoSplit.t

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,12 @@ foreach (@tests) {
149149

150150
if ($args{Files}) {
151151
$args{Files} =~ s!/!:!gs if $^O eq 'MacOS';
152+
$args{Files} =~ s!\\!/!g if $^O eq 'MSWin32';
152153
my (%missing, %got);
153-
find (sub {$got{$File::Find::name}++ unless -d $_}, $dir);
154+
find(
155+
sub { (my $f = $File::Find::name) =~ s!\\!/!g; $got{$f}++ unless -d $_ },
156+
$dir
157+
);
154158
foreach (split /\n/, $args{Files}) {
155159
next if /^#/;
156160
$_ = lc($_) if $Is_VMS_lc;

ext/File-Find/lib/File/Find.pm

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,21 @@ sub contract_name {
4343
return $abs_name;
4444
}
4545

46+
sub _is_absolute {
47+
return $_[0] =~ m|^(?:[A-Za-z]:)?/| if $Is_Win32;
48+
return substr($_[0], 0, 1) eq '/';
49+
}
50+
51+
sub _is_root {
52+
return $_[0] =~ m|^(?:[A-Za-z]:)?/\z| if $Is_Win32;
53+
return $_[0] eq '/';
54+
}
55+
4656
sub PathCombine($$) {
4757
my ($Base,$Name) = @_;
4858
my $AbsName;
4959

50-
if (substr($Name,0,1) eq '/') {
60+
if (_is_absolute($Name)) {
5161
$AbsName= $Name;
5262
}
5363
else {
@@ -123,6 +133,7 @@ sub is_tainted_pp {
123133
return length($@) != 0;
124134
}
125135

136+
126137
sub _find_opt {
127138
my $wanted = shift;
128139
return unless @_;
@@ -183,19 +194,17 @@ sub _find_opt {
183194

184195
($topdev,$topino,$topmode,$topnlink) = $follow ? stat $top_item : lstat $top_item;
185196

186-
if ($Is_Win32) {
187-
$top_item =~ s|[/\\]\z||
188-
unless $top_item =~ m{^(?:\w:)?[/\\]$};
189-
}
190-
else {
191-
$top_item =~ s|/\z|| unless $top_item eq '/';
192-
}
197+
# canonicalize directory separators
198+
$top_item =~ s|[/\\]|/|g if $Is_Win32;
199+
200+
# no trailing / unless path is root
201+
$top_item =~ s|/\z|| unless _is_root($top_item);
193202

194203
$Is_Dir= 0;
195204

196205
if ($follow) {
197206

198-
if (substr($top_item,0,1) eq '/') {
207+
if (_is_absolute($top_item)) {
199208
$abs_dir = $top_item;
200209
}
201210
elsif ($top_item eq $File::Find::current_dir) {
@@ -304,11 +313,7 @@ sub _find_dir($$$) {
304313
my $tainted = 0;
305314
my $no_nlink;
306315

307-
if ($Is_Win32) {
308-
$dir_pref
309-
= ($p_dir =~ m{^(?:\w:[/\\]?|[/\\])$} ? $p_dir : "$p_dir/" );
310-
} elsif ($Is_VMS) {
311-
316+
if ($Is_VMS) {
312317
# VMS is returning trailing .dir on directories
313318
# and trailing . on files and symbolic links
314319
# in UNIX syntax.
@@ -319,7 +324,7 @@ sub _find_dir($$$) {
319324
$dir_pref = ($p_dir =~ m/[\]>]+$/ ? $p_dir : "$p_dir/" );
320325
}
321326
else {
322-
$dir_pref= ( $p_dir eq '/' ? '/' : "$p_dir/" );
327+
$dir_pref = _is_root($p_dir) ? $p_dir : "$p_dir/";
323328
}
324329

325330
local ($dir, $name, $prune);
@@ -471,12 +476,7 @@ sub _find_dir($$$) {
471476
$CdLvl = $Level;
472477
}
473478

474-
if ($Is_Win32) {
475-
$dir_name = ($p_dir =~ m{^(?:\w:[/\\]?|[/\\])$}
476-
? "$p_dir$dir_rel" : "$p_dir/$dir_rel");
477-
$dir_pref = "$dir_name/";
478-
}
479-
elsif ($^O eq 'VMS') {
479+
if ($^O eq 'VMS') {
480480
if ($p_dir =~ m/[\]>]+$/) {
481481
$dir_name = $p_dir;
482482
$dir_name =~ s/([\]>]+)$/.$dir_rel$1/;
@@ -488,7 +488,7 @@ sub _find_dir($$$) {
488488
}
489489
}
490490
else {
491-
$dir_name = ($p_dir eq '/' ? "/$dir_rel" : "$p_dir/$dir_rel");
491+
$dir_name = _is_root($p_dir) ? "$p_dir$dir_rel" : "$p_dir/$dir_rel";
492492
$dir_pref = "$dir_name/";
493493
}
494494

@@ -540,8 +540,8 @@ sub _find_dir_symlnk($$$) {
540540
my $tainted = 0;
541541
my $ok = 1;
542542

543-
$dir_pref = ( $p_dir eq '/' ? '/' : "$p_dir/" );
544-
$loc_pref = ( $dir_loc eq '/' ? '/' : "$dir_loc/" );
543+
$dir_pref = _is_root($p_dir) ? $p_dir : "$p_dir/";
544+
$loc_pref = _is_root($dir_loc) ? $dir_loc : "$dir_loc/";
545545

546546
local ($dir, $name, $fullname, $prune);
547547

@@ -677,7 +677,7 @@ sub _find_dir_symlnk($$$) {
677677
continue {
678678
while (defined($SE = pop @Stack)) {
679679
($dir_loc, $updir_loc, $p_dir, $dir_rel, $byd_flag) = @$SE;
680-
$dir_name = ($p_dir eq '/' ? "/$dir_rel" : "$p_dir/$dir_rel");
680+
$dir_name = _is_root($p_dir) ? "$p_dir$dir_rel" : "$p_dir/$dir_rel";
681681
$dir_pref = "$dir_name/";
682682
$loc_pref = "$dir_loc/";
683683
if ( $byd_flag < 0 ) { # must be finddepth, report dirname now
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
#!./perl
2+
3+
use strict;
4+
use warnings;
5+
6+
use File::Find qw( find finddepth );
7+
use File::Temp qw();
8+
use Test::More;
9+
10+
my $warn_msg;
11+
12+
BEGIN {
13+
$SIG{'__WARN__'} = sub {
14+
$warn_msg = $_[0];
15+
warn "# $_[0]";
16+
return;
17+
}
18+
}
19+
20+
sub test_find_correct_paths_with_follow {
21+
$warn_msg = '';
22+
my $dir = File::Temp->newdir('file-find-XXXXXX', TMPDIR => 1, CLEANUP => 1);
23+
24+
find(
25+
{
26+
follow => 1,
27+
wanted => sub { return },
28+
},
29+
$dir,
30+
);
31+
32+
unlike(
33+
$warn_msg,
34+
qr/Couldn't chdir/,
35+
'find: Derive absolute path correctly with follow => 1',
36+
);
37+
}
38+
39+
sub test_finddepth_correct_paths_with_follow {
40+
$warn_msg = '';
41+
my $dir = File::Temp->newdir('file-find-XXXXXX', TMPDIR => 1, CLEANUP => 1);
42+
43+
finddepth(
44+
{
45+
follow => 1,
46+
wanted => sub { return },
47+
},
48+
$dir,
49+
);
50+
51+
unlike(
52+
$warn_msg,
53+
qr/Couldn't chdir/,
54+
'finddepth: Derive absolute path correctly with follow => 1',
55+
);
56+
}
57+
sub run {
58+
test_find_correct_paths_with_follow;
59+
test_finddepth_correct_paths_with_follow;
60+
done_testing;
61+
}
62+
63+
run();

ext/File-Find/t/find.t

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,7 @@ if ($^O eq 'MSWin32') {
10601060
'wanted' => sub {
10611061
-f or return; # the first call is for $root_dir itself.
10621062
my $got = $File::Find::name;
1063-
my $exp = "$root_dir$expected_first_file";
1063+
(my $exp = "$root_dir$expected_first_file") =~ s|\\|/|g;
10641064
print "# no_chdir=$no_chdir $root_dir '$got'\n";
10651065
is($got, $exp,
10661066
"Win32: Run 'find' with 'no_chdir' set to $no_chdir" );

t/porting/customized.dat

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Regenerate this file using:
22
# cd t
33
# ./perl -I../lib porting/customized.t --regen
4+
AutoLoader cpan/AutoLoader/t/02AutoSplit.t bb90cda13b88599ad45de4b45799d5218afcb6d8
45
ExtUtils::Constant cpan/ExtUtils-Constant/lib/ExtUtils/Constant/Base.pm 7560e1018f806db5689dee78728ccb8374aea741
56
ExtUtils::Constant cpan/ExtUtils-Constant/t/Constant.t 165e9c7132b003fd192d32a737b0f51f9ba4999e
67
Filter::Util::Call pod/perlfilter.pod 545265af2f45741a0e59eecdd0cfc0c9e490c1e8

0 commit comments

Comments
 (0)