kenahoo / Path-Class

Cross-platform path specification manipulation
http://search.cpan.org/dist/Path-Class/
15 stars 28 forks source link

Path::Class::Dir->contains doesn't handle ".." #43

Closed nrdvana closed 8 years ago

nrdvana commented 8 years ago

The documentation for contains says that it checks the filesystem to see if one directory is contained within another, however

perl -e 'use Path::Class "dir"; print dir("lib")->contains(dir("lib","..."))."\n"'

shows that it didn't actually resolve the directories or even logically deal with the "..".

kenahoo commented 8 years ago

I'm assuming you meant two dots instead of three dots in the example?

nrdvana commented 8 years ago

yes :-)

Also to reproduce the test there needs to be a subdirectory named "lib".

kenahoo commented 8 years ago

I added a fix & tests for this, want to take a look?

vanstyn commented 8 years ago

@kenahoo, @nrdvana --

The changes made in https://github.com/kenahoo/Path-Class/commit/935eeede90dda31384adf54df0fb2d55a3f856a9 introduces a regression (confirmed with git bisect)

Starting with that commit, the following cmdline prints FAIL!!:

mkdir -p deleteme/test/path
touch deleteme/test/path/somefile
perl -Ilib -e '
  use Path::Class "dir", "file";
  dir("deleteme/test/../test/path/")
    ->contains(file("deleteme/test/../test/path/somefile"))
    or print "FAIL!!\n"
'

This change is breaking some of the auto-generated dev scripts for RapidApp users, which are already out in the wild, so the sooner this can be fixed (broken in v0.36 on CPAN) the better.

Thanks

nrdvana commented 8 years ago

Sorry for not following up sooner. My issue was resolved, but I found the problem affecting vanstyn, and went ahead and fixed them in pull request #48.

kenahoo commented 8 years ago

Now that #48 is merged, is this issue resolved to everyone's satisfaction?

vanstyn commented 8 years ago

Yes! Can we ship to CPAN quickly??

kenahoo commented 8 years ago

Sure. Will do now.

vanstyn commented 8 years ago

Thank you!!!

kenahoo commented 8 years ago

No problem, thanks for reaching out to get it fixed.

nrdvana commented 8 years ago

No prob, thanks for authoring a great module like this!

BTW, my original use case was to check web URLs vs. a path of statically-served files

if (dir($static_path)->contains(file($static_path, @path)) { ... }

and I wanted to make sure that @path had not escaped this directory via ".." or a symlink.