[GitHub] [cxf] reta commented on a change in pull request #822: CXF-8557: Incorrect Proxy Path Segmenting when @Path Annotation RegexExpression Contains '/'

[GitBox <gi...@apache.org>]

reta commented on a change in pull request #822:
URL: https://github.com/apache/cxf/pull/822#discussion_r663560255



##########
File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
##########
@@ -181,20 +181,51 @@ public static boolean isStreamingOutType(Class<?> type) {
         return getPathSegments(thePath, decode, true);
     }
 
+    /**
+     * Parses path segments taking into account the URI templates and template regexes. Per RFC-3986, 
+     * "A path consists of a sequence of path segments separated by a slash ("/") character.", however
+     * it is possible to include slash ("/") inside template regex, for example {a:b/c}. In this case,
+     * the whole template definition is extracted as a path segment, without breaking it.
+     * @param thePath path
+     * @param decode should the path segments be decoded or not
+     * @param ignoreLastSlash should the last slash be ignored or not
+     * @return
+     */
     public static List<PathSegment> getPathSegments(String thePath, boolean decode,
                                                     boolean ignoreLastSlash) {
-        List<PathSegment> theList =
-            Arrays.asList(thePath.split("/")).stream()
-            .filter(StringUtils.notEmpty())
-            .map(p -> new PathSegmentImpl(p, decode))
-            .collect(Collectors.toList());
-
-        int len = thePath.length();
-        if (len > 0 && thePath.charAt(len - 1) == '/') {
+        
+        final List<PathSegment> segments = new ArrayList<>();
+        int depth = 0;
+        int start = 0;
+        for (int i = 0; i < thePath.length(); ++i) {
+            if (thePath.charAt(i) == '/') {
+                // The '/' is in template (regex) definition
+                if (depth > 0) {
+                    continue;
+                } else if (start != i) {
+                    final String segment = thePath.substring(start, i);
+                    segments.add(new PathSegmentImpl(segment, decode));
+                }
+                
+                // advance the positions, empty path segments
+                start = i + 1;
+            } else if (thePath.charAt(i) == '{') {
+                ++depth;
+            } else if (thePath.charAt(i) == '}') {
+                --depth; // could go negative, since the template could be unbalanced

Review comment:
       Thanks a lot for looking, @andymc12 . I was also not sure what the correct behavior should be. I think throwing the exception in this case would be surprising to the users (we may break existing code), however I think we have to address this issue in any case. I would suggest to fallback to straightforward `/` separator (current implementation) if we detect in unparsable template definitions (like unbalanced curly braces). What do you think?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org