Skip to content

Fix handling of overlapping paths in resteasy-reactive server #50384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: 3.28
Choose a base branch
from
+90 −16
Draft

Fix handling of overlapping paths in resteasy-reactive server #50384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a38fa7
Improve path matching in RESTEasy Reactive for ambiguous path templates
TomasHofman Oct 1, 2025
f32604a
Improve path matching in RESTEasy Reactive for ambiguous path templates
TomasHofman Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions .../server/runtime/src/main/java/org/jboss/resteasy/reactive/server/mapping/PathMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.jboss.resteasy.reactive.server.mapping;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -34,24 +36,28 @@ class PathMatcher<T> implements Dumpable {
* @param path The relative path to match
* @return The match match. This will never be null, however if none matched its value field will be
*/
PathMatch<T> match(String path) {
List<PathMatch<T>> match(String path) {
int length = path.length();
final int[] lengths = this.lengths;
ArrayList<PathMatch<T>> matches = new ArrayList<>();
for (int i = 0; i < lengths.length; ++i) {
int pathLength = lengths[i];
if (pathLength == length) {
SubstringMap.SubstringMatch<T> next = paths.get(path, length);
if (next != null) {
return new PathMatch<>(path, "", next.getValue());
matches.add(new PathMatch<>(path, "", next.getValue()));
}
} else if (pathLength < length) {
SubstringMap.SubstringMatch<T> next = paths.get(path, pathLength);
if (next != null) {
return new PathMatch<>(next.getKey(), path.substring(pathLength), next.getValue());
matches.add(new PathMatch<>(next.getKey(), path.substring(pathLength), next.getValue()));
}
}
}
return defaultMatch(path);
if (!matches.isEmpty()) {
return matches;
}
return Collections.singletonList(defaultMatch(path));
}

PathMatch<T> defaultMatch(String path) {
Expand Down
31 changes: 20 additions & 11 deletions ...erver/runtime/src/main/java/org/jboss/resteasy/reactive/server/mapping/RequestMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -48,9 +49,12 @@ public void accept(String stem, ArrayList<RequestPath<T>> list) {
* @return best RequestMatch, or null if the path has no match
*/
public RequestMatch<T> map(String path) {
var result = mapFromPathMatcher(path, requestPaths.match(path), 0);
if (result != null) {
return result;
List<PathMatcher.PathMatch<ArrayList<RequestPath<T>>>> matches = requestPaths.match(path);
for (PathMatcher.PathMatch<ArrayList<RequestPath<T>>> match : matches) {
var result = mapFromPathMatcher(path, match, 0);
if (result != null) {
return result;
}
}

// the following code is meant to handle cases like https://github.com/quarkusio/quarkus/issues/30667
Expand All @@ -68,16 +72,21 @@ public RequestMatch<T> continueMatching(String path, RequestMatch<T> lastMatch)
return null;
}

var initialMatches = requestPaths.match(path);
var result = mapFromPathMatcher(path, initialMatches, 0);
if (result != null) {
int idx = nextMatchStartingIndex(initialMatches, lastMatch);
return mapFromPathMatcher(path, initialMatches, idx);
var initialMatchesList = requestPaths.match(path);
for (var initialMatches : initialMatchesList) {
var result = mapFromPathMatcher(path, initialMatches, 0);
if (result != null) {
int idx = nextMatchStartingIndex(initialMatches, lastMatch);
RequestMatch<T> match = mapFromPathMatcher(path, initialMatches, idx);
if (match != null) {
return match;
}
}
}

// the following code is meant to handle cases like https://github.com/quarkusio/quarkus/issues/30667
initialMatches = requestPaths.defaultMatch(path);
result = mapFromPathMatcher(path, initialMatches, 0);
var initialMatches = requestPaths.defaultMatch(path);
var result = mapFromPathMatcher(path, initialMatches, 0);
if (result != null) {
int idx = nextMatchStartingIndex(initialMatches, lastMatch);
return mapFromPathMatcher(path, initialMatches, idx);
Expand All @@ -101,7 +110,7 @@ private int nextMatchStartingIndex(PathMatcher.PathMatch<ArrayList<RequestPath<T
}
}

return -1;
return 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have added a comment here... This last return statement represents a case when the current match is not found in the list of new initial matches. In which case we want to search those new matches from index 0. The -1 would mean that no more matches will be tested.

}

@SuppressWarnings({ "rawtypes", "unchecked" })
Expand Down
61 changes: 60 additions & 1 deletion ...est/java/org/jboss/resteasy/reactive/server/vertx/test/matching/PathParamOverlapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class PathParamOverlapTest {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(TestResource.class);
.addClasses(TestResource.class, AnotherResource.class);
}
});

Expand Down Expand Up @@ -53,6 +53,40 @@ public void test() {
get("/hello/other/test/wrong")
.then()
.statusCode(404);

get("/hello/foo")
.then()
.statusCode(404);

get("/hello/foo/value")
.then()
.statusCode(200)
.body(equalTo("Foo value"));

get("/hello/foo/bar")
.then()
.statusCode(200)
.body(equalTo("Foo bar"));

get("/hello/foo/bar/value")
.then()
.statusCode(200)
.body(equalTo("FooBar value"));

get("/hello/foo/bah_value")
.then()
.statusCode(200)
.body(equalTo("Foo bah_value"));

get("/hello/foo/bar_value")
.then()
.statusCode(200)
.body(equalTo("Foo bar_value"));

get("/hello/foo/bar/foo/bar")
.then()
.statusCode(200)
.body(equalTo("FooBarFooBar"));
}

@Path("/hello")
Expand All @@ -70,5 +104,30 @@ public String test() {
public String second(@RestPath String id) {
return "Hello " + id;
}

@GET
@Path("/foo/{param}")
public String foo(@RestPath String param) {
return "Foo " + param;
}

@GET
@Path("/foo/bar/{param}")
public String fooBar(@RestPath String param) {
return "FooBar " + param;
}

}

/**
* This is to also test paths spread over multiple resource classes.
*/
@Path("/hello/foo/bar")
public static class AnotherResource {
@GET
@Path("/foo/bar")
public String fooBarFooBar() {
return "FooBarFooBar";
}
}
}