Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces URLEncoder with Guava's PercentEscaper for URL encoding and introduces validation to reject path segments that are exactly '.' or '..' during template instantiation. It also adds comprehensive unit tests to verify these validation rules. The review feedback suggests optimizing the hasPathWildcard helper method by breaking out of the loop early once a path wildcard is found.
| private static boolean hasPathWildcard(ListIterator<Segment> iterator) { | ||
| int start = iterator.nextIndex(); | ||
| boolean found = false; | ||
| while (iterator.hasNext()) { | ||
| Segment seg = iterator.next(); | ||
| if (seg.kind() == SegmentKind.END_BINDING) { | ||
| break; | ||
| } | ||
| if (seg.kind() == SegmentKind.PATH_WILDCARD) { | ||
| found = true; | ||
| } | ||
| } | ||
| restore(iterator, start); | ||
| return found; | ||
| } |
There was a problem hiding this comment.
Once a PATH_WILDCARD segment is found, we can break out of the loop early. Since restore(iterator, start) is called at the end of the method, the iterator's position will be correctly restored regardless of how far we advanced it. This avoids unnecessary iterations over the remaining segments of the binding.
| private static boolean hasPathWildcard(ListIterator<Segment> iterator) { | |
| int start = iterator.nextIndex(); | |
| boolean found = false; | |
| while (iterator.hasNext()) { | |
| Segment seg = iterator.next(); | |
| if (seg.kind() == SegmentKind.END_BINDING) { | |
| break; | |
| } | |
| if (seg.kind() == SegmentKind.PATH_WILDCARD) { | |
| found = true; | |
| } | |
| } | |
| restore(iterator, start); | |
| return found; | |
| } | |
| private static boolean hasPathWildcard(ListIterator<Segment> iterator) { | |
| int start = iterator.nextIndex(); | |
| boolean found = false; | |
| while (iterator.hasNext()) { | |
| Segment seg = iterator.next(); | |
| if (seg.kind() == SegmentKind.END_BINDING) { | |
| break; | |
| } | |
| if (seg.kind() == SegmentKind.PATH_WILDCARD) { | |
| found = true; | |
| break; | |
| } | |
| } | |
| restore(iterator, start); | |
| return found; | |
| } |
Validates URI path template variable segments during
PathTemplate.instantiateto prevent unintended dot-segment ("."and"..") path traversal and ensures strict percent-encoding of non-unreserved URI characters:"."and".."segments in single/multi-segment wildcard (*) bindings (ValidationException: "Invalid value %s for %s.")."/"-delimited segment equal to"."or".."in path wildcard (**) bindings (ValidationException: "Value for %s must not contain segments that are exactly . or ..").PercentEscaper("-_.~", false)so all characters outside[-_.~0-9a-zA-Z]are percent-encoded (e.g., spaces as%20,?as%3F,#as%23,%as%25).