TOMEE-4406 - Improved request matching in CXFJAXRSFilter

arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/faces/AvoidConflictWithFacesTest.java

@@ -0,0 +1,70 @@
+/*
+ * 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.faces;
+
+import org.apache.openejb.arquillian.tests.jaxrs.JaxrsTest;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+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.facesconfig22.WebFacesConfigDescriptor;
+import org.jboss.shrinkwrap.descriptor.api.webapp30.WebAppDescriptor;
+import org.jboss.shrinkwrap.descriptor.api.webcommon30.WebAppVersionType;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+
+// Test that reproduces https://issues.apache.org/jira/browse/TOMEE-4406
+@RunWith(Arquillian.class)
+public class AvoidConflictWithFacesTest extends JaxrsTest {
+    @Deployment(testable = false)
+    public static WebArchive createDeployment() {
+        WebFacesConfigDescriptor facesConfig = Descriptors.create(WebFacesConfigDescriptor.class);
+
+        WebAppDescriptor webXml = Descriptors.create(WebAppDescriptor.class)
+                .version(WebAppVersionType._3_0)
+                .createServlet()
+                    .servletName("Faces Servlet")
+                    .servletClass("jakarta.faces.webapp.FacesServlet")
+                .up()
+                .createServletMapping()
+                    .servletName("Faces Servlet")
+                    .urlPattern("*.xhtml")
+                .up();
+
+        return ShrinkWrap.create(WebArchive.class)
+                .addClasses(MyRestApi.class, MyRestApplication.class)
+                .setWebXML(new StringAsset(webXml.exportAsString()))
+                .addAsWebInfResource(new StringAsset(facesConfig.exportAsString()), "faces-config.xml")
+                .addAsResource(new StringAsset("Hello from Faces"), "META-INF/resources/my-resources/hello.txt");
+    }
+
+    @Test
+    public void facesResourceAccessible() throws IOException {
+        assertEquals("Hello from Faces", get("/jakarta.faces.resources/hello.txt.xhtml?ln=my-resources"));
+    }
+
+    @Test
+    public void validateJaxrsDeployed() throws IOException {
+        assertEquals("Hello world!", get("/api/test"));
+    }
+}

arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/faces/MyRestApi.java

@@ -0,0 +1,31 @@
+/*
+ * 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.faces;
+
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.core.Response;
+
+@Path("/api")
+public class MyRestApi {
+
+    @GET
+    @Path("/test")
+    public Response getTest() {
+        return Response.ok("Hello world!").build();
+    }
+}

arquillian/arquillian-tomee-tests/arquillian-tomee-jaxrs-tests/src/test/java/org/apache/openejb/arquillian/tests/jaxrs/faces/MyRestApplication.java

@@ -0,0 +1,22 @@
+/*
+ * 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.faces;
+
+import jakarta.ws.rs.core.Application;
+
+public class MyRestApplication extends Application {
+}

server/openejb-cxf-rs/src/main/java/org/apache/openejb/server/cxf/rs/CxfRsHttpListener.java

@@ -351,44 +351,37 @@ private Application findApplication() {
     }
 
     private boolean applicationProvidesResources(final Application application) {
-        try {
-            if (application == null) {
-                return false;
-            }
-            if (InternalApplication.class.isInstance(application) && (InternalApplication.class.cast(application).getOriginal() == null)) {
-                return false;
-            }
-            return !application.getClasses().isEmpty() || !application.getSingletons().isEmpty();
-        } catch (final Exception e) {
+        if (application == null) {
             return false;
         }
+
+        Set<Class<?>> classes = application.getClasses();
+        Set<Object> singletons = application.getSingletons();
+        return classes != null && !classes.isEmpty() || singletons != null && !singletons.isEmpty();
     }
 
     public boolean isCXFResource(final HttpServletRequest request) {
         try {
-            Application application = findApplication();
-            if (!applicationProvidesResources(application)) {
-                JAXRSServiceImpl service = (JAXRSServiceImpl) server.getEndpoint().getService();
-
-                if (service == null) {
-                    return false;
-                }
+            if (!applicationProvidesResources(findApplication())) { // nothing is registered in JAX-RS
+                return false;
+            }
 
-                String pathToMatch = HttpUtils.getPathToMatch(request.getServletPath(), pattern, true);
+            JAXRSServiceImpl service = (JAXRSServiceImpl) server.getEndpoint().getService();
+            if (service == null) {
+                return false;
+            }
 
-                final List<ClassResourceInfo> resources = service.getClassResourceInfos();
-                for (final ClassResourceInfo info : resources) {
-                    if (info.getResourceClass() == null || info.getURITemplate() == null) { // possible?
-                        continue;
-                    }
+            String pathToMatch = HttpUtils.getPathToMatch(request.getServletPath(), pattern, true);
+            final List<ClassResourceInfo> resources = service.getClassResourceInfos();
+            for (final ClassResourceInfo info : resources) {
+                if (info.getResourceClass() == null || info.getURITemplate() == null) { // possible?
+                    continue;
+                }
 
-                    final MultivaluedMap<String, String> parameters = new MultivaluedHashMap<>();
-                    if (info.getURITemplate().match(pathToMatch, parameters)) {
-                        return true;
-                    }
+                final MultivaluedMap<String, String> parameters = new MultivaluedHashMap<>();
+                if (info.getURITemplate().match(pathToMatch, parameters)) {
+                    return true;
                 }
-            } else {
-                return true;
             }
         } catch (final Exception e) {
             LOGGER.info("No JAX-RS service");

tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/CXFJAXRSFilter.java

@@ -16,43 +16,24 @@
  */
 package org.apache.tomee.webservices;
 
-import org.apache.catalina.Wrapper;
-import org.apache.catalina.connector.Request;
-import org.apache.catalina.connector.RequestFacade;
+import jakarta.servlet.ServletRegistration;
 import org.apache.openejb.server.cxf.rs.CxfRsHttpListener;
 import org.apache.openejb.server.httpd.ServletRequestAdapter;
 import org.apache.openejb.server.httpd.ServletResponseAdapter;
 
 import jakarta.servlet.Filter;
 import jakarta.servlet.FilterChain;
-import jakarta.servlet.FilterConfig;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.ServletRequest;
 import jakarta.servlet.ServletResponse;
 import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletRequestWrapper;
 import jakarta.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 REQUEST;
-    static {
-        try {
-            REQUEST = RequestFacade.class.getDeclaredField("request");
-        } catch (final NoSuchFieldException e) {
-            throw new IllegalStateException(e);
-        }
-        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;
@@ -63,11 +44,6 @@ public CXFJAXRSFilter(final CxfRsHttpListener delegate, final String[] welcomeFi
         }
     }
 
-    @Override
-    public void init(final FilterConfig filterConfig) throws ServletException {
-        mapping = filterConfig.getInitParameter("mapping");
-    }
-
     @Override
     public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException {
         if (!HttpServletRequest.class.isInstance(request)) {
@@ -83,18 +59,23 @@ public void doFilter(final ServletRequest request, final ServletResponse respons
             return;
         }
 
-        if (CxfRsHttpListener.TRY_STATIC_RESOURCES) { // else 100% JAXRS
-            if (servletMappingIsUnderRestPath(httpServletRequest)) {
-                chain.doFilter(request, response);
-                return;
-            }
-            final InputStream staticContent = delegate.findStaticContent(httpServletRequest, welcomeFiles);
-            if (staticContent != null) {
-                chain.doFilter(request, response);
-                return;
+        // if a servlet matched it always has priority over JAX-RS endpoints, see TOMEE-4406
+        if (nonDefaultServletMatches(httpServletRequest)) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        // Try to figure out if Tomcat DefaultServlet would handle this request
+        if (CxfRsHttpListener.TRY_STATIC_RESOURCES) {
+            try (final InputStream staticContent = delegate.findStaticContent(httpServletRequest, welcomeFiles)) {
+                if (staticContent != null) {
+                    chain.doFilter(request, response);
+                    return;
+                }
             }
         }
 
+        // else 100% JAXRS
         try {
             delegate.doInvoke(
                     new ServletRequestAdapter(httpServletRequest, httpServletResponse, request.getServletContext()),
@@ -104,63 +85,17 @@ public void doFilter(final ServletRequest request, final ServletResponse respons
         }
     }
 
-    private boolean servletMappingIsUnderRestPath(final HttpServletRequest request) {
-        final HttpServletRequest unwrapped = unwrap(request);
-        if (!RequestFacade.class.isInstance(unwrapped)) {
-            return false;
-        }
-
-        final Request tr;
-        try {
-            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("*")
-                            && mapping.startsWith(this.mapping)) {
-                        accept = true;
-                        break;
-                    }
-                }
-            } // else will be handed by getResourceAsStream()
-            mappingByServlet.putIfAbsent(wrapper, accept);
-            return accept;
-        }
-
-        return accept;
-    }
-
-    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;
+    /**
+     * Checks if the request matched a defined servlet mapping matches the given request
+     *
+     * @param request the HttpServletRequest to check
+     * @return if the servlet mapping matches and is not the tomcat DefaultServlet
+     */
+    private boolean nonDefaultServletMatches(final HttpServletRequest request) {
+        ServletRegistration servletRegistration = request.getServletContext().getServletRegistration(
+                request.getHttpServletMapping().getServletName());
+
+        return !"org.apache.catalina.servlets.DefaultServlet".equals(servletRegistration.getClassName());
     }
 
     @Override

tomee/tomee-jaxrs/src/main/java/org/apache/tomee/webservices/TomcatRsRegistry.java

@@ -123,7 +123,6 @@ public AddressInfo createRsHttpListener(final String appId, final String webCont
         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();