eclipse-ee4j / eclipse-ee4j/jersey

Inconsistent handling of invalid and empty strings for numeric parameters

Open
#4,790 20 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.