Avoids running the encoders (such as GZIP) when no data is written as…

… part of the response. Issue helidon-io#9116. Signed-off-by: Santiago Pericas-Geertsen <santiago.pericasgeertsen@oracle.com>
spericas · Aug 8, 2024 · 5f46847 · 5f46847
1 parent 4d51871
commit 5f46847
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 36 deletions.
diff --git a/...er/tests/webserver/src/test/java/io/helidon/webserver/tests/ContentEncodingEmptyTest.java b/...er/tests/webserver/src/test/java/io/helidon/webserver/tests/ContentEncodingEmptyTest.java
@@ -44,6 +44,10 @@ static void routing(HttpRules rules) {
         rules.post("/hello", (req, res) -> {
             res.status(Status.NO_CONTENT_204);
             res.send();     // empty entity
+        }).post("/hello_filter", (req, res) -> {
+            res.streamFilter(os -> os);     // forces filter codepath
+            res.status(Status.NO_CONTENT_204);
+            res.send();
         });
     }
 
@@ -55,4 +59,13 @@ void gzipEncodeEmptyEntity() {
                 .request();
         assertThat(res.status().code(), is(204));
     }
+
+    @Test
+    void gzipEncodeEmptyEntityFilter() {
+        Http1ClientResponse res = client.post("hello_filter")
+                .header(HeaderNames.CONTENT_TYPE, "application/json")
+                .header(HeaderNames.ACCEPT_ENCODING, "gzip")
+                .request();
+        assertThat(res.status().code(), is(204));
+    }
 }
diff --git a/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1ServerResponse.java b/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1ServerResponse.java
@@ -181,7 +181,9 @@ public void send(byte[] bytes) {
             dataWriter.write(bufferData);
             afterSend();
         } else {
-            try (OutputStream os = outputStream()) {
+            // we should skip encoders if no data is written (e.g. for GZIP)
+            boolean skipEncoders = (bytes.length == 0);
+            try (OutputStream os = outputStream(skipEncoders)) {
                 os.write(bytes);
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
@@ -196,41 +198,7 @@ public boolean isSent() {
 
     @Override
     public OutputStream outputStream() {
-        if (isSent) {
-            throw new IllegalStateException("Response already sent");
-        }
-        if (streamingEntity) {
-            throw new IllegalStateException("OutputStream already obtained");
-        }
-        streamingEntity = true;
-
-        BlockingOutputStream bos = new BlockingOutputStream(headers,
-                                                     trailers,
-                                                     this::status,
-                                                     () -> streamResult,
-                                                     dataWriter,
-                                                     () -> {
-                                                         this.isSent = true;
-                                                         afterSend();
-                                                         request.reset();
-                                                     },
-                                                     ctx,
-                                                     sendListener,
-                                                     request,
-                                                     keepAlive,
-                                                     validateHeaders);
-
-        int writeBufferSize = ctx.listenerContext().config().writeBufferSize();
-        outputStream = new ClosingBufferedOutputStream(bos, writeBufferSize);
-
-        OutputStream encodedOutputStream = contentEncode(outputStream);
-        // Headers can be augmented by encoders
-        bos.checkResponseHeaders();
-        if (outputStreamFilter == null) {
-            return encodedOutputStream;
-        } else {
-            return outputStreamFilter.apply(encodedOutputStream);
-        }
+        return outputStream(false);
     }
 
     @Override
@@ -401,6 +369,42 @@ private BufferData responseBuffer(byte[] bytes) {
         return responseBuffer;
     }
 
+    private OutputStream outputStream(boolean skipEncoders) {
+        if (isSent) {
+            throw new IllegalStateException("Response already sent");
+        }
+        if (streamingEntity) {
+            throw new IllegalStateException("OutputStream already obtained");
+        }
+        streamingEntity = true;
+
+        BlockingOutputStream bos = new BlockingOutputStream(headers,
+                                                            trailers,
+                                                            this::status,
+                                                            () -> streamResult,
+                                                            dataWriter,
+                                                            () -> {
+                                                                this.isSent = true;
+                                                                afterSend();
+                                                                request.reset();
+                                                            },
+                                                            ctx,
+                                                            sendListener,
+                                                            request,
+                                                            keepAlive,
+                                                            validateHeaders);
+
+        int writeBufferSize = ctx.listenerContext().config().writeBufferSize();
+        outputStream = new ClosingBufferedOutputStream(bos, writeBufferSize);
+
+        OutputStream encodedOutputStream = outputStream;
+        if (!skipEncoders) {
+            encodedOutputStream = contentEncode(outputStream);
+            bos.checkResponseHeaders();     // headers can be augmented by encoders
+        }
+        return outputStreamFilter == null ? encodedOutputStream : outputStreamFilter.apply(encodedOutputStream);
+    }
+
     static class BlockingOutputStream extends OutputStream {
         private final ServerResponseHeaders headers;
         private final WritableHeaders<?> trailers;