You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by am...@apache.org on 2017/11/11 19:59:55 UTC

[cxf] 01/03: [CXF-7527] getMatchedURIs to avoid duplicate URIs

This is an automated email from the ASF dual-hosted git repository.

amccright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 9e0f7ea38b6afe8f4180082d3a82d9f969a05df3
Author: Andy McCright <j....@gmail.com>
AuthorDate: Thu Nov 9 21:15:59 2017 -0600

    [CXF-7527] getMatchedURIs to avoid duplicate URIs
    
    The UriInfo.getMatchedURIs method currently returns "duplicate" URI
    entries when called from a method that is not annotated with an
    @Path annotation. The "duplicate" URI is basically the same URI but
    with a trailing slash. This was first discovered when using sub
    resource locators, but could also occur in simpler scenarios (like
    a resource method annotated with @GET but not @Path).
    
    When comparing against the javadoc and the RI, it looks like the
    URIs we were returning started with a slash, where the RI and
    javadoc examples do not. This commit removes the starting slash as
    well.
---
 .../org/apache/cxf/jaxrs/impl/UriInfoImpl.java     |  18 +-
 .../org/apache/cxf/jaxrs/impl/UriInfoImplTest.java | 246 +++++++++++++++++----
 2 files changed, 213 insertions(+), 51 deletions(-)

diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java
index 569ec2b..a07dd54 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriInfoImpl.java
@@ -189,13 +189,15 @@ public class UriInfoImpl implements UriInfo {
                     }
                     uris.add(0, createMatchedPath(paths[0].getValue(), rootObjects, decode));
                 }
-                for (URITemplate t : paths) {
-                    if (t != null) {
-                        sumPath.append("/").append(t.getValue());
+                if (paths[1] != null && paths[1].getValue().length() > 1) {
+                    for (URITemplate t : paths) {
+                        if (t != null) {
+                            sumPath.append("/").append(t.getValue());
+                        }
                     }
+                    objects.addAll(templateObjects);
+                    uris.add(0, createMatchedPath(sumPath.toString(), objects, decode));
                 }
-                objects.addAll(templateObjects);
-                uris.add(0, createMatchedPath(sumPath.toString(), objects, decode));
             }
             return uris;
         }
@@ -205,7 +207,11 @@ public class UriInfoImpl implements UriInfo {
 
     private static String createMatchedPath(String uri, List<? extends Object> vars, boolean decode) {
         String uriPath = UriBuilder.fromPath(uri).buildFromEncoded(vars.toArray()).getRawPath();
-        return decode ? HttpUtils.pathDecode(uriPath) : uriPath;
+        uriPath = decode ? HttpUtils.pathDecode(uriPath) : uriPath;
+        if (uriPath.startsWith("/")) {
+            uriPath = uriPath.substring(1);
+        }
+        return uriPath;
     }
     private String doGetPath(boolean decode, boolean addSlash) {
         String path = HttpUtils.getPathToMatch(message, addSlash);
diff --git a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriInfoImplTest.java b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriInfoImplTest.java
index 6c9cbb5..beb8d6e 100644
--- a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriInfoImplTest.java
+++ b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriInfoImplTest.java
@@ -19,14 +19,24 @@
 
 package org.apache.cxf.jaxrs.impl;
 
+import java.lang.reflect.Method;
 import java.net.URI;
+import java.util.ArrayList;
 import java.util.List;
 
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.PathSegment;
+import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
 
+import org.apache.cxf.jaxrs.model.ClassResourceInfo;
+import org.apache.cxf.jaxrs.model.MethodInvocationInfo;
+import org.apache.cxf.jaxrs.model.OperationResourceInfo;
+import org.apache.cxf.jaxrs.model.OperationResourceInfoStack;
 import org.apache.cxf.jaxrs.model.URITemplate;
+import org.apache.cxf.jaxrs.utils.AnnotationUtils;
 import org.apache.cxf.message.Exchange;
 import org.apache.cxf.message.ExchangeImpl;
 import org.apache.cxf.message.Message;
@@ -71,7 +81,7 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativize() {
         UriInfoImpl u = new UriInfoImpl(
-            mockMessage("http://localhost:8080/app/root", "/a/b/c"), null);
+                                        mockMessage("http://localhost:8080/app/root", "/a/b/c"), null);
         assertEquals("Wrong Request Uri", "http://localhost:8080/app/root/a/b/c",
                      u.getRequestUri().toString());
         URI relativized = u.relativize(URI.create("http://localhost:8080/app/root/a/d/e"));
@@ -81,23 +91,23 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativizeAlreadyRelative() throws Exception {
         Message mockMessage = mockMessage("http://localhost:8080/app/root/",
-                "/soup/");
+            "/soup/");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         assertEquals("http://localhost:8080/app/root/soup/", u.getRequestUri()
-                .toString());
+                     .toString());
         URI x = URI.create("x/");
         assertEquals("http://localhost:8080/app/root/x/", u.resolve(x)
-                .toString());
+                     .toString());
         assertEquals("../x/", u.relativize(x).toString());
     }
 
     @Test
     public void testRelativizeNoCommonPrefix() throws Exception {
         Message mockMessage = mockMessage("http://localhost:8080/app/root/",
-                "/soup");
+            "/soup");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         assertEquals("http://localhost:8080/app/root/soup", u.getRequestUri()
-                .toString());
+                     .toString());
         URI otherHost = URI.create("http://localhost:8081/app/root/x");
         assertEquals(otherHost, u.resolve(otherHost));
 
@@ -108,25 +118,25 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativizeChild() throws Exception {
         /** From UriInfo.relativize() javadoc (2013-04-21):
-*
-* <br/><b>Request URI:</b> <tt>http://host:port/app/root/a/b/c</tt>
-* <br/><b>Supplied URI:</b> <tt>a/b/c/d/e</tt>
-* <br/><b>Returned URI:</b> <tt>d/e</tt>
-*
-* NOTE: Although the above is correct JAX-RS API-wise (as of 2013-04-21),
-* it is WRONG URI-wise (but correct API wise)
-* as the request URI is missing the trailing / -- if the request returned HTML at
-* that location, then resolving "d/e" would end up instead at /app/root/a/b/d/e
-* -- see URI.create("/app/root/a/b/c").resolve("d/e"). Therefore the below tests
-* use the slightly modified request URI http://example.com/app/root/a/b/c/ with a trailing /
-*
-* See the test testRelativizeSibling for a non-slash-ending request URI
-*/
+         *
+         * <br/><b>Request URI:</b> <tt>http://host:port/app/root/a/b/c</tt>
+         * <br/><b>Supplied URI:</b> <tt>a/b/c/d/e</tt>
+         * <br/><b>Returned URI:</b> <tt>d/e</tt>
+         *
+         * NOTE: Although the above is correct JAX-RS API-wise (as of 2013-04-21),
+         * it is WRONG URI-wise (but correct API wise)
+         * as the request URI is missing the trailing / -- if the request returned HTML at
+         * that location, then resolving "d/e" would end up instead at /app/root/a/b/d/e
+         * -- see URI.create("/app/root/a/b/c").resolve("d/e"). Therefore the below tests
+         * use the slightly modified request URI http://example.com/app/root/a/b/c/ with a trailing /
+         *
+         * See the test testRelativizeSibling for a non-slash-ending request URI
+         */
         Message mockMessage = mockMessage("http://example.com/app/root/",
-                "/a/b/c/");
+            "/a/b/c/");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         assertEquals("http://example.com/app/root/a/b/c/", u.getRequestUri()
-                .toString());
+                     .toString());
         URI absolute = URI.create("http://example.com/app/root/a/b/c/d/e");
         assertEquals("d/e", u.relativize(absolute).toString());
 
@@ -137,11 +147,11 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativizeSibling() throws Exception {
         Message mockMessage = mockMessage("http://example.com/app/root/",
-                "/a/b/c.html");
+            "/a/b/c.html");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         // NOTE: No slash in the end!
         assertEquals("http://example.com/app/root/a/b/c.html", u
-                .getRequestUri().toString());
+                     .getRequestUri().toString());
         URI absolute = URI.create("http://example.com/app/root/a/b/c.pdf");
         assertEquals("c.pdf", u.relativize(absolute).toString());
 
@@ -152,11 +162,11 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativizeGrandParent() throws Exception {
         Message mockMessage = mockMessage("http://example.com/app/root/",
-                "/a/b/c/");
+            "/a/b/c/");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         // NOTE: All end with slashes (imagine they are folders)
         assertEquals("http://example.com/app/root/a/b/c/", u.getRequestUri()
-                .toString());
+                     .toString());
         URI absolute = URI.create("http://example.com/app/root/a/");
         // Need to go two levels up from /a/b/c/ to /a/
         assertEquals("../../", u.relativize(absolute).toString());
@@ -168,11 +178,11 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativizeCousin() throws Exception {
         Message mockMessage = mockMessage("http://example.com/app/root/",
-                "/a/b/c/");
+            "/a/b/c/");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         // NOTE: All end with slashes (imagine they are folders)
         assertEquals("http://example.com/app/root/a/b/c/", u.getRequestUri()
-                .toString());
+                     .toString());
         URI absolute = URI.create("http://example.com/app/root/a/b2/c2/");
         // Need to go two levels up from /a/b/c/ to /a/
         assertEquals("../../b2/c2/", u.relativize(absolute).toString());
@@ -184,19 +194,19 @@ public class UriInfoImplTest extends Assert {
     @Test
     public void testRelativizeOutsideBase() throws Exception {
         Message mockMessage = mockMessage("http://example.com/app/root/",
-                "/a/b/c/");
+            "/a/b/c/");
         UriInfoImpl u = new UriInfoImpl(mockMessage, null);
         // NOTE: All end with slashes (imagine they are folders)
         assertEquals("http://example.com/app/root/a/b/c/", u.getRequestUri()
-                .toString());
+                     .toString());
         URI absolute = URI.create("http://example.com/otherapp/fred.txt");
 
         assertEquals("../../../../../otherapp/fred.txt", u.relativize(absolute)
-                .toString());
+                     .toString());
 
         URI relativeToBase = URI.create("../../otherapp/fred.txt");
         assertEquals("../../../../../otherapp/fred.txt",
-                u.relativize(relativeToBase).toString());
+                     u.relativize(relativeToBase).toString());
     }
 
     @Test
@@ -217,7 +227,7 @@ public class UriInfoImplTest extends Assert {
                      u.getAbsolutePath().toString());
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz/", "/bar"),
-                                        null);
+                            null);
         assertEquals("Wrong absolute path", "http://localhost:8080/baz/bar",
                      u.getAbsolutePath().toString());
 
@@ -259,7 +269,7 @@ public class UriInfoImplTest extends Assert {
         assertEquals("Wrong absolute path", "http://localhost:8080/baz%20foo/bar",
                      u.getAbsolutePath().toString());
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz/%20foo", "/bar%20foo"),
-                                        null);
+                            null);
         assertEquals("Wrong absolute path", "http://localhost:8080/baz/%20foo/bar%20foo",
                      u.getAbsolutePath().toString());
 
@@ -279,7 +289,7 @@ public class UriInfoImplTest extends Assert {
         assertEquals("Wrong query value", qps.getFirst("n"), "1%202");
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz", "/bar",
-                                        "N=0&n=1%202&n=3&b=2&a%2Eb=ab"),
+            "N=0&n=1%202&n=3&b=2&a%2Eb=ab"),
                             null);
 
         qps = u.getQueryParameters();
@@ -298,7 +308,7 @@ public class UriInfoImplTest extends Assert {
         assertEquals("unexpected queries", 0, u.getQueryParameters().size());
 
         Message m = mockMessage("http://localhost:8080/baz", "/bar",
-                                "N=1%202&n=3&b=2&a%2Eb=ab");
+            "N=1%202&n=3&b=2&a%2Eb=ab");
         m.put("org.apache.cxf.http.case_insensitive_queries", "true");
 
         u = new UriInfoImpl(m, null);
@@ -315,7 +325,7 @@ public class UriInfoImplTest extends Assert {
     public void testGetRequestURI() {
 
         UriInfo u = new UriInfoImpl(mockMessage("http://localhost:8080/baz/bar", "/foo", "n=1%202"),
-                            null);
+                                    null);
 
         assertEquals("Wrong request uri", "http://localhost:8080/baz/bar/foo?n=1%202",
                      u.getRequestUri().toString());
@@ -325,7 +335,7 @@ public class UriInfoImplTest extends Assert {
     public void testGetRequestURIWithEncodedChars() {
 
         UriInfo u = new UriInfoImpl(mockMessage("http://localhost:8080/baz/bar", "/foo/%20bar", "n=1%202"),
-                            null);
+                                    null);
 
         assertEquals("Wrong request uri", "http://localhost:8080/baz/bar/foo/%20bar?n=1%202",
                      u.getRequestUri().toString());
@@ -344,7 +354,7 @@ public class UriInfoImplTest extends Assert {
         values.clear();
         new URITemplate("/{id}").match("/bar%201", values);
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz", "/bar%201"),
-                                        values);
+                            values);
 
         MultivaluedMap<String, String> tps = u.getPathParameters(false);
         assertEquals("Number of templates is wrong", 1, tps.size());
@@ -365,7 +375,7 @@ public class UriInfoImplTest extends Assert {
         new URITemplate("/bar").match("/bar", values);
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz", "/bar"),
-                                        values);
+                            values);
         assertEquals("unexpected templates", 0, u.getPathParameters().size());
     }
 
@@ -376,7 +386,7 @@ public class UriInfoImplTest extends Assert {
         assertEquals("Wrong base path", "http://localhost:8080/baz",
                      u.getBaseUri().toString());
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz/", null),
-                                        null);
+                            null);
         assertEquals("Wrong base path", "http://localhost:8080/baz/",
                      u.getBaseUri().toString());
     }
@@ -385,20 +395,20 @@ public class UriInfoImplTest extends Assert {
     public void testGetPath() {
 
         UriInfoImpl u = new UriInfoImpl(mockMessage("http://localhost:8080/bar/baz",
-                                                    "/baz"),
+            "/baz"),
                                         null);
         assertEquals("Wrong path", "baz", u.getPath());
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/bar/baz",
-                            "/bar/baz"), null);
+            "/bar/baz"), null);
         assertEquals("Wrong path", "/", u.getPath());
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/bar/baz/",
-                "/bar/baz/"), null);
+            "/bar/baz/"), null);
         assertEquals("Wrong path", "/", u.getPath());
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz", "/baz/bar%201"),
-                                        null);
+                            null);
         assertEquals("Wrong path", "bar 1", u.getPath());
 
         u = new UriInfoImpl(mockMessage("http://localhost:8080/baz", "/baz/bar%201"),
@@ -408,6 +418,151 @@ public class UriInfoImplTest extends Assert {
 
     }
 
+    @Path("foo")
+    public static class RootResource {
+
+        @GET
+        public Response get() {
+            return null;
+        }
+
+        @GET
+        @Path("bar")
+        public Response getSubMethod() {
+            return null;
+        }
+
+        @Path("sub")
+        public SubResource getSubResourceLocator() {
+            return new SubResource();
+        }
+    }
+
+    public static class SubResource {
+        @GET
+        public Response getFromSub() {
+            return null;
+        }
+
+        @GET
+        @Path("subSub")
+        public Response getFromSubSub() {
+            return null;
+        }
+    }
+
+    private static ClassResourceInfo getCri(Class<?> clazz, boolean setUriTemplate) {
+        ClassResourceInfo cri = new ClassResourceInfo(clazz);
+        Path path = AnnotationUtils.getClassAnnotation(clazz, Path.class);
+        if (setUriTemplate) {
+            cri.setURITemplate(URITemplate.createTemplate(path));
+        }
+        return cri;
+    }
+
+    private static OperationResourceInfo getOri(ClassResourceInfo cri, String methodName) throws Exception {
+        Method method = cri.getResourceClass().getMethod(methodName);
+        OperationResourceInfo ori = new OperationResourceInfo(method, cri);
+        ori.setURITemplate(URITemplate.createTemplate(AnnotationUtils.getMethodAnnotation(method, Path.class)));
+        return ori;
+    }
+
+    private static List<String> getMatchedURIs(UriInfo u) {
+        List<String> matchedUris = u.getMatchedURIs();
+        //        for (String s : matchedUris) {
+        //            System.out.println(s);
+        //        }
+        return matchedUris;
+    }
+
+    @Test
+    public void testGetMatchedURIsRoot() throws Exception {
+        System.out.println("testGetMatchedURIsRoot");
+        Message m = mockMessage("http://localhost:8080/app", "/foo");
+        OperationResourceInfoStack oriStack = new OperationResourceInfoStack();
+        ClassResourceInfo cri = getCri(RootResource.class, true);
+        OperationResourceInfo ori = getOri(cri, "get");
+
+        MethodInvocationInfo miInfo = new MethodInvocationInfo(ori, RootResource.class, new ArrayList<String>());
+        oriStack.push(miInfo);
+        m.put(OperationResourceInfoStack.class, oriStack);
+
+        UriInfoImpl u = new UriInfoImpl(m);
+        List<String> matchedUris = getMatchedURIs(u);
+        assertEquals(1, matchedUris.size());
+        assertTrue(matchedUris.contains("foo"));
+    }
+
+    @Test
+    public void testGetMatchedURIsRootSub() throws Exception {
+        System.out.println("testGetMatchedURIsRootSub");
+        Message m = mockMessage("http://localhost:8080/app", "/foo/bar");
+        OperationResourceInfoStack oriStack = new OperationResourceInfoStack();
+        ClassResourceInfo cri = getCri(RootResource.class, true);
+        OperationResourceInfo ori = getOri(cri, "getSubMethod");
+
+        MethodInvocationInfo miInfo = new MethodInvocationInfo(ori, RootResource.class, new ArrayList<String>());
+        oriStack.push(miInfo);
+        m.put(OperationResourceInfoStack.class, oriStack);
+
+        UriInfoImpl u = new UriInfoImpl(m);
+        List<String> matchedUris = getMatchedURIs(u);
+        assertEquals(2, matchedUris.size());
+        assertEquals("foo/bar", matchedUris.get(0));
+        assertEquals("foo", matchedUris.get(1));
+    }
+
+    @Test
+    public void testGetMatchedURIsSubResourceLocator() throws Exception {
+        System.out.println("testGetMatchedURIsSubResourceLocator");
+        Message m = mockMessage("http://localhost:8080/app", "/foo/sub");
+        OperationResourceInfoStack oriStack = new OperationResourceInfoStack();
+        ClassResourceInfo rootCri = getCri(RootResource.class, true);
+        OperationResourceInfo rootOri = getOri(rootCri, "getSubResourceLocator");
+
+        MethodInvocationInfo miInfo = new MethodInvocationInfo(rootOri, RootResource.class, new ArrayList<String>());
+        oriStack.push(miInfo);
+
+        ClassResourceInfo subCri = getCri(SubResource.class, false);
+        OperationResourceInfo subOri = getOri(subCri, "getFromSub");
+
+        miInfo = new MethodInvocationInfo(subOri, SubResource.class, new ArrayList<String>());
+        oriStack.push(miInfo);
+        m.put(OperationResourceInfoStack.class, oriStack);
+
+        UriInfoImpl u = new UriInfoImpl(m);
+        List<String> matchedUris = getMatchedURIs(u);
+        assertEquals(2, matchedUris.size());
+        assertEquals("foo/sub", matchedUris.get(0));
+        assertEquals("foo", matchedUris.get(1));
+    }
+
+    @Test
+    public void testGetMatchedURIsSubResourceLocatorSubPath() throws Exception {
+        System.out.println("testGetMatchedURIsSubResourceLocatorSubPath");
+        Message m = mockMessage("http://localhost:8080/app", "/foo/sub/subSub");
+        OperationResourceInfoStack oriStack = new OperationResourceInfoStack();
+        ClassResourceInfo rootCri = getCri(RootResource.class, true);
+        OperationResourceInfo rootOri = getOri(rootCri, "getSubResourceLocator");
+
+        MethodInvocationInfo miInfo = new MethodInvocationInfo(rootOri, RootResource.class, new ArrayList<String>());
+        oriStack.push(miInfo);
+
+        ClassResourceInfo subCri = getCri(SubResource.class, false);
+        OperationResourceInfo subOri = getOri(subCri, "getFromSubSub");
+
+        miInfo = new MethodInvocationInfo(subOri, SubResource.class, new ArrayList<String>());
+        oriStack.push(miInfo);
+        m.put(OperationResourceInfoStack.class, oriStack);
+
+        UriInfoImpl u = new UriInfoImpl(m);
+        List<String> matchedUris = getMatchedURIs(u);
+        assertEquals(3, matchedUris.size());
+        assertEquals("foo/sub/subSub", matchedUris.get(0));
+        assertEquals("foo/sub", matchedUris.get(1));
+        assertEquals("foo", matchedUris.get(2));
+    }
+
     private Message mockMessage(String baseAddress, String pathInfo) {
         return mockMessage(baseAddress, pathInfo, null, null);
     }
@@ -435,3 +590,4 @@ public class UriInfoImplTest extends Assert {
     }
 
 }
+

-- 
To stop receiving notification emails like this one, please contact
"commits@cxf.apache.org" <co...@cxf.apache.org>.