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

Bug fix for relative directory removal #2055

Merged
merged 2 commits into from
Dec 6, 2012
Merged

Conversation

chernjie
Copy link
Contributor

@chernjie chernjie commented Dec 6, 2012

This patch attempts to fix two bugs:

  • for segments that ends with ".." e.g. /user/username../details, this should not be replaced
  • current solution only replace double slashes, this solution removes an infinite number of recurring slashes

@chernjie
Copy link
Contributor Author

chernjie commented Dec 6, 2012

UnitTest: https://gist.github.com/4221865

time php UnitTest-e8af5d1.php
. ../
. /../
. abc/../
. ../def
. /../def
. abc/../../def
. abc../
. abc/def../
. abc/def../ghi
. abc//def
. abc///def
. abc////def
. 0/abc
. 00/abc
. abc/0
. abc/00
. abc/0/def
. abc/00/def

This fixes two bugs:
- for segments that ends with ".." e.g. /user/username../details, this should not be replaced
- current solution only replace double slashes, this solutions removes the infinite number of recurring slashes
@narfbg
Copy link
Contributor

narfbg commented Dec 6, 2012

Nice! However, in order to get this into CI, some adjustments need to be made in order to meet the styleguide requiremens. I'll comment on the diff.

Edit: Also, if this fixes a bug - it should be noted in the changelog.


/**
* Remove relative directory (../) and multi slashes (///)
* @param string $url
Copy link
Contributor

Choose a reason for hiding this comment

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

Use only tabs as separators - no spaces (between 'string' and '$url' + although followed by a tab, you have spaces after both @param and @return).
Additionally, for the sake of consistency there should be an empty description line between 228 & 229.

narfbg added a commit that referenced this pull request Dec 6, 2012
Bug fix for relative directory removal
@narfbg narfbg merged commit 3fd3345 into bcit-ci:develop Dec 6, 2012
@chernjie
Copy link
Contributor Author

chernjie commented Dec 6, 2012

@narfbg I realized there's another snippet that also shares the same code
chernjie@5addf14

@narfbg
Copy link
Contributor

narfbg commented Dec 6, 2012

b2280ce

@chernjie
Copy link
Contributor Author

chernjie commented Dec 6, 2012

That's fast! :p

nonchip pushed a commit to nonchip/CodeIgniter that referenced this pull request Jun 29, 2013
Bug fix for relative directory removal
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.

2 participants