Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

Updated the fix for normalize the URL #49

Merged
merged 6 commits into from
Apr 18, 2018
Merged

Updated the fix for normalize the URL #49

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
577e703
Added README.txt for jar files
jayasheelankumar Apr 13, 2018
f89a979
Updated the URL Normalize logic
jayasheelankumar Apr 13, 2018
a98f613
Removing the draft README
jayasheelankumar Apr 13, 2018
8ef0991
Added a few simple test cases.
kenpaulsen Apr 13, 2018
cc5b481
Updating with few fixes
jayasheelankumar Apr 16, 2018
6069d03
Merge pull request #1 from kenpaulsen/pr-49-suggestions
jayasheelankumar Apr 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions ...mplating/src/main/java/com/sun/jsftemplating/util/fileStreamer/ResourceContentSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,8 @@ public static String normalize(String origPath) {
// Normalize it...
if ((path != null) && (path.length() > 0)) {
path = path.replace('\\', '/');
// Remove leading '/' chars
while ((path.length() > 0) && (path.charAt(0) == '/')) {
path = path.substring(1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

I opened a PR to your PR which adds a few tests.

// Replace all double "//" with "/"

// Replace all double "//" with "/"
while (path.indexOf("//") != -1) {
path = path.replace("//", "/");
}
Expand Down