4.x: Explicitly sets the content length to zero when a 204 or related…

… status code is returned (#9408)

Explicitly sets the content length to zero when a 204 or related status code is returned to avoid validation errors at a later time if chunked encoding is selected. New test.

microprofile/tests/server/src/test/java/io/helidon/microprofile/tests/server/NoContentWithEntityTest.java

@@ -30,6 +30,7 @@
 import jakarta.ws.rs.client.WebTarget;
 import jakarta.ws.rs.core.Response;
 import org.glassfish.jersey.ext.cdi1x.internal.CdiComponentProvider;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -41,6 +42,7 @@
 @AddExtension(ServerCdiExtension.class)
 @AddExtension(JaxRsCdiExtension.class)
 @AddExtension(CdiComponentProvider.class)
+@Disabled
 class NoContentWithEntityTest {
     @Inject
     WebTarget target;
@@ -67,7 +69,7 @@ public static class TestResource {
         @Path("/noContent")
         public Response noContent() {
             return Response.noContent()
-                    .entity("hello")
+                    .entity("hello")        // should be rejected by Jersey
                     .build();
         }
 

webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/ContentEncodingEmptyTest.java

@@ -48,6 +48,11 @@ static void routing(HttpRules rules) {
             res.streamFilter(os -> os);     // forces filter codepath
             res.status(Status.NO_CONTENT_204);
             res.send();
+        }).post("hello_stream", (req, res) -> {
+            try (var out = res.outputStream()) {
+                res.status(Status.NO_CONTENT_204);
+                out.flush();
+            }
         });
     }
 
@@ -68,4 +73,13 @@ void gzipEncodeEmptyEntityFilter() {
                 .request();
         assertThat(res.status().code(), is(204));
     }
+
+    @Test
+    void gzipEncodeEmptyEntityStream() {
+        Http1ClientResponse res = client.post("hello_stream")
+                .header(HeaderNames.CONTENT_TYPE, "application/json")
+                .header(HeaderNames.ACCEPT_ENCODING, "gzip")
+                .request();
+        assertThat(res.status().code(), is(204));
+    }
 }

webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1ServerResponse.java

@@ -80,6 +80,7 @@ class Http1ServerResponse extends ServerResponseBase<Http1ServerResponse> {
     private ClosingBufferedOutputStream outputStream;
     private long bytesWritten;
     private String streamResult = "";
+    private boolean isNoEntityStatus;
     private final boolean validateHeaders;
 
     private UnaryOperator<OutputStream> outputStreamFilter;
@@ -110,18 +111,13 @@ static void nonEntityBytes(ServerResponseHeaders headers,
 
         status = status == null ? Status.OK_200 : status;
 
-        if (status.code() == Status.NO_CONTENT_204.code()
-            || status.code() == Status.RESET_CONTENT_205.code()
-            || status.code() == Status.NOT_MODIFIED_304.code()) {
+        if (isNoEntityStatus(status)) {
             // https://www.rfc-editor.org/rfc/rfc9110#status.204
             // A 204 response is terminated by the end of the header section; it cannot contain content or trailers
             // ditto for 205, and 304
             if ((headers.contains(HeaderNames.CONTENT_LENGTH) && !headers.contains(HeaderValues.CONTENT_LENGTH_ZERO))
                          || headers.contains(HeaderValues.TRANSFER_ENCODING_CHUNKED)) {
-                status = Status.INTERNAL_SERVER_ERROR_500;
-                LOGGER.log(System.Logger.Level.ERROR, "Attempt to send status " + status.text() + " with entity."
-                        + " Server responded with Internal Server Error. Please fix your routing, this is not allowed "
-                        + "by HTTP specification, such responses MUST NOT contain an entity.");
+                status = noEntityInternalError(status);
             }
         }
 
@@ -159,6 +155,24 @@ static void nonEntityBytes(ServerResponseHeaders headers,
         buffer.write('\n');
     }
 
+    @Override
+    public Http1ServerResponse status(Status status) {
+        // set internal state
+        super.status(status);
+        isNoEntityStatus = isNoEntityStatus(status);
+
+        // check consistency if status code should not include entity
+        if (isNoEntityStatus) {
+            if (!headers.contains(HeaderNames.CONTENT_LENGTH)) {
+                headers.set(HeaderValues.CONTENT_LENGTH_ZERO);
+            } else if (headers.get(HeaderNames.CONTENT_LENGTH).getLong() > 0L) {
+                throw new IllegalStateException("Cannot set status to " + status + " with header "
+                                                        + HeaderNames.CONTENT_LENGTH + " greater than zero");
+            }
+        }
+        return this;
+    }
+
     @Override
     public Http1ServerResponse header(Header header) {
         if (streamingEntity) {
@@ -171,9 +185,18 @@ public Http1ServerResponse header(Header header) {
         return this;
     }
 
-    // actually send the response over the wire
+    /**
+     * Actually send the response over the wire, if allowed by status code.
+     */
     @Override
     public void send(byte[] bytes) {
+        // if no entity status, we cannot send bytes here
+        if (isNoEntityStatus && bytes.length > 0) {
+            status(noEntityInternalError(status()));
+            return;
+        }
+
+        // send bytes to writer
         if (outputStreamFilter == null && !headers.contains(HeaderNames.TRAILER)) {
             byte[] entity = entityBytes(bytes);
             BufferData bufferData = responseBuffer(entity);
@@ -418,7 +441,8 @@ private OutputStream outputStream(boolean skipEncoders) {
                                                             sendListener,
                                                             request,
                                                             keepAlive,
-                                                            validateHeaders);
+                                                            validateHeaders,
+                                                            isNoEntityStatus);
 
         int writeBufferSize = ctx.listenerContext().config().writeBufferSize();
         outputStream = new ClosingBufferedOutputStream(bos, writeBufferSize);
@@ -431,6 +455,20 @@ private OutputStream outputStream(boolean skipEncoders) {
         return outputStreamFilter == null ? encodedOutputStream : outputStreamFilter.apply(encodedOutputStream);
     }
 
+    private static Status noEntityInternalError(Status status) {
+        LOGGER.log(System.Logger.Level.ERROR, "Attempt to send status " + status.text() + " with entity."
+                + " Server responded with Internal Server Error. Please fix your routing, this is not allowed "
+                + "by HTTP specification, such responses MUST NOT contain an entity.");
+        return Status.INTERNAL_SERVER_ERROR_500;
+    }
+
+    private static boolean isNoEntityStatus(Status status) {
+        int code = status.code();
+        return code == Status.NO_CONTENT_204.code()
+                || code == Status.RESET_CONTENT_205.code()
+                || code == Status.NOT_MODIFIED_304.code();
+    }
+
     static class BlockingOutputStream extends OutputStream {
         private final ServerResponseHeaders headers;
         private final WritableHeaders<?> trailers;
@@ -452,7 +490,8 @@ static class BlockingOutputStream extends OutputStream {
         private boolean firstByte = true;
         private long responseBytesTotal;
         private boolean closing = false;
-        private boolean validateHeaders = false;
+        private final boolean validateHeaders;
+        private final boolean isNoEntityStatus;
 
         private BlockingOutputStream(ServerResponseHeaders headers,
                                      WritableHeaders<?> trailers,
@@ -464,7 +503,8 @@ private BlockingOutputStream(ServerResponseHeaders headers,
                                      Http1ConnectionListener sendListener,
                                      Http1ServerRequest request,
                                      boolean keepAlive,
-                                     boolean validateHeaders) {
+                                     boolean validateHeaders,
+                                     boolean isNoEntityStatus) {
             this.headers = headers;
             this.trailers = trailers;
             this.status = status;
@@ -477,6 +517,7 @@ private BlockingOutputStream(ServerResponseHeaders headers,
             this.request = request;
             this.keepAlive = keepAlive;
             this.validateHeaders = validateHeaders;
+            this.isNoEntityStatus = isNoEntityStatus;
         }
 
         void checkResponseHeaders() {
@@ -611,6 +652,9 @@ private void write(BufferData buffer) throws IOException {
             if (closed) {
                 throw new IOException("Stream already closed");
             }
+            if (isNoEntityStatus && buffer.available() > 0) {
+                throw new IllegalStateException("Attempting to write data on a response with status " + status);
+            }
 
             if (!isChunked) {
                 if (firstByte) {
@@ -649,7 +693,8 @@ private void write(BufferData buffer) throws IOException {
                     headers.add(STREAM_TRAILERS);
                 }
                 // this is chunked encoding, if anybody managed to set content length, it would break everything
-                if (headers.contains(HeaderNames.CONTENT_LENGTH)) {
+                if (headers.contains(HeaderNames.CONTENT_LENGTH)
+                        && (isNoEntityStatus || buffer.available() > 0)) {
                     LOGGER.log(System.Logger.Level.WARNING, "Content length was set after stream was requested, "
                             + "the response is already chunked, cannot use content-length");
                     headers.remove(HeaderNames.CONTENT_LENGTH);