You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/07/01 02:35:44 UTC

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

reta opened a new pull request #822:
URL: https://github.com/apache/cxf/pull/822


   A service in question needs to be reachable via multiple paths. The @Path parameter allows for regex matching on a class-level like the following: @Path("/{a: regex}"). A class-level `@PathParam` is created by the name of a. On the server side, the following annotations both correctly receive requests at `/foo/bar`:
   
   ```
   @Path("/{a : foo/bar}")
   @Path("/{a : foo\\/bar}")
   ```
   
   However, on the client side, the following URI(s)
   ```
   final URI uri = UriBuilder
               .fromUri("/")
               .path("{a : foo/bar}")
               .buildFromEncoded("foo/bar");
   ```
   are not properly constructed due to the fact that `/` is used as path segment separator without taking into account the URI template expression.


-- 
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



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

Posted by GitBox <gi...@apache.org>.
reta merged pull request #822:
URL: https://github.com/apache/cxf/pull/822


   


-- 
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



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

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #822:
URL: https://github.com/apache/cxf/pull/822#issuecomment-871866447


   @andymc12 if you have a moment please to take a look, much appreciate it


-- 
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



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

Posted by 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



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

Posted by GitBox <gi...@apache.org>.
andymc12 commented on a change in pull request #822:
URL: https://github.com/apache/cxf/pull/822#discussion_r662371261



##########
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:
       if it goes negative, should we throw an IllegalArgumentException?




-- 
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