Fix off-by-one issue caused by ObservabilityIntegrationRecorder using…

… its own method for getting path without prefix
quarkusio · Jul 16, 2024 · a669db3 · a669db3
1 parent 7f3bd55
commit a669db3
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 29 deletions.
diff --git a/extensions/resteasy-reactive/rest/runtime/pom.xml b/extensions/resteasy-reactive/rest/runtime/pom.xml
@@ -41,6 +41,11 @@
             <artifactId>junit-jupiter</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-jsonp</artifactId>

diff --git a/...rkus/resteasy/reactive/server/runtime/observability/ObservabilityIntegrationRecorder.java b/...rkus/resteasy/reactive/server/runtime/observability/ObservabilityIntegrationRecorder.java
@@ -5,6 +5,7 @@
 import jakarta.ws.rs.HttpMethod;
 
 import org.jboss.logging.Logger;
+import org.jboss.resteasy.reactive.common.util.PathHelper;
 import org.jboss.resteasy.reactive.server.core.Deployment;
 import org.jboss.resteasy.reactive.server.handlers.ClassRoutingHandler;
 import org.jboss.resteasy.reactive.server.mapping.RequestMapper;
@@ -112,20 +113,7 @@ public static void setTemplatePath(RoutingContext rc, Deployment deployment) {
         setUrlPathTemplate(rc, templatePath);
     }
 
-    private static String getPath(RoutingContext rc) {
-        return rc.normalizedPath();
-    }
-
     private static String getPathWithoutPrefix(RoutingContext rc, Deployment deployment) {
-        String path = getPath(rc);
-        if (path != null) {
-            String prefix = deployment.getPrefix();
-            if (!prefix.isEmpty()) {
-                if (path.startsWith(prefix)) {
-                    return path.substring(prefix.length());
-                }
-            }
-        }
-        return path;
+        return PathHelper.getPathWithoutPrefix(rc.normalizedPath(), deployment.getPrefix());
     }
 }
diff --git a/...va/io/quarkus/resteasy/reactive/runtime/mapping/ObservabilityIntegrationRecorderTest.java b/...va/io/quarkus/resteasy/reactive/runtime/mapping/ObservabilityIntegrationRecorderTest.java
@@ -0,0 +1,50 @@
+package io.quarkus.resteasy.reactive.runtime.mapping;
+
+import io.quarkus.resteasy.reactive.server.runtime.observability.ObservabilityIntegrationRecorder;
+import io.vertx.ext.web.RoutingContext;
+import org.jboss.resteasy.reactive.server.core.Deployment;
+import org.jboss.resteasy.reactive.server.handlers.RestInitialHandler;
+import org.jboss.resteasy.reactive.server.mapping.RequestMapper;
+import org.jboss.resteasy.reactive.server.mapping.URITemplate;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class ObservabilityIntegrationRecorderTest {
+
+    private static RoutingContext rc;
+    private static Deployment deployment;
+
+    @BeforeAll
+    public static void setup() {
+        rc = mock(RoutingContext.class);
+        deployment = mock(Deployment.class);
+        when(rc.normalizedPath()).thenReturn("/foo");
+        when(deployment.getClassMappers()).thenReturn(getRequestPaths());
+    }
+
+    @Test
+    public void testSetTemplatePathWithoutPathPrefixDoesNotThrow() {
+        when(deployment.getPrefix()).thenReturn("");
+        assertDoesNotThrow(() -> ObservabilityIntegrationRecorder.setTemplatePath(rc, deployment), "Should not throw exception when prefix is not set");
+    }
+
+    @Test
+    public void testSetTemplatePathWithPathPrefixDoesNotThrow() {
+        when(deployment.getPrefix()).thenReturn("/foo");
+        assertDoesNotThrow(() -> ObservabilityIntegrationRecorder.setTemplatePath(rc, deployment), "Should not throw exception when prefix is set");
+    }
+
+    private static ArrayList<RequestMapper.RequestPath<RestInitialHandler.InitialMatch>> getRequestPaths() {
+        RestInitialHandler.InitialMatch initialMatch = mock(RestInitialHandler.InitialMatch.class);
+        ArrayList<RequestMapper.RequestPath<RestInitialHandler.InitialMatch>> classMappers = new ArrayList<>();
+        classMappers.add(new RequestMapper.RequestPath<>(true, new URITemplate("/", true), initialMatch));
+        classMappers.add(new RequestMapper.RequestPath<>(true, new URITemplate("{param}/bar", true), initialMatch));
+        return classMappers;
+    }
+}
diff --git a/...tive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/util/PathHelper.java b/...tive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/util/PathHelper.java
@@ -7,7 +7,7 @@
 
 /**
  * A utility class for handling URI template parameters. As the Java
- * regulare expressions package does not handle named groups, this
+ * regular expressions package does not handle named groups, this
  * class attempts to simulate that functionality by using groups.
  *
  * @author Ryan J. McDonough
@@ -99,4 +99,19 @@ public static String recoverEnclosedCurlyBraces(String str) {
         return str.replace(openCurlyReplacement, '{').replace(closeCurlyReplacement, '}');
     }
 
+    public static String getPathWithoutPrefix(String path, String prefix) {
+        if (path != null) {
+            if (!prefix.isEmpty()) {
+                // FIXME: can we really have paths that don't start with the prefix if there's a prefix?
+                if (path.startsWith(prefix)) {
+                    if (path.length() == prefix.length()) {
+                        return "/";
+                    }
+                    return path.substring(prefix.length());
+                }
+            }
+        }
+        return path;
+    }
+
 }
diff --git a/...src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java b/...src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java
@@ -32,6 +32,7 @@
 import org.jboss.resteasy.reactive.common.NotImplementedYet;
 import org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext;
 import org.jboss.resteasy.reactive.common.util.Encode;
+import org.jboss.resteasy.reactive.common.util.PathHelper;
 import org.jboss.resteasy.reactive.common.util.PathSegmentImpl;
 import org.jboss.resteasy.reactive.server.core.multipart.FormData;
 import org.jboss.resteasy.reactive.server.core.serialization.EntityWriter;
@@ -429,20 +430,7 @@ public String getRemaining() {
      * Returns the normalised non-decoded path excluding any prefix.
      */
     public String getPathWithoutPrefix() {
-        String path = getPath();
-        if (path != null) {
-            String prefix = deployment.getPrefix();
-            if (!prefix.isEmpty()) {
-                // FIXME: can we really have paths that don't start with the prefix if there's a prefix?
-                if (path.startsWith(prefix)) {
-                    if (path.length() == prefix.length()) {
-                        return "/";
-                    }
-                    return path.substring(prefix.length());
-                }
-            }
-        }
-        return path;
+        return PathHelper.getPathWithoutPrefix(getPath(), deployment.getPrefix());
     }
 
     /**