Skip Menu |
Report information
Id: 111510
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: perldoc [at] volkerschatz.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



Subject: PATCH: File::Spec::UNIX->abs2rel() gets it wrong with ".." components
Date: Sat, 3 Mar 2012 11:02:00 +0100 (CET)
To: perlbug [...] perl.org
From: Volker Schatz <perldoc [...] volkerschatz.com>
Download (untitled) / with headers
text/plain 2.3k
Hello, this is a bugfix I submitted to CPAN a while ago (#61451). I have verified that the issue persists in PathTools 3.33 and rerun the test suite with the patched version. In the light of my experience with CPAN, I have added a line to the POD that asks bug reports to go to perlbug@perl.org (last hunk of the patch: @@ -469,6 +494,8 @@). If you have follow-up questions, it would probably be best to CC me, though I will try to watch the perl5-porters list archives. Kind regards, Volker Here is the detailed bug description: File::Spec::UNIX->abs2rel() returns wrong results in a few cases, most of which involve ".." path components. To reproduce, paste the following test cases into: perl -MFile::Spec::Unix -n -e 'print File::Spec::Unix->abs2rel(split),"\n";' ../foo bar/bat bar/bat ../foo foo bar/../bat . . / / Correct results when run at /home/me and no symlinks in base path: ../../../foo ../me/bar/bat ../foo . . Results for File::Spec::Unix from PathTols 3.33: ../../foo ../bar/bat ../../../foo / / The error in the first test case is due to an optimisation applied when both arguments are relative paths, which prepends "/" instead of the current directory. "/../" is then converted to "/" by canonpath(). I have replaced this optimisation by a single call to _cwd() in the following patch. This also fixes the fourth test case. Besides, I have moved checks which make sense only for absolute path arguments to the first branch of the if. (hunk @@ -362,28 +363,32 @@) The error in the last test case arises because a root dir $base is treated specially, and catdir() does not work well for fewer than two path components. The first added line in the following patch catches that. As regards the second and third test case, they can be solved without consulting the filesystem only if no symlinks are involved. Whereever $path contains .. components, the corresponding directory has to be descended into. The following patch does this. (hunk @@ -391,19 +396,39 @@) It can be impossible for abs2rel() to work correctly without looking at the filesystem if $base contains symlinks. I understand from the documentation that the File::Spec modules are not meant to consult the filesystem. Even though the docs state that abs2rel() does not consult the filesystem, the implications could perhaps be made clearer, for example like this: (hunk @@ -348,9 +348,10 @@)
Download Unix.pm.diff
text/plain 4.4k

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 744b
On Sat Mar 03 02:02:56 2012, perldoc@volkerschatz.com wrote: Show quoted text
> > Hello, > > this is a bugfix I submitted to CPAN a while ago (#61451). I have > verified > that the issue persists in PathTools 3.33 and rerun the test suite > with the > patched version. In the light of my experience with CPAN, I have > added a line > to the POD that asks bug reports to go to perlbug@perl.org (last hunk > of the > patch: @@ -469,6 +494,8 @@). > > If you have follow-up questions, it would probably be best to CC me, > though I > will try to watch the perl5-porters list archives. > > Kind regards, > > Volker
Thank you for the patch. It looks good to me. Is there any chance you could add some tests to rel2abs2rel.t? -- Father Chrysostomos
Subject: Re: [perl #111510] PATCH: File::Spec::UNIX->abs2rel() gets it wrong with ".." components
Date: Sat, 26 May 2012 21:04:33 +0200 (CEST)
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Volker Schatz <perldoc [...] volkerschatz.com>
Download (untitled) / with headers
text/plain 1.5k
Show quoted text
> Is there any chance you could add some tests to rel2abs2rel.t?
I don't understand what rel2abs2rel.t does; but Spec.t contains all the other tests for Unix->abs2rel, so this seemed the place to patch: --- Spec.t.orig 2010-07-23 09:55:30.000000000 +0200 +++ Spec.t 2012-05-26 20:54:57.608000003 +0200 @@ -135,6 +135,10 @@ [ "Unix->abs2rel('/t1/t2/t3', '/t1')", 't2/t3' ], [ "Unix->abs2rel('t1/t2/t3', 't1')", 't2/t3' ], [ "Unix->abs2rel('t1/t2/t3', 't4')", '../t1/t2/t3' ], +[ "Unix->abs2rel('.', '.')", '.' ], +[ "Unix->abs2rel('/', '/')", '.' ], +[ "Unix->abs2rel('../t1', 't2/t3')", '../../../t1' ], +[ "Unix->abs2rel('t1', 't2/../t3')", '../t1' ], [ "Unix->rel2abs('t4','/t1/t2/t3')", '/t1/t2/t3/t4' ], [ "Unix->rel2abs('t4/t5','/t1/t2/t3')", '/t1/t2/t3/t4/t5' ], My second test case (by the original count) presents difficulties, as its result contains the name of the current directory. For PathTools-3.33, the following works: [ "Unix->abs2rel('t1/t2', '../t3')", '../PathTools-3.33/t1/t2' ], If the expected result is a static string, it would have to be adapted with every release... Unfortunately this test is independent of the other four, in that it tests the descending into directories when $base contains leading ".."s ($base, not $path as I wrote in my OP). Probably less of a hassle to leave it out, though. Regards, Volker
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 533b
On Sat May 26 12:05:12 2012, perldoc@volkerschatz.com wrote: Show quoted text
>
> > Is there any chance you could add some tests to rel2abs2rel.t?
> > I don't understand what rel2abs2rel.t does; but Spec.t contains all > the other > tests for Unix->abs2rel, so this seemed the place to patch: > > --- Spec.t.orig 2010-07-23 09:55:30.000000000 +0200 > +++ Spec.t 2012-05-26 20:54:57.608000003 +0200 > @@ -135,6 +135,10 @@
Thank you. I’ve applied both your patches, as 70b6afc16df and 593dacfb29387, respectively. -- Father Chrysostomos


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org