Skip to content
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.

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

RESTEasy reactive illegally restricts JAX-RSpath parameter names #25258

Closed
sithmein opened this issue Apr 29, 2022 · 4 comments · Fixed by #25285
Closed

RESTEasy reactive illegally restricts JAX-RSpath parameter names #25258

sithmein opened this issue Apr 29, 2022 · 4 comments · Fixed by #25285
Labels
area/resteasy-reactive kind/bug Something isn't working
Milestone

Comments

@sithmein
Copy link

Describe the bug

Consider the following JAX-RS resource:

@Produces(MediaType.APPLICATION_JSON)
@Path("/")
public class ContextNotWorking {
    @GET
    @Path("foo/{something-with-dash:[^a]*}")
	public Response doSomething(@PathParam("something-with-dash") String param) {
	    return Response.noContent().build();
	}
}

Note that path parameter something-with-dash. According to the Javadoc of @path the dash is an allowed character in a parameter name. However, if you try to run this with resteasy-reactive startup fails with

Caused by: java.lang.IllegalArgumentException: Path '/foo/{something-with-dash:[^a]*}' of method 'org.knime.xxx.ContextNotWorking#doSomething' is not a valid expression
	at org.jboss.resteasy.reactive.server.processor.ServerEndpointIndexer.validateMethodPath(ServerEndpointIndexer.java:226)
	at org.jboss.resteasy.reactive.server.processor.ServerEndpointIndexer.handleAdditionalMethodProcessing(ServerEndpointIndexer.java:218)
	at io.quarkus.resteasy.reactive.server.deployment.QuarkusServerEndpointIndexer.handleAdditionalMethodProcessing(QuarkusServerEndpointIndexer.java:164)
	at io.quarkus.resteasy.reactive.server.deployment.QuarkusServerEndpointIndexer.handleAdditionalMethodProcessing(QuarkusServerEndpointIndexer.java:32)
	at org.jboss.resteasy.reactive.common.processor.EndpointIndexer.createResourceMethod(EndpointIndexer.java:657)
	... 14 more
Caused by: java.util.regex.PatternSyntaxException: named capturing group is missing trailing '>' near index 12
(?<something-with-dash>[^a]*)$
            ^
	at java.base/java.util.regex.Pattern.error(Pattern.java:2027)
	at java.base/java.util.regex.Pattern.groupname(Pattern.java:2949)
	at java.base/java.util.regex.Pattern.group0(Pattern.java:2995)
	at java.base/java.util.regex.Pattern.sequence(Pattern.java:2123)
	at java.base/java.util.regex.Pattern.expr(Pattern.java:2068)
	at java.base/java.util.regex.Pattern.compile(Pattern.java:1782)
	at java.base/java.util.regex.Pattern.<init>(Pattern.java:1428)
	at java.base/java.util.regex.Pattern.compile(Pattern.java:1068)
	at org.jboss.resteasy.reactive.server.mapping.URITemplate.<init>(URITemplate.java:157)
	at org.jboss.resteasy.reactive.server.processor.ServerEndpointIndexer.validateMethodPath(ServerEndpointIndexer.java:223)
	... 18 more

It seems he reactive extension does not comply with the JAX-RS specification.
The same class works fine with the non-reactive version of RESTEasy.
Patterns in Java indeed do not allow dashes in capture group names.

Expected behavior

Code that is legal according to the JAX-RS specification should work in Quarkus.

Actual behavior

Quarkus startup fails with the error above.

How to Reproduce?

Try to run the provided JAX-RS resource class with quarkus-resteasy-reactive.

Output of uname -a or ver

No response

Output of java -version

17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.8.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@sithmein sithmein added the kind/bug Something isn't working label Apr 29, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 29, 2022

/cc @FroMage, @geoand, @stuartwdouglas

@gsmet
Copy link
Member

gsmet commented Apr 29, 2022

@geoand if you don't support regexp in paths, my guess is that you should use Pattern.quote().

@Postremus
Copy link
Member

Postremus commented Apr 29, 2022

The name of a group can only contain following characters:
A-Za-z0-9

Additionally, it may only start with:
A-Za-z

(See java.util.Pattern.groupName)

Lets compare this with the restrictions outlined in the javax.ws.rs.Path#value javadoc.

  • must start with {
  • may have zero to infinite whitespaces (space or tab)
  • name, consisting of
    ** one letter in the range A-Za-z0-9_
    ** zero to ininite letters in the range A-Za-z0-9_-.
  • after the name, zero to infinite whitespaces
  • optional regex
    ** : to start the regex
    ** Zero to infinite whitespace
    ** zero to infinite non brace characters, or zero to infinite a non brace character which are wrapped in braces
    ** Zero to infinite whitespace
  • ending with }

The full ABNF, just for reference:

param = "{" *WSP name *WSP [ ":" *WSP regex *WSP ] "}"
  name = (ALPHA / DIGIT / "_")*(ALPHA / DIGIT / "." / "_" / "-" ) ; \w[\w\.-]*
  regex = *( nonbrace / "{" *nonbrace "}" ) ; where nonbrace is any char other than "{" and "}"

The capturing group name is created in:

components.add(new TemplateComponent(Type.CUSTOM_REGEX, null, null, Pattern.compile(regexAggregator.toString()),
nameAggregator.toArray(new String[0])));

These lines create a simple regex pattern, not respecting that uri templates are more complex than group names allow for.

A solution would introduce some kind of normalization when creating the capture group, for example (not a fully tested solution, just as inspiration for now):
Postremus@9918aff

@geoand
Copy link
Contributor

geoand commented Apr 30, 2022

@Postremus mind opening a draft PR with that change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/resteasy-reactive kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants