From a284e8ee09e4e2ed03e6fb7695bb242a58752918 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Mon, 9 Nov 2026 13:31:01 +0100 Subject: [PATCH] Fix premature termination of incomplete chunked messages Prevent chunked transfer codecs from writing the terminating chunk when entity serialization fails. Preserve early response handling and close inconsistent connections after write failures. --- .../impl/io/DefaultBHttpClientConnection.java | 14 ++++- .../impl/io/DefaultBHttpServerConnection.java | 9 ++- .../io/TestDefaultBHttpClientConnection.java | 63 +++++++++++++++++++ .../io/TestDefaultBHttpServerConnection.java | 25 ++++++++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java index 3ad8691616..6fad738cf5 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpClientConnection.java @@ -61,6 +61,7 @@ import org.apache.hc.core5.http.io.HttpMessageWriterFactory; import org.apache.hc.core5.http.io.ResponseOutOfOrderStrategy; import org.apache.hc.core5.http.message.MessageSupport; +import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.util.Args; /** @@ -209,7 +210,7 @@ public void sendRequestEntity(final ClassicHttpRequest request) throws HttpExcep if (len == ContentLengthStrategy.UNDEFINED) { throw new LengthRequiredException(); } - try (final OutputStream outStream = createContentOutputStream( + final OutputStream outStream = createContentOutputStream( len, this.outbuffer, new OutputStream() { final OutputStream socketOutputStream = socketHolder.getOutputStream(); @@ -281,15 +282,22 @@ public void close() throws IOException { socketOutputStream.close(); } - }, entity.getTrailers())) { + }, entity.getTrailers()); + try { entity.writeTo(outStream); + outStream.close(); } catch (final ResponseOutOfOrderException ex) { - if (len > 0) { + if (len != 0) { this.consistent = false; } + } catch (final IOException | RuntimeException ex) { + this.consistent = false; + close(CloseMode.IMMEDIATE); + throw ex; } } + @Override public boolean isConsistent() { return this.consistent; diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java index ab674c7da5..d569b9d85b 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/DefaultBHttpServerConnection.java @@ -52,6 +52,7 @@ import org.apache.hc.core5.http.io.HttpMessageWriter; import org.apache.hc.core5.http.io.HttpMessageWriterFactory; import org.apache.hc.core5.http.io.HttpServerConnection; +import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.util.Args; /** @@ -193,8 +194,14 @@ public void sendResponseEntity(final ClassicHttpResponse response) return; } final long len = this.outgoingContentStrategy.determineLength(response); - try (final OutputStream outStream = createContentOutputStream(len, this.outbuffer, socketHolder.getOutputStream(), entity.getTrailers())) { + final OutputStream outStream = createContentOutputStream( + len, this.outbuffer, socketHolder.getOutputStream(), entity.getTrailers()); + try { entity.writeTo(outStream); + outStream.close(); + } catch (final IOException | RuntimeException ex) { + close(CloseMode.IMMEDIATE); + throw ex; } } } diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java index f967b7f917..3ed592e956 100644 --- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpClientConnection.java @@ -28,7 +28,9 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.EOFException; import java.io.InputStream; +import java.io.OutputStream; import java.net.Socket; import java.nio.charset.StandardCharsets; @@ -230,6 +232,67 @@ void testReadResponseNoEntity() throws Exception { Assertions.assertInstanceOf(IdentityInputStream.class, content); } + @Test + void testEarlyResponseDoesNotAbortConnection() throws Exception { + final ByteArrayOutputStream output = new ByteArrayOutputStream(); + final ByteArrayInputStream input = new ByteArrayInputStream( + "HTTP/1.1 400 Bad Request\r\nContent-Length: 0\r\n\r\n" + .getBytes(StandardCharsets.US_ASCII)); + Mockito.when(socket.getOutputStream()).thenReturn(output); + Mockito.when(socket.getInputStream()).thenReturn(input); + + final org.apache.hc.core5.http.io.ResponseOutOfOrderStrategy strategy = + Mockito.mock(org.apache.hc.core5.http.io.ResponseOutOfOrderStrategy.class); + Mockito.when(strategy.isEarlyResponseDetected( + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())) + .thenReturn(true); + + conn = new DefaultBHttpClientConnection( + Http1Config.DEFAULT, null, null, + DefaultContentLengthStrategy.INSTANCE, + DefaultContentLengthStrategy.INSTANCE, + strategy, + DefaultHttpRequestWriterFactory.INSTANCE, + DefaultHttpResponseParserFactory.INSTANCE); + conn.bind(socket); + + final ClassicHttpRequest request = new BasicClassicHttpRequest(Method.POST, "/stuff"); + request.addHeader("Transfer-Encoding", "chunked"); + request.setEntity(new ByteArrayEntity(new byte[16384], ContentType.TEXT_PLAIN, true)); + + conn.sendRequestHeader(request); + Assertions.assertDoesNotThrow(() -> conn.sendRequestEntity(request)); + Assertions.assertTrue(conn.isOpen()); + Assertions.assertFalse(conn.isConsistent()); + Assertions.assertEquals(400, conn.receiveResponseHeader().getCode()); + Assertions.assertFalse(new String(output.toByteArray(), StandardCharsets.US_ASCII) + .endsWith("0\r\n\r\n")); + } + + @Test + void testChunkedEntityFailureDoesNotTerminateMessage() throws Exception { + final ByteArrayOutputStream outStream = new ByteArrayOutputStream(); + Mockito.when(socket.getOutputStream()).thenReturn(outStream); + conn.bind(socket); + + final HttpEntity entity = Mockito.mock(HttpEntity.class); + Mockito.doAnswer(invocation -> { + final OutputStream out = invocation.getArgument(0); + out.write("123".getBytes(StandardCharsets.US_ASCII)); + throw new EOFException("source stream failed"); + }).when(entity).writeTo(Mockito.any(OutputStream.class)); + + final ClassicHttpRequest request = new BasicClassicHttpRequest(Method.POST, "/stuff"); + request.addHeader("Transfer-Encoding", "chunked"); + request.setEntity(entity); + conn.sendRequestHeader(request); + Assertions.assertThrows(EOFException.class, () -> conn.sendRequestEntity(request)); + Assertions.assertFalse(conn.isConsistent()); + Assertions.assertFalse(conn.isOpen()); + Assertions.assertFalse(new String(outStream.toByteArray(), StandardCharsets.US_ASCII) + .endsWith("0\r\n\r\n")); + } + @Test void testWriteRequestHead() throws Exception { final ByteArrayOutputStream outStream = new ByteArrayOutputStream(); diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpServerConnection.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpServerConnection.java index 7080c8bc0d..a4f3b7a6b3 100644 --- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpServerConnection.java +++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestDefaultBHttpServerConnection.java @@ -28,7 +28,9 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.EOFException; import java.io.InputStream; +import java.io.OutputStream; import java.net.Socket; import java.nio.charset.StandardCharsets; @@ -210,6 +212,29 @@ void testReadRequestNoEntity() throws Exception { Assertions.assertNull(entity); } + @Test + void testChunkedEntityFailureDoesNotTerminateMessage() throws Exception { + final ByteArrayOutputStream outStream = new ByteArrayOutputStream(); + Mockito.when(socket.getOutputStream()).thenReturn(outStream); + conn.bind(socket); + + final HttpEntity entity = Mockito.mock(HttpEntity.class); + Mockito.doAnswer(invocation -> { + final OutputStream out = invocation.getArgument(0); + out.write("123".getBytes(StandardCharsets.US_ASCII)); + throw new EOFException("source stream failed"); + }).when(entity).writeTo(Mockito.any(OutputStream.class)); + + final ClassicHttpResponse response = new BasicClassicHttpResponse(200, "OK"); + response.addHeader("Transfer-Encoding", "chunked"); + response.setEntity(entity); + conn.sendResponseHeader(response); + Assertions.assertThrows(EOFException.class, () -> conn.sendResponseEntity(response)); + Assertions.assertFalse(conn.isOpen()); + Assertions.assertFalse(new String(outStream.toByteArray(), StandardCharsets.US_ASCII) + .endsWith("0\r\n\r\n")); + } + @Test void testWriteResponseHead() throws Exception { final ByteArrayOutputStream outStream = new ByteArrayOutputStream();