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

Apply path segment normalization #29

Merged
merged 2 commits into from Mar 20, 2017

Conversation

@kanongil
Copy link
Member

kanongil commented Feb 20, 2017

This implements path segment normalization discussed in #28.

I don't use the algorithm defined in the spec, but a segment based one, which I suspect will be faster for most use cases.

I have gated the algorithm behind a simple pre-detection if case, causing it to not be executed for something like 99% of URLs.

I have also added a somewhat comprehensive test suite, testing many of the edge cases. I also tested this new logic with hapi, which still passes the test suite. I encountered 2 failing tests with inert. In both cases the test expected a 403 but got at 404 instead, due to no longer matching the tested route (as expected). I don't consider this an issue, and I can rework the tests to handle the new logic.

kanongil added a commit to kanongil/hapi that referenced this pull request Feb 21, 2017
 * The passed url object is not cloned.
 * Trailing slash stripping should be done after normalization (important if hapijs/call#29 is implemented).
 * Always update pathname derived url properties when path is modified.
@devinivy

This comment has been minimized.

Copy link
Member

devinivy commented Mar 2, 2017

Good stuff! I wouldn't mind seeing a test case for a path containing /. but not traversing, like some/.hidden/file.

@kanongil

This comment has been minimized.

Copy link
Member Author

kanongil commented Mar 2, 2017

Sure, I could add such a test. It won't trigger anything in the code, but I guess it could be nice to have it in case someone tries to refactor it.

@hueniverse hueniverse self-assigned this Mar 20, 2017
@hueniverse hueniverse added the bug label Mar 20, 2017
@hueniverse hueniverse added this to the 4.0.1 milestone Mar 20, 2017
@hueniverse hueniverse merged commit e764f24 into hapijs:master Mar 20, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hueniverse added a commit that referenced this pull request Mar 20, 2017
kanongil added a commit to kanongil/hapi that referenced this pull request May 29, 2017
 * The passed url object is not cloned.
 * Trailing slash stripping should be done after normalization (important if hapijs/call#29 is implemented).
 * Always update pathname derived url properties when path is modified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.