-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Properly handle char and Character parameter types in JAX-RS methods #24549
Conversation
...a/org/jboss/resteasy/reactive/server/core/parameters/converters/CharacterParamConverter.java
Show resolved
Hide resolved
...n/java/org/jboss/resteasy/reactive/server/core/parameters/converters/CharParamConverter.java
Show resolved
Hide resolved
@@ -75,8 +75,10 @@ public String get(@PathParam("id") String id) { | |||
public String params(@PathParam("p") String p, | |||
@QueryParam("q") String q, | |||
@HeaderParam("h") int h, | |||
@HeaderParam("h2") char h2, | |||
@HeaderParam("h3") Character h3, | |||
@FormParam("f") String f) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the other primitive types work, because I have tests for them, but I'm surprised that we don't here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean boolean
, byte
, short
, long
, float
, double
and all that junk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add them, but there is not much point I would say due the existence of valueOf
(which of course does not exist for char)
@gsmet This would be great to backport, it's preventing me from releasing the Renarde extension because I test for this, and it doesn't compile natively without this. |
The label is present, so it will be backported 😉 |
Curious though, what Renarde feature needs a char? |
It doesn't, but it's an existing test, so CI can't pass without this, or I need to disable the test. |
Fixes: #24541