Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove extra '/' when findShortestPath $from is a directory #4105

Merged
merged 2 commits into from Jun 5, 2015
Merged

Remove extra '/' when findShortestPath $from is a directory #4105

merged 2 commits into from Jun 5, 2015

Conversation

gmsantos
Copy link
Contributor

@gmsantos gmsantos commented Jun 3, 2015

Resolves #3024

Passing the third argument as true on findShortestPath method, a dummy file is added to $from.
If $from ends with / (e.g. C:/ , /foo/bar/ ), a second / is inserted resulting on a invalid path.

@alcohol
Copy link
Member

alcohol commented Jun 4, 2015

Please rename the function as it does not relate to the context. To check if you're dealing with a directory you would use http://php.net/is_dir. You are simply interested in knowing if the string ends with a / or not. That doesn't have anything to do with it actually being a directory.

The problem here is that after the normalizePath() call (which strips trailing / characters), root paths remain / (on *nix) and <DRIVE>:/ (on windows). This then turns $from into //dummy_file on *nix and into c://dummy_file on windows. The function then correctly determines the path on *nix, but fails on windows.

On *nix the $commonPath ends up being / which then results in simply returning $to as shortest path. On Windows the $commonPath value is c: which does not equal / so it continues processing and ends up with the leading ../.

* @param string $path
* @return bool
*/
public static function isDirectory($path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it is public ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that should definitely be private or made to work with \ paths as well, because as is it depends on the path being normalized first.

Seldaek added a commit that referenced this pull request Jun 5, 2015
Remove extra '/' when findShortestPath $from is a directory
@Seldaek Seldaek merged commit 7351136 into composer:master Jun 5, 2015
@Seldaek
Copy link
Member

Seldaek commented Jun 5, 2015

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants