Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Dancer2::FileUtils::normalize path returns unexpected results for "../../.." #1102

Closed
dboehmer opened this issue Jan 14, 2016 · 6 comments
Closed

Comments

@dboehmer
Copy link

I am writing a Dancer2 app with a 3rd level Perl namespace (Foo::Bar::MyApp). To retrieve a file from my projects root directory I tried to use path( dirname( __FILE__), qw<.. .. .. foo bar.baz> ) but didn't get the expected result.

After some debugging this test file shows how normalize_path is behaving differently from what I expected:

#!/usr/bin/env perl

use Test::More;

BEGIN { use_ok 'Dancer2::FileUtils', qw<normalize_path> }

sub is_path {
    my ( $input => $expected ) = @_;
    is normalize_path($input) => $expected, "$input => $expected";
}

is_path "/a/b/c/d/A/B/C"             => "/a/b/c/d/A/B/C";
is_path "/a/b/c/d/../A/B/C"          => "/a/b/c/A/B/C";
is_path "/a/b/c/d/../../A/B/C"       => "/a/b/A/B/C";
is_path "/a/b/c/d/../../../A/B/C"    => "/a/A/B/C";
is_path "/a/b/c/d/../../../../A/B/C" => "/A/B/C";

done_testing;

Output for me on Linux with Dancer2 0.166000:

normalize_path.t ..
ok 1 - use Dancer2::FileUtils;
ok 2 - /a/b/c/d/A/B/C => /a/b/c/d/A/B/C
ok 3 - /a/b/c/d/../A/B/C => /a/b/c/A/B/C
ok 4 - /a/b/c/d/../../A/B/C => /a/b/A/B/C
not ok 5 - /a/b/c/d/../../../A/B/C => /a/A/B/C
not ok 6 - /a/b/c/d/../../../../A/B/C => /A/B/C
1..6

#   Failed test '/a/b/c/d/../../../A/B/C => /a/A/B/C'
#   at catfile.pl line 9.
#          got: '/a/b/c/A/B/C'
#     expected: '/a/A/B/C'

#   Failed test '/a/b/c/d/../../../../A/B/C => /A/B/C'
#   at catfile.pl line 9.
#          got: '/a/b/A/B/C'
#     expected: '/A/B/C'
# Looks like you failed 2 tests of 6.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/6 subtests 

Test Summary Report
-------------------
normalize_path.t (Wstat: 512 Tests: 6 Failed: 2)
  Failed tests:  5-6
  Non-zero exit status: 2
Files=1, Tests=6,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.02 cusr  0.00 csys =  0.05 CPU)
Result: FAIL
@SysPete
Copy link
Member

SysPete commented Jan 14, 2016

Already fixed in D1 by PerlDancer/Dancer@399bd50 so I guess we need to apply the same fix.

@SysPete
Copy link
Member

SysPete commented Jan 14, 2016

Many thanks @dboehmer pending PR #1103 fixes this issue.

@SysPete
Copy link
Member

SysPete commented Jan 14, 2016

Having thought further on this I think I prefer using Cwd's realpath as a replacement for normalize_path function. We import it anyway but don't use it so it seems someone at some point thought it was a better idea. Will update the PR later today.

@veryrusty
Copy link
Member

Note that normalize_path also works on relative paths and is a "logical" cleanup. Whereas realpath works with absolute paths and will traverse the filesystem. Sometimes you want/need both variants.
( My preference is to apply this PR as is )

@SysPete
Copy link
Member

SysPete commented Jan 14, 2016

And realpath blows up lots of things as I just found out. Also seems that Cwd isn't actually used so removing that from FileUtils so we drop a dep too.

xsawyerx added a commit that referenced this issue Jan 23, 2016
@dboehmer
Copy link
Author

Are you going to make a release with this fix? I've patched this in my local lib but every now and then one of my co-workers stumbles over this after a fresh CPAN install of Dancer2.

xsawyerx added a commit that referenced this issue Apr 19, 2016
    [ BUG FIXES ]
    * GH #1102: Handle multiple '..' in file path utilities.
      (Oleg A. Mamontov, Peter Mottram)
    * GH #1114: Fix missing prereqs as reported by CPANTS.
      (Mohammad S Anwar)
    * GH #1128: Shh warning if optional megasplat is not present.
      (David Precious)
    * GH #1139: Fix incorrect Content-Length header added by AutoPage
      handler (Michael Kröll, Russell Jenkins)
    * GH #1144: Change tt tags to span in skel (Jason Lewis)
    * GH #1046: "no_server_tokens" configuration option doesn't work.
      (Sawyer X)
    # GH #1155, #1157: Fix megasplat value splitting when there are empty
      trailing path segments. (Tatsuhiko Miyagawa, Russell Jenkins)
      NOTE: Paths matching a megasplat that end with a '/' will now include
      an empty string as the last value. For the route pattern '/foo/**',
      the path '/foo/bar', the megasplat gives ['bar'], whereas '/foo/bar/'
      now gives ['bar','']. Joining the array of megasplat values will now
      always be the string matched against for the megasplit.

    [ DOCUMENTATION ]
    * GH #1119: Improve the deployment documentation. (Andrew Beverley)
    * GH #1123: Document import of utf8 pragma. (Victor Adam)
    * GH #1132: Fix spelling mistakes in POD (Gregor Herrmann)
    * GH #1134: Fix spelling errors detected by codespell (James McCoy)
    * GH #1153: Fix POD rendering error. (Sawyer X)

    [ ENHANCEMENTS ]
    * GH #1129: engine.logger.* hooks are called around logging a message.
      (Russell @veryrusty Jenkins)
    * GH #1146: Cleaner display of error context (Vernon Lyon)
    * GH #1085: Add consistent keywords for accessing headers;
      'request_header' for request, 'response_header', 'response_headers'
      and 'push_response_header' for response. (Russell @veryrusty Jenkins)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants