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 <[email protected]> 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 "[email protected]" <[email protected]>.
