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

[filesystem] makeRelativePath does not work correctly from root #16226

Merged
merged 2 commits into from Oct 16, 2015

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Oct 13, 2015

Q A
Bug fix? yes/no
New feature? yes/no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #14066, #14067
License MIT
Doc PR n/a

When using makeRelativePath, it returns an incorrect path when trying to fetch an entry from the root:

  $fs->makePathRelative('/foo/bar/baz', '/');

Actual result:

  ../foo/bar/baz

Expected result:

  foo/bar/baz

As we have specified an absolute path, there is no point on having an .. added. It works, because a root directory has a .. which points to itself, but it could result in issues when the relative path is actually prefixed or concatted.

@fabpot
Copy link
Member Author

fabpot commented Oct 13, 2015

👍

@fabpot
Copy link
Member Author

fabpot commented Oct 16, 2015

Thank you @jaytaph.

@fabpot fabpot merged commit 791b124 into symfony:2.3 Oct 16, 2015
fabpot added a commit that referenced this pull request Oct 16, 2015
… root (jaytaph, fabpot)

This PR was merged into the 2.3 branch.

Discussion
----------

[filesystem] makeRelativePath does not work correctly from root

| Q             | A
| ------------- | ---
| Bug fix?      | yes/no
| New feature?  | yes/no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #1234
| License       | MIT
| Doc PR        | #14066, #14067

When using `makeRelativePath`, it returns an incorrect path when trying to fetch an entry from the root:

      $fs->makePathRelative('/foo/bar/baz', '/');

Actual result:

      ../foo/bar/baz

Expected result:

      foo/bar/baz

As we have specified an absolute path, there is no point on having an `..` added. It works, because a root directory has a `..` which points to itself, but it could result in issues when the relative path is actually prefixed or concatted.

Commits
-------

791b124 fixed CS
7bb394e Added separated handling of root paths
This was referenced Oct 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants