JavaNetHttpConnector behavior conflicts with TerminalReaderInterceptor.aroundReadFrom(ReaderInterceptorContext)'s isEmpty() call

[oramattkosem]

Environment:
Jersey 3.1
JDK17
JavaNetHttpConnector

Scenario:
Use JerseyClient to perform a call against an endpoint that returns no response (void on the server side, returning 204).

Expected result:
The client should successfully identify that the call has no input and should not fail with an error like this:
org.glassfish.jersey.message.internal.MessageBodyProviderNotFoundException: MessageBodyReader not found for media type=application/octet-stream, type=void, genericType=void.

Actual result:
When TerminalReaderInterceptor makes its call to org.glassfish.jersey.message.internal.EntityInputStream.isEmpty(), the result is false. This occurs because this call within EntityInputStream receives a reply of 1 from the underlying InputStream during this call:

                   if (in.available() > 0) {
                        return false;
                    } 

This occurs because the JDK code in jdk.internal.net.http.ResponseSubscribers.HttpResponseInputStream.available() returns a value of 1.

        @Override
        public int available() throws IOException {
            // best effort: returns the number of remaining bytes in
            // the current buffer if any, or 1 if the current buffer
            // is null or empty but the queue or current buffer list
            // are not empty. Returns 0 otherwise.
            if (closed) return 0;
            int available = 0;
            ByteBuffer current = currentBuffer;
            if (current == LAST_BUFFER) return 0;
            if (current != null) available = current.remaining();
            if (available != 0) return available;
            Iterator<?> iterator = currentListItr;
            if (iterator != null && iterator.hasNext()) return 1;
            if (buffers.isEmpty()) return 0;
            return 1;
        }

This happens, even though there truly is no response, because the buffers queue contains a single ByteBuffer with no capacity:

The Javadocs for the InputStream.available() method do suggest it to be an "estimate" so it is not clear if this behavior is necessarily a "bug" in the JDK, nevertheless Jersey is unable to obtain a message body provider and calls like this fail.

[jansupol]

We would need a reproducer. We have a test case that passes just fine.

[oramattkosem]

Can do. The test case will pass if run on JDK11. In JDK17 it fails as indicated above, even if just driving the client directly, because available() returns 1.

[oramattkosem]

@jansupol the existing test is sufficient for reproducing the scenario with the following additions.

The missing pieces in the current suite are:

1. An endpoint that returns success
2. A get() call requesting a void or Void result

diff --git a/connectors/jnh-connector/src/test/java/org/glassfish/jersey/jnh/connector/NoEntityTest.java b/connectors/jnh-connector/src/test/java/org/glassfish/jersey/jnh/connector/NoEntityTest.java
index 0954072..b042ef4 100644
--- a/connectors/jnh-connector/src/test/java/org/glassfish/jersey/jnh/connector/NoEntityTest.java
+++ b/connectors/jnh-connector/src/test/java/org/glassfish/jersey/jnh/connector/NoEntityTest.java
@@ -28,6 +28,7 @@
 import jakarta.ws.rs.Path;
 import jakarta.ws.rs.client.WebTarget;
 import jakarta.ws.rs.core.Application;
+import jakarta.ws.rs.core.GenericType;
 import jakarta.ws.rs.core.Response;
 import org.junit.jupiter.api.Test;
 
@@ -40,6 +41,12 @@
         public Response get() {
             return Response.status(Response.Status.CONFLICT).build();
         }
+        
+        @GET
+        @Path("/success")
+        public Response getSuccessfully() {
+          return Response.status(Response.Status.NO_CONTENT).build();
+        }
 
         @POST
         public void post(String entity) {
@@ -69,6 +76,22 @@
     }
 
     @Test
+    public void testGetVoid() {
+      WebTarget r = target("test/success");
+      for (int i = 0; i < 5; i++ ) {
+        r.request().get(void.class);
+      }
+    }
+
+    @Test
+    public void testGetGenericVoid() {
+      WebTarget r = target("test/success");
+      for (int i = 0; i < 5; i++ ) {
+        r.request().get(new GenericType<Void>() {});
+      }
+    }
+
+    @Test
     public void testGetWithClose() {
         WebTarget r = target("test");
         for (int i = 0; i < 5; i++) {

[jansupol]

This is a bit awkward scenario. The Invocation.Builder#get(void.class) expects a return type Void, but the only Void one can create is null. So you send a GET request requiring the response to be null? I can understand it for 204, but still, this construct won't even allow you to check the response status whether it went as expected.

Do I assume correctly this "Void" class is a result of some automatism that puts the argument into the get method so that you cannot use get() without an argument? If so, I'd suggest using Response.class instead of Void.class.

[oramattkosem]

It's a common pattern. Jersey's provided org.glassfish.jersey.client.proxy.WebResourceFactory and org.glassfish.jersey.microprofile.restclient.RestClientBuilderImpl impls of "type safe APIs" both do this.

In cases like this, you can determine if the response is as expected by noting the lack of exceptions. When using this pattern, successful replies return WebApplicationException, which can be used to obtain the actual response.

[jansupol]

Sorry, no. You can get 404, but that would not cause WebApplicationException to be thrown.

You argue with WebResourceFactory and RestClientBuilder but both are proxies, they do not have get(void.class) methods. I do not see any relevance to this.

[oramattkosem]

That's not accurate. If you follow this path, a javax.ws.rs.NotFoundException (a subclass of javax.ws.rs.WebApplicationException) is thrown. This is all covered in the Javadocs for the get() methods which return a result type.

Both of those implementations create proxies of whatever API you provide. If the API has methods returning void or Void, a call effectively comparable to get(void.class)/get(Void.class) is made. It may end up using method in practice, one performs readEntity as a separate step, and one has some special handling for Void but not void, but the result is the same across the board for anything building a proxy for an API with methods returning void. ([1], [2])

The relevance of this, in my case, is that I use both of them and both are affected by this behavior since they can create proxies for interfaces with void returns that trigger this behavior. In short, that's specifically where I found it. I'm not sure why the MP variant has special handling for Void.class, since it's pretty uncommon for methods to use Void over void as a return in practice, but both of them produce the same behavior when proxying an interface method that returns void.

[oramattkosem]

For what it's worth, this misbehavior isn't specific to just the void/Void use case. Any other object type tied to a 204 response produces this behavior as well. This exception ends up getting thrown where any 204/null scenario is found.