Updated the fix for normalize the URL #49
Conversation
| // Remove leading '/' chars | ||
| while ((path.length() > 0) && (path.charAt(0) == '/')) { | ||
| path = path.substring(1); | ||
| } |
There was a problem hiding this comment.
I think it is expected that leading '/' characters are stripped during this method. You could move the stripping of leading '/''s to the end of this method instead of doing it 1st. And you could add a leading '/' at the beginning to ensure it always has one... that might work. I suspect w/ this latest the path ../bad/path would fail to throw an exception.
Also, I recommend that you check in these test cases (I can submit a PR if you'd like). This way it will ensure these cases always get considered when making changes.
There was a problem hiding this comment.
I opened a PR to your PR which adds a few tests.
|
@kenpaulsen Thanks for your comments. I have updated the fix considering all the cases. |
| // which is fine, unless it is also at the beginning... | ||
| if (path.equals("..")) { | ||
| path = null; | ||
| } |
There was a problem hiding this comment.
Could do an else if (path.endsWith("/") to avoid the need to check for null. But looks good either way!
|
Minor (optional) comment. Looks good! |
Added a few simple test cases.
Updated the fix for normalize the URL