eclipse-ee4j / eclipse-ee4j/jersey
Inconsistent handling of invalid and empty strings for numeric parameters
- Dominant language
- Java
- Stars
- 730
- Forks
- 382
- PR merge metrics
- No merged PRs in 30d
Description
This is a follow-up of the discussion with @jbescos in https://github.com/eclipse-ee4j/jersey/issues/4651#issuecomment-826080312.
The handling of invalid strings and empty strings is inconsistent for numeric query parameters.
Intuitively, I would expect the strings `""` (empty string) and `"not-a-number"` (invalid string when trying to parse a numeric value) to lead to the same result, but Jersey currently handles them differently.
This also leads to an unidiomatic use of the `Optional` container for which support was introduced just recently in Jersey 2.34.
The following example (using the Jersey testing framework) illustrates the issue:
```java
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
import org.junit.Test;
import javax.ws.rs.*;
import javax.ws.rs.core.*;
import java.util.Optional;
import static org.junit.Assert.assertEquals;
public class OptionalTest extends JerseyTest {
@Override
protected Application configure() {
return new ResourceConfig(TestResource.class);
}
@Test
public void testOptionalInteger() {
String paramMissing = target("/optionalInteger").request().get(String.class);
assertEquals("default", paramMissing);
String paramProvided = target("/optionalInteger").queryParam("i", 42).request().get(String.class);
assertEquals("i was 42", paramProvided);
Response paramInvalidResponse = target("/optionalInteger").queryParam("i", "not-a-number").request().get();
assertEquals(404, paramInvalidResponse.getStatus());
Response paramEmptyResponse = target("/optionalInteger").queryParam("i", "").request().get();
assertEquals(404, paramEmptyResponse.getStatus()); // 🛑 <-- Fails with response status: 200, response body: "null")
}
@Test
public void testInteger() {
String paramMissing = target("/integer").request().get(String.class);
assertEquals("null", paramMissing);
String paramProvided = target("/integer").queryParam("i", 42).request().get(String.class);
assertEquals("i was 42", paramProvided);
Response paramInvalidResponse = target("/integer").queryParam("i", "not-a-number").request().get();
assertEquals(404, paramInvalidResponse.getStatus());
Response paramEmptyResponse = target("/integer").queryParam("i", "").request().get();
assertEquals(404, paramEmptyResponse.getStatus()); // 🛑 <-- Fails with response status: 200, response body: "null")
}
@Test
public void testPrimitiveInt() {
String paramMissing = target("/int").request().get(String.class);
assertEquals("i was 0", paramMissing);
String paramProvided = target("/int").queryParam("i", 42).request().get(String.class);
assertEquals("i was 42", paramProvided);
Response paramInvalidResponse = target("/int").queryParam("i", "not-a-number").request().get();
assertEquals(404, paramInvalidResponse.getStatus());
String paramEmpty = target("/int").queryParam("i", "").request().get(String.class);
assertEquals("i was 0", paramEmpty);
}
@Produces(MediaType.TEXT_PLAIN)
@Path("/")
public static class TestResource {
@GET
@Path("/optionalInteger")
public String optionalInt(@QueryParam("i") Optional i) {
if (i == null) {
return "null";
}
return i.map(param -> "i was " + param).orElse("default");
}
@GET
@Path("/integer")
public String integer(@QueryParam("i") Integer i) {
if (i == null) {
return "null";
}
return "i was " + i;
}
@GET
@Path("/int")
public String primitiveInt(@QueryParam("i") int i) {
return "i was " + i;
}
}
}
```
Contributor guide
Assessment
This issue has not been assessed yet.