Bug #52624 for PathTools: File::Spec::Win32->catdir loses double slash from UNC paths [+PATCH]

Wed Dec 09 18:01:23 2009 jloverso [...] mathworks.com - Ticket created

Subject:

File::Spec::Win32->catdir loses double slash from UNC paths

On Windows, File::Spec->catdir() can no longer reconstruct a UNC path split up by File::Spec->splitdir(). The leading double slash gets output as a single slash. This worked correctly up to 3.25; I know this is broken in 3.27, 3.30, and 3.31 (this might be broken in 3.26; I have not tested that version). This sample script shows the problem: use File::Spec; my $path = "\\\\server\\share\\dir"; print "\npath |$path|\n"; my @dirs = File::Spec->splitdir($path); print "dirs |".join("|",@dirs)."|\n"; my $npath = File::Spec->catdir(@dirs); print "npath |$npath|\n"; print (($path eq $npath) ? "OK paths match\n" : "****ERROR****\n"); With AS1005 (using PathTools 3.25): C:\> d:\Perl-5.10.0-1005\bin\perl fst path |\\server\share\dir| dirs |||server|share|dir| npath |\\server\share\dir| OK paths match But switching in 3.30 (or 3.27 or 3.31): C:\> d:\Perl-5.10.0-1005\bin\perl -IPathTools-3.30/lib fst path |\\server\share\dir| dirs |||server|share|dir| npath |\server\share\dir| ****ERROR**** Notice $npath has only a single leading \.

Wed Dec 09 18:18:42 2009 jloverso [...] mathworks.com - Correspondence added

Subject:

Possible patch

As best as I can tell, the problem is caused by line 426 in File::Spec::Win32::_canon_cat $path =~ s#\A\\##; # \xx --> xx NOTE: this is *not* root That comment is exactly wrong; in this case , that *is* the root. Commenting out that line fixes this bug.

Wed Dec 09 18:18:43 2009 jloverso [...] mathworks.com - Status changed from 'new' to 'open'

Wed Dec 09 18:42:47 2009 kwilliams [...] cpan.org - Correspondence added

This is incorrect usage. If the input string may have a volume, then the caller should do splitpath() first: ($volume,$directories,$file) = File::Spec−>splitpath( $path ); @dirs = File::Spec->splitdir($directories); $dirs = File::Spec->catdir(@dirs); $newpath = File::Spec->catpath($volume,$dirs,$file); Given the path \\server\share\dir, The older File::Spec releases basically thought you had a directory called 'dir' inside a directory called 'share' inside a directory called 'server' inside a directory called ''. As to what should/shouldn't be done now for backwards compatibility, I defer to P5P.

Thu Dec 10 10:18:24 2009 jloverso [...] mathworks.com - Correspondence added

Subject:

Corrected PATCH

The attached patch restores the previous behavior (3.25) by putting the change in the 'legacy/compatibility' support of File::Spec. This patch 'breaks' 5 tests that I'm guessing were originally added for this incompatible change: $ perl -MExtUtils::Command::MM -e "test_harness(0, 'blib/lib')" t/Spec.t t/Spec....# Test 130 got: "\\\\d1\\d2" (t/Spec.t at line 830 fail #130) # Expected: "\\d1\\d2" (File::Spec::Win32->catdir('','','/d1','d2')) # t/Spec.t line 830 is: ok $got, $expected, $function; # Test 132 got: "\\\\d1\\d2" (t/Spec.t at line 830 fail #132) # Expected: "\\d1\\d2" (File::Spec::Win32->catdir('','','//d1','d2')) # Test 143 got: "\\\\" (t/Spec.t at line 830 fail #143) # Expected: "\\" (File::Spec::Win32->catdir('','','..')) # Test 160 got: "\\\\" (t/Spec.t at line 830 fail #160) # Expected: "\\" (File::Spec::Win32->canonpath('////')) # Test 161 got: "\\\\" (t/Spec.t at line 830 fail #161) # Expected: "\\" (File::Spec::Win32->canonpath('//')) FAILED tests 130, 132, 143, 160-161 Failed 5/598 tests, 99.16% okay (less 126 skipped tests: 467 okay, 78.09%) Failed Test Stat Wstat Total Fail List of Failed ------------------------------------------------------------------------------- t/Spec.t 598 5 130 132 143 160-161 126 subtests skipped. Failed 1/1 test scripts. 5/598 subtests failed. Files=1, Tests=598, 0 wallclock secs ( 0.06 cusr + 0.00 csys = 0.06 CPU) Failed 1/1 test programs. 5/598 subtests failed.

==== //3rdparty/tmw/perl-modules/PathTools/lib/File/Spec/Win32.pm#2 - /sandbox/jloverso/ws/3rdparty/tmw/perl-modules/PathTools/lib/File/Spec/Win32.pm ==== @@ -131,8 +131,11 @@ # Legacy / compatibility support # - shift, return _canon_cat( "/", @_ ) - if $_[0] eq ""; + if ($_[0] eq "") { + shift, return _canon_cat( "//", @_ ) + if $_[0] eq ""; + return _canon_cat( "/", @_ ); + } # Compatibility with File::Spec <= 3.26: # catfile('A:', 'foo') should return 'A:\foo'. @@ -149,8 +152,11 @@ # return "" unless @_; - shift, return _canon_cat( "/", @_ ) - if $_[0] eq ""; + if ($_[0] eq "") { + shift, return _canon_cat( "//", @_ ) + if $_[0] eq ""; + return _canon_cat( "/", @_ ); + } # Compatibility with File::Spec <= 3.26: # catdir('A:', 'foo') should return 'A:\foo'. @@ -386,6 +392,8 @@ (?: [\\/] ([^\\/]+) )? [\\/]? }{}xs # UNC volume ? "\\\\$1".( defined $2 ? "\\$2" : "" )."\\" + : $first =~ s{ \A [\\/]{2} }{}x # UNC + ? "\\\\" : $first =~ s{ \A [\\/] }{}x # root dir ? "\\" : "";

Thu Dec 10 10:29:42 2009 jloverso [...] mathworks.com - Correspondence added

On Wed Dec 09 18:42:47 2009, KWILLIAMS wrote: Show quoted text

> This is incorrect usage. If the input string may have a volume, then > the caller should do splitpath() first:

... Show quoted text

> As to what should/shouldn't be done now for backwards compatibility, I > defer to P5P.

Ken, Thanks for your response. I can see that -- and it does work correctly with both versions when doing splitpath() first. The requirement to call splitpath() makes sense, at least when one isn't just thinking of plain 'UNIX-style' paths. Please, at the very least add a note to the Changes and/or README about this incompatibility. Seeing it there would have alerted me to this problem weeks ago. (And I hope you decide to include my backwards compatibility patch) Meanwhile, this incompatibility has broken both some internal code and made a (or a few) CPAN modules work incorrectly when dealing with UNC paths. Right now I've got to decide whether to revert our installed base to File::Spec 3.25, or update the 3.31 to include my patch above.

Thu Dec 10 10:30:46 2009 jloverso [...] mathworks.com - Subject changed from 'File::Spec::Win32->catdir loses double slash from UNC paths' to 'File::Spec::Win32->catdir loses double slash from UNC paths [+PATCH]'