eclipse-ee4j / eclipse-ee4j/jersey
ApacheConnector: closing ClientResponse throws if not consumed
- Dominant language
- Java
- Stars
- 730
- Forks
- 382
- PR merge metrics
- No merged PRs in 30d
Description
#3124 seems to have introduced a bug, with response.close() throwing an unexpected ProcessingException if the response has not been consumed. That is quite unfortunate because this ProcessingException overrides any original exception if close is called inside a finally-block. (And try-with-resources is not yet supported by JAX-RS 2.0).
```
javax.ws.rs.ProcessingException: Error closing message content input stream.
at org.glassfish.jersey.message.internal.EntityInputStream.close(EntityInputStream.java:161)
at org.glassfish.jersey.message.internal.InboundMessageContext$EntityContent.close(InboundMessageContext.java:156)
at org.glassfish.jersey.message.internal.InboundMessageContext.close(InboundMessageContext.java:939)
at org.glassfish.jersey.client.InboundJaxrsResponse.close(InboundJaxrsResponse.java:167)
Caused by: org.apache.http.ConnectionClosedException: Premature end of Content-Length delimited message body (expected: 4; received: 0
at org.apache.http.impl.io.ContentLengthInputStream.read(ContentLengthInputStream.java:178)
at org.apache.http.impl.io.ContentLengthInputStream.read(ContentLengthInputStream.java:198)
at org.apache.http.impl.io.ContentLengthInputStream.close(ContentLengthInputStream.java:101)
at org.apache.http.impl.execchain.ResponseEntityProxy.streamClosed(ResponseEntityProxy.java:140)
at org.apache.http.conn.EofSensorInputStream.checkClose(EofSensorInputStream.java:228)
at org.apache.http.conn.EofSensorInputStream.close(EofSensorInputStream.java:174)
at java.io.BufferedInputStream.close(BufferedInputStream.java:483)
at java.io.FilterInputStream.close(FilterInputStream.java:181)
at org.glassfish.jersey.apache.connector.ApacheConnector$3.close(ApacheConnector.java:644)
at java.io.FilterInputStream.close(FilterInputStream.java:181)
at org.glassfish.jersey.apache.connector.ApacheConnector$HttpClientResponseInputStream.close(ApacheConnector.java:621)
at org.glassfish.jersey.message.internal.EntityInputStream.close(EntityInputStream.java:158)
... 29 more
```
Seems to affect Jersey 2.22.2 upwards; experienced it in jersey 2.23.1
```
[INFO] +- org.glassfish.jersey.connectors:jersey-apache-connector:jar:2.23.1:test
[INFO] | +- org.apache.httpcomponents:httpclient:jar:4.5.2:test (version managed from 4.5)
[INFO] | | +- org.apache.httpcomponents:httpcore:jar:4.4.5:test (version managed from 4.4.4)
[INFO] | | \- commons-codec:commons-codec:jar:1.10:test (version managed from 1.9)
[INFO] | +- (org.glassfish.jersey.core:jersey-client:jar:2.23.1:test - omitted for duplicate)
[INFO] | \- (javax.ws.rs:javax.ws.rs-api:jar:2.0.1:test - omitted for duplicate)
```
Reproducable by this UnitTest:
```
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.ProcessingException;
import javax.ws.rs.Produces;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.glassfish.jersey.apache.connector.ApacheConnectorProvider;
import org.glassfish.jersey.client.ClientConfig;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
import org.glassfish.jersey.test.jdkhttp.JdkHttpServerTestContainerFactory;
import org.glassfish.jersey.test.spi.TestContainerException;
import org.glassfish.jersey.test.spi.TestContainerFactory;
import org.junit.Test;
public class FunctionalCloseableItTest extends JerseyTest {
@Override
protected Application configure() {
ResourceConfig config = new ResourceConfig();
config.register(TestResource.class);
return config;
}
@Override
protected void configureClient(ClientConfig config) {
config.connectorProvider(new ApacheConnectorProvider());
}
@Override
protected TestContainerFactory getTestContainerFactory() throws TestContainerException {
return new JdkHttpServerTestContainerFactory();
}
@Test
public void testProcessingExceptionThrownInCloseOverridesMyException() throws Exception {
try {
Response response = target().path("productInfo")
.request(MediaType.TEXT_PLAIN_TYPE)
.get();
try {
assertEquals(200, response.getStatus());
// this exception is unfortunately overridden in finally-block
throw new MyException();
} finally {
// BUG: will throw because not consumed
response.close();
}
} catch (ProcessingException e) {
e.printStackTrace();
assertEquals("Error closing message content input stream.", e.getMessage());
assertEquals(0, e.getSuppressed().length);
}
}
@SuppressWarnings("serial")
private static class MyException extends Exception {
}
@Path("/")
public static class TestResource {
@GET
@Path("/productInfo")
@Produces(MediaType.TEXT_PLAIN)
public String getProductInfo() {
return "foo\n";
}
}
}
```
The thrown exception seems like a bug to me because #3124 fully intended to prematurely close the connection instead of consuming it. An Exception would not make sense. There is also the javadoc of Response#close():
> The close() method should be invoked on all instances that
> contain an un-consumed entity input stream to ensure the resources associated
> with the instance are properly cleaned-up and prevent potential memory leaks.
> This is typical for client-side scenarios where application layer code
> processes only the response headers and ignores the response entity.
The bug seems to be caused by Jersey closing both HttpResponse and response-Stream. First closing the HttpResponse closes the socket-connection. The second close of the response-stream tries to consume the remaining body, but the socket is already closed, which is considered a protocol error by apache httpclient. Not closing the stream probably circumvents the exception.
I may prepare a PullRequest if i get the OK from my higher-ups to sign OCA (real PITA).
Contributor guide
Assessment
This issue has not been assessed yet.