Repository: tomee Updated Branches: refs/heads/develop 857bcc445 -> 25b6a814e
use wrapper to determine if we are in a jaxrs request or not Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/d7f3efe1 Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/d7f3efe1 Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/d7f3efe1 Branch: refs/heads/develop Commit: d7f3efe121a2b3c6b9c205e4c941c2c3da75e6e5 Parents: 857bcc4 Author: Romain Manni-Bucau <[email protected]> Authored: Mon Jan 26 14:29:56 2015 +0100 Committer: Romain Manni-Bucau <[email protected]> Committed: Mon Jan 26 14:29:56 2015 +0100 ---------------------------------------------------------------------- .../AvoidConflictWithWebXmlTest.java | 84 ++++++++++++++++++ ...ictWithWebXmlWithNoResourceMatchingTest.java | 87 ++++++++++++++++++ ...thNoResourceMatchingWithRestSubPathTest.java | 92 ++++++++++++++++++++ .../tomee/webservices/CXFJAXRSFilter.java | 88 ++++++++++++++----- .../tomee/webservices/TomcatRsRegistry.java | 19 ++-- .../org/apache/tomee/jaxrs/ReflectionTest.java | 7 +- 6 files changed, 343 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/d7f3efe1/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlTest.java ---------------------------------------------------------------------- diff --git a/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlTest.java b/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlTest.java new file mode 100644 index 0000000..747d452 --- /dev/null +++ b/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlTest.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.arquillian.tests.jaxrs.staticresources; + +import org.apache.ziplock.IO; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.junit.Arquillian; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.jboss.shrinkwrap.api.Archive; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.jboss.shrinkwrap.descriptor.api.Descriptors; +import org.jboss.shrinkwrap.descriptor.api.webapp30.WebAppDescriptor; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.IOException; +import java.net.URL; + +import static org.junit.Assert.assertEquals; + +@RunWith(Arquillian.class) +public class AvoidConflictWithWebXmlTest { + @Deployment(testable = false) + public static Archive<?> war() { + return ShrinkWrap.create(WebArchive.class, "app.war") + .addClasses(TheResource.class, SimpleServlet.class, PreviousFilter.class) + .addAsWebResource(new StringAsset("JSP <%= 5 %>"), "index.jsp") // works cause name is welcome-file + .setWebXML(new StringAsset( + Descriptors.create(WebAppDescriptor.class) + .createServlet() + .servletName("home") + .jspFile("/index.jsp") + .up() + .createServletMapping() + .servletName("home") + .urlPattern("/*") + .up() + .exportAsString())); + } + + @ArquillianResource + private URL url; + + @Test + public void jaxrs() throws IOException { + assertEquals("resource", IO.slurp(new URL(url.toExternalForm() + "the"))); + } + + @Test + public void servlet() throws IOException { + assertEquals("Servlet!", IO.slurp(new URL(url + "servlet"))); + } + + @Test + public void jsp() throws IOException { + assertEquals("JSP 5", IO.slurp(new URL(url + "index.jsp")).trim()); + } + + @Test + public void home() throws IOException { + assertEquals("JSP 5", IO.slurp(url).trim()); + } + + @Test + public void filterOrder() throws IOException { + assertEquals("I'm the first", IO.slurp(new URL(url.toExternalForm() + "gotFilter"))); + } +} http://git-wip-us.apache.org/repos/asf/tomee/blob/d7f3efe1/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingTest.java ---------------------------------------------------------------------- diff --git a/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingTest.java b/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingTest.java new file mode 100644 index 0000000..929032b --- /dev/null +++ b/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingTest.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.arquillian.tests.jaxrs.staticresources; + +import org.apache.ziplock.IO; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.junit.Arquillian; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.jboss.shrinkwrap.api.Archive; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.jboss.shrinkwrap.descriptor.api.Descriptors; +import org.jboss.shrinkwrap.descriptor.api.webapp30.WebAppDescriptor; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.URL; + +import static org.junit.Assert.assertEquals; + +@RunWith(Arquillian.class) +public class AvoidConflictWithWebXmlWithNoResourceMatchingTest { + @Deployment(testable = false) + public static Archive<?> war() { + return ShrinkWrap.create(WebArchive.class, "app.war") + .addClasses(TheResource.class, SimpleServlet.class, PreviousFilter.class) + .addAsWebResource(new StringAsset("JSP <%= 5 %>"), "sample.jsp") + .setWebXML(new StringAsset( + Descriptors.create(WebAppDescriptor.class) + .createServlet() + .servletName("home") + .jspFile("/sample.jsp") + .up() + .createServletMapping() + .servletName("home") + .urlPattern("/*") + .up() + .exportAsString())); + } + + @ArquillianResource + private URL url; + + @Test + public void jaxrs() throws IOException { + assertEquals("resource", IO.slurp(new URL(url.toExternalForm() + "the"))); + } + + @Test + public void servlet() throws IOException { + assertEquals("Servlet!", IO.slurp(new URL(url + "servlet"))); + } + + // conflict between 2 servlets on pattern /* - here we know it fails cause we use a filter + // but it would be not deterministic otherwise so not better + @Test(expected = FileNotFoundException.class) + public void jsp() throws IOException { + assertEquals("JSP 5", IO.slurp(new URL(url + "index.jsp")).trim()); + } + + @Test(expected = FileNotFoundException.class) // same as for jsp() + public void home() throws IOException { + assertEquals("JSP 5", IO.slurp(url).trim()); + } + + @Test + public void filterOrder() throws IOException { + assertEquals("I'm the first", IO.slurp(new URL(url.toExternalForm() + "gotFilter"))); + } +} http://git-wip-us.apache.org/repos/asf/tomee/blob/d7f3efe1/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingWithRestSubPathTest.java ---------------------------------------------------------------------- diff --git a/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingWithRestSubPathTest.java b/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingWithRestSubPathTest.java new file mode 100644 index 0000000..d2a6b88 --- /dev/null +++ b/arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/staticresources/AvoidConflictWithWebXmlWithNoResourceMatchingWithRestSubPathTest.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.arquillian.tests.jaxrs.staticresources; + +import org.apache.ziplock.IO; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.junit.Arquillian; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.jboss.shrinkwrap.api.Archive; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.jboss.shrinkwrap.descriptor.api.Descriptors; +import org.jboss.shrinkwrap.descriptor.api.webapp30.WebAppDescriptor; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.ws.rs.core.Application; +import java.io.IOException; +import java.net.URL; + +import static org.junit.Assert.assertEquals; + +@RunWith(Arquillian.class) +public class AvoidConflictWithWebXmlWithNoResourceMatchingWithRestSubPathTest { + @Deployment(testable = false) + public static Archive<?> war() { + return ShrinkWrap.create(WebArchive.class, "app.war") + .addClasses(TheResource.class, SimpleServlet.class, PreviousFilter.class) + .addAsWebResource(new StringAsset("JSP <%= 5 %>"), "sample.jsp") + .setWebXML(new StringAsset( + Descriptors.create(WebAppDescriptor.class) + .createServlet() + .servletName("home") + .jspFile("/sample.jsp") + .up() + .createServlet() + .servletName(Application.class.getName()) + .up() + .createServletMapping() + .servletName("home") + .urlPattern("/*") + .up() + .createServletMapping() + .servletName(Application.class.getName()) + .urlPattern("/api") + .up() + .exportAsString())); + } + + @ArquillianResource + private URL url; + + @Test + public void jaxrs() throws IOException { + assertEquals("resource", IO.slurp(new URL(url.toExternalForm() + "api/the"))); + } + + @Test + public void servlet() throws IOException { + assertEquals("Servlet!", IO.slurp(new URL(url + "servlet"))); + } + + @Test + public void jsp() throws IOException { + assertEquals("JSP 5", IO.slurp(new URL(url + "index.jsp")).trim()); + } + + @Test + public void home() throws IOException { + assertEquals("JSP 5", IO.slurp(url).trim()); + } + + @Test + public void filterOrder() throws IOException { + assertEquals("I'm the first", IO.slurp(new URL(url.toExternalForm() + "api/gotFilter"))); + } +} http://git-wip-us.apache.org/repos/asf/tomee/blob/d7f3efe1/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/CXFJAXRSFilter.java ---------------------------------------------------------------------- diff --git a/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/CXFJAXRSFilter.java b/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/CXFJAXRSFilter.java index 436f049..4085175 100644 --- a/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/CXFJAXRSFilter.java +++ b/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/CXFJAXRSFilter.java @@ -16,8 +16,9 @@ */ package org.apache.tomee.webservices; -import org.apache.catalina.servlets.DefaultServlet; -import org.apache.openejb.core.ParentClassLoaderFinder; +import org.apache.catalina.Wrapper; +import org.apache.catalina.connector.Request; +import org.apache.catalina.connector.RequestFacade; import org.apache.openejb.server.cxf.rs.CxfRsHttpListener; import org.apache.openejb.server.httpd.ServletRequestAdapter; import org.apache.openejb.server.httpd.ServletResponseAdapter; @@ -29,27 +30,29 @@ import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Field; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; public class CXFJAXRSFilter implements Filter { - private static final Field SERVLET_FIELD; + private static final Field REQUEST; static { - Field servletFieldTmp = null; try { - final Class<?> clazz = ParentClassLoaderFinder.Helper.get().loadClass("org.apache.catalina.core.ApplicationFilterChain"); - servletFieldTmp = clazz.getDeclaredField("servlet"); - servletFieldTmp.setAccessible(true); - } catch (final Exception e) { - // no-op + REQUEST = RequestFacade.class.getDeclaredField("request"); + } catch (final NoSuchFieldException e) { + throw new IllegalStateException(e); } - SERVLET_FIELD = servletFieldTmp; + REQUEST.setAccessible(true); } private final CxfRsHttpListener delegate; + private final ConcurrentMap<Wrapper, Boolean> mappingByServlet = new ConcurrentHashMap<>(); private final String[] welcomeFiles; + private String mapping; public CXFJAXRSFilter(final CxfRsHttpListener delegate, final String[] welcomeFiles) { this.delegate = delegate; @@ -62,7 +65,7 @@ public class CXFJAXRSFilter implements Filter { @Override public void init(final FilterConfig filterConfig) throws ServletException { - // no-op + mapping = filterConfig.getInitParameter("mapping"); } @Override @@ -76,7 +79,7 @@ public class CXFJAXRSFilter implements Filter { final HttpServletResponse httpServletResponse = HttpServletResponse.class.cast(response); if (CxfRsHttpListener.TRY_STATIC_RESOURCES) { // else 100% JAXRS - if (isServlet(chain)) { + if (servletMappingIsUnderRestPath(httpServletRequest)) { chain.doFilter(request, response); return; } @@ -96,19 +99,62 @@ public class CXFJAXRSFilter implements Filter { } } - // see org.apache.tomcat.util.http.mapper.Mapper.internalMapWrapper - private boolean isServlet(final FilterChain chain) { - // will not work if we are not the first filter - which is likely the case the keep security etc - - // and the chain is wrapped which is more unlikely so this should work as long as these untyped constraints are respeted: - // - org.apache.catalina.core.ApplicationFilterChain name is stable (case on tomcat 8 for now) - // - ApplicationFilterChain as a field servlet with the expected servlet + private boolean servletMappingIsUnderRestPath(final HttpServletRequest request) { + final HttpServletRequest unwrapped = unwrap(request); + if (!RequestFacade.class.isInstance(unwrapped)) { + return false; + } + + final Request tr; try { - return SERVLET_FIELD != null - && "org.apache.catalina.core.ApplicationFilterChain".equals(chain.getClass().getName()) - && !DefaultServlet.class.isInstance(SERVLET_FIELD.get(chain)); + tr = Request.class.cast(REQUEST.get(unwrapped)); } catch (final IllegalAccessException e) { return false; } + final Wrapper wrapper = tr.getWrapper(); + if (wrapper == null || mapping == null) { + return false; + } + + Boolean accept = mappingByServlet.get(wrapper); + if (accept == null) { + accept = false; + if (!"org.apache.catalina.servlets.DefaultServlet".equals(wrapper.getServletClass())) { + for (final String mapping : wrapper.findMappings()) { + if (!mapping.isEmpty() && !"/*".equals(mapping) && !"/".equals(mapping) && mapping.startsWith(this.mapping)) { + accept = true; + break; + } + } + } // else will be handed by getResourceAsStream() + mappingByServlet.putIfAbsent(wrapper, accept); + return accept; + } + + return false; + } + + private HttpServletRequest unwrap(final HttpServletRequest request) { + HttpServletRequest unwrapped = request; + boolean changed; + do { + changed = false; + while (HttpServletRequestWrapper.class.isInstance(unwrapped)) { + final HttpServletRequest tmp = HttpServletRequest.class.cast(HttpServletRequestWrapper.class.cast(unwrapped).getRequest()); + if (tmp != unwrapped) { + unwrapped = tmp; + } else { + changed = false; // quit + break; + } + changed = true; + } + while (ServletRequestAdapter.class.isInstance(unwrapped)) { + unwrapped = ServletRequestAdapter.class.cast(unwrapped).getRequest(); + changed = true; + } + } while (changed); + return unwrapped; } @Override http://git-wip-us.apache.org/repos/asf/tomee/blob/d7f3efe1/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/TomcatRsRegistry.java ---------------------------------------------------------------------- diff --git a/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/TomcatRsRegistry.java b/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/TomcatRsRegistry.java index 774c270..7754adf 100644 --- a/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/TomcatRsRegistry.java +++ b/tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/TomcatRsRegistry.java @@ -104,15 +104,6 @@ public class TomcatRsRegistry implements RsRegistry { final CxfRsHttpListener cxfRsHttpListener = findCxfRsHttpListener(listener); final String description = "tomee-jaxrs-" + listener; - final FilterDef filterDef = new FilterDef(); - filterDef.setAsyncSupported("true"); - filterDef.setDescription(description); - filterDef.setFilterName(description); - filterDef.setDisplayName(description); - filterDef.setFilter(new CXFJAXRSFilter(cxfRsHttpListener, context.findWelcomeFiles())); - filterDef.setFilterClass(CXFJAXRSFilter.class.getName()); - context.addFilterDef(filterDef); - String mapping = completePath; if (!completePath.endsWith("/*")) { // respect servlet spec (!= from our embedded listeners) if (completePath.endsWith("*")) { @@ -124,6 +115,16 @@ public class TomcatRsRegistry implements RsRegistry { final String urlPattern = removeWebContext(webContext, mapping); cxfRsHttpListener.setUrlPattern(urlPattern.substring(0, urlPattern.length() - 1)); + final FilterDef filterDef = new FilterDef(); + filterDef.setAsyncSupported("true"); + filterDef.setDescription(description); + filterDef.setFilterName(description); + filterDef.setDisplayName(description); + filterDef.setFilter(new CXFJAXRSFilter(cxfRsHttpListener, context.findWelcomeFiles())); + filterDef.setFilterClass(CXFJAXRSFilter.class.getName()); + filterDef.addInitParameter("mapping", urlPattern.substring(0, urlPattern.length() - "/*".length())); // just keep base path + context.addFilterDef(filterDef); + final FilterMap filterMap = new FilterMap(); filterMap.addURLPattern(urlPattern); filterMap.setFilterName(filterDef.getFilterName()); http://git-wip-us.apache.org/repos/asf/tomee/blob/d7f3efe1/tomee/tomee-jaxrs/src/test/java/org/apache/tomee/jaxrs/ReflectionTest.java ---------------------------------------------------------------------- diff --git a/tomee/tomee-jaxrs/src/test/java/org/apache/tomee/jaxrs/ReflectionTest.java b/tomee/tomee-jaxrs/src/test/java/org/apache/tomee/jaxrs/ReflectionTest.java index 9bd5fcd..98ebf07 100644 --- a/tomee/tomee-jaxrs/src/test/java/org/apache/tomee/jaxrs/ReflectionTest.java +++ b/tomee/tomee-jaxrs/src/test/java/org/apache/tomee/jaxrs/ReflectionTest.java @@ -16,16 +16,15 @@ */ package org.apache.tomee.jaxrs; +import org.apache.catalina.connector.Request; import org.junit.Test; -import javax.servlet.Servlet; - import static org.junit.Assert.assertEquals; public class ReflectionTest { @Test // a quick test to break the build if upgrading tomcat our reflection will silently be broken public void breakTheBuildIfWhatWeUseChanged() throws ClassNotFoundException, NoSuchFieldException { - final Class<?> clazz = Thread.currentThread().getContextClassLoader().loadClass("org.apache.catalina.core.ApplicationFilterChain"); - assertEquals(Servlet.class, clazz.getDeclaredField("servlet").getType()); + final Class<?> clazz = Thread.currentThread().getContextClassLoader().loadClass("org.apache.catalina.connector.RequestFacade"); + assertEquals(Request.class, clazz.getDeclaredField("request").getType()); } }
