From 5884c2b124b43166f84f1d7ecca2870af61673cd Mon Sep 17 00:00:00 2001 From: Peer Bech Hansen Date: Tue, 16 Jul 2024 12:47:09 +0200 Subject: [PATCH 1/2] Fix off-by-one issue path handling in Observability recorder Fixes: #41854 --- .../ObservabilityIntegrationRecorder.java | 16 ++-------------- .../reactive/common/util/PathHelper.java | 17 ++++++++++++++++- .../core/ResteasyReactiveRequestContext.java | 16 ++-------------- .../opentelemetry/reactive/SecuredResource.java | 4 ++-- .../src/main/resources/application.properties | 2 +- .../reactive/OpenTelemetryReactiveTest.java | 8 ++++---- 6 files changed, 27 insertions(+), 36 deletions(-) diff --git a/extensions/resteasy-reactive/rest/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/observability/ObservabilityIntegrationRecorder.java b/extensions/resteasy-reactive/rest/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/observability/ObservabilityIntegrationRecorder.java index 3e550fa61c9e4..125c84eddf876 100644 --- a/extensions/resteasy-reactive/rest/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/observability/ObservabilityIntegrationRecorder.java +++ b/extensions/resteasy-reactive/rest/runtime/src/main/java/io/quarkus/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/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/util/PathHelper.java b/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/util/PathHelper.java index a9445e1ba587f..7920b2611f308 100644 --- a/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/util/PathHelper.java +++ b/independent-projects/resteasy-reactive/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/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java index 0066c10be3bde..79dcd392f79a9 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java @@ -43,6 +43,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.SimpleResourceInfo; import org.jboss.resteasy.reactive.server.core.multipart.FormData; @@ -443,20 +444,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()); } /** diff --git a/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/SecuredResource.java b/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/SecuredResource.java index 126184518bf61..fd0d2855e178f 100644 --- a/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/SecuredResource.java +++ b/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/SecuredResource.java @@ -5,11 +5,11 @@ import org.jboss.resteasy.reactive.RestPath; -@Path("secured") +@Path("{dummy}/secured") public class SecuredResource { @GET @Path("item/{value}") - public String get(@RestPath String value) { + public String get(@RestPath String dummy, @RestPath String value) { return "Received: " + value; } } diff --git a/integration-tests/opentelemetry-reactive/src/main/resources/application.properties b/integration-tests/opentelemetry-reactive/src/main/resources/application.properties index 0518dd1e503eb..26116d022023e 100644 --- a/integration-tests/opentelemetry-reactive/src/main/resources/application.properties +++ b/integration-tests/opentelemetry-reactive/src/main/resources/application.properties @@ -10,6 +10,6 @@ quarkus.security.users.embedded.users.stuart=writer quarkus.security.users.embedded.roles.scott=READER quarkus.security.users.embedded.roles.stuart=READER,WRITER quarkus.http.auth.permission.secured.policy=authenticated -quarkus.http.auth.permission.secured.paths=/secured/* +quarkus.http.auth.permission.secured.paths=/foo/secured/* quarkus.otel.security-events.enabled=true diff --git a/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveTest.java b/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveTest.java index 193ee7dfaa66b..757e4455e200e 100644 --- a/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveTest.java +++ b/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveTest.java @@ -248,26 +248,26 @@ void multipleUsingCombine() { @Test public void securedInvalidCredential() { - given().auth().preemptive().basic("scott", "reader2").when().get("/secured/item/something") + given().auth().preemptive().basic("scott", "reader2").when().get("/foo/secured/item/something") .then() .statusCode(401); await().atMost(5, SECONDS).until(() -> getSpans().size() == 1); assertThat(getSpans()).singleElement().satisfies(m -> { - assertThat(m).extractingByKey("name").isEqualTo("GET /secured/item/{value}"); + assertThat(m).extractingByKey("name").isEqualTo("GET /{dummy}/secured/item/{value}"); assertEvent(m, SecurityEventUtil.AUTHN_FAILURE_EVENT_NAME); }); } @Test public void securedProperCredentials() { - given().auth().preemptive().basic("scott", "reader").when().get("/secured/item/something") + given().auth().preemptive().basic("scott", "reader").when().get("/foo/secured/item/something") .then() .statusCode(200); await().atMost(5, SECONDS).until(() -> getSpans().size() == 1); assertThat(getSpans()).singleElement().satisfies(m -> { - assertThat(m).extractingByKey("name").isEqualTo("GET /secured/item/{value}"); + assertThat(m).extractingByKey("name").isEqualTo("GET /{dummy}/secured/item/{value}"); assertEvent(m, SecurityEventUtil.AUTHN_SUCCESS_EVENT_NAME, SecurityEventUtil.AUTHZ_SUCCESS_EVENT_NAME); }); } From 04def9c1246f10475cb29590407fde805dc87d5d Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Thu, 29 Aug 2024 12:25:35 +0300 Subject: [PATCH 2/2] Remove useless stacktrace in OTel tests --- .../quarkus/it/opentelemetry/reactive/ReactiveResource.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/ReactiveResource.java b/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/ReactiveResource.java index f8e4c576301c1..da526e9182f19 100644 --- a/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/ReactiveResource.java +++ b/integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/ReactiveResource.java @@ -17,6 +17,7 @@ import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.instrumentation.annotations.WithSpan; import io.smallrye.mutiny.Uni; +import io.vertx.core.impl.NoStackTraceException; @Path("/reactive") public class ReactiveResource { @@ -82,14 +83,14 @@ public Uni helloPost(String body) { @Path("blockingException") @GET public String blockingException() { - throw new RuntimeException("dummy"); + throw new NoStackTraceException("dummy"); } @Path("reactiveException") @GET public Uni reactiveException() { return Uni.createFrom().item(() -> { - throw new RuntimeException("dummy2"); + throw new NoStackTraceException("dummy2"); }); } }