4.x: Allow Unbuffered OutputStream for Http1ServerResponse (#9276)

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

@@ -75,7 +75,7 @@ class Http1ServerResponse extends ServerResponseBase<Http1ServerResponse> {
 
     private boolean streamingEntity;
     private boolean isSent;
-    private ClosingBufferedOutputStream outputStream;
+    private ResponseOutputStream outputStream;
     private long bytesWritten;
     private String streamResult = "";
     private final boolean validateHeaders;
@@ -395,7 +395,11 @@ private OutputStream outputStream(boolean skipEncoders) {
                                                             validateHeaders);
 
         int writeBufferSize = ctx.listenerContext().config().writeBufferSize();
-        outputStream = new ClosingBufferedOutputStream(bos, writeBufferSize);
+        if (writeBufferSize > 0) {
+            outputStream = new ClosingBufferedOutputStream(bos, writeBufferSize);
+        } else {
+            outputStream = bos;
+        }
 
         OutputStream encodedOutputStream = outputStream;
         if (!skipEncoders) {
@@ -405,7 +409,29 @@ private OutputStream outputStream(boolean skipEncoders) {
         return outputStreamFilter == null ? encodedOutputStream : outputStreamFilter.apply(encodedOutputStream);
     }
 
-    static class BlockingOutputStream extends OutputStream {
+    abstract static class ResponseOutputStream extends OutputStream {
+        abstract long totalBytesWritten();
+
+        abstract void commit();
+
+        /**
+         * This is a noop, even when user closes the output stream, we wait for the
+         * call to {@link this#commit()}.
+         */
+        @Override
+        public void close() {
+            // no-op
+        }
+
+        /**
+         * Calls the {@link OutputStream#close()}, which is currently a no-op.
+         */
+        void superClose() throws IOException {
+            super.close();
+        }
+    }
+
+    static class BlockingOutputStream extends ResponseOutputStream {
         private final ServerResponseHeaders headers;
         private final WritableHeaders<?> trailers;
         private final Supplier<Status> status;
@@ -490,15 +516,6 @@ public void flush() throws IOException {
             }
         }
 
-        /**
-         * This is a noop, even when user closes the output stream, we wait for the
-         * call to {@link this#commit()}.
-         */
-        @Override
-        public void close() {
-            // no-op
-        }
-
         /**
          * Informs output stream that closing phase has started. Special handling
          * for {@link this#flush()}.
@@ -542,12 +559,13 @@ void commit() {
 
             responseCloseRunnable.run();
             try {
-                super.close();
+                superClose();
             } catch (IOException e) {
                 throw new ServerConnectionException("Failed to close server response stream.", e);
             }
         }
 
+        @Override
         long totalBytesWritten() {
             return responseBytesTotal;
         }
@@ -728,7 +746,7 @@ private void writeContent(BufferData buffer) throws IOException {
      * of close logic. Note that due to some locking issues in the JDK, this class
      * must use delegation with {@link BufferedOutputStream} instead of subclassing.
      */
-    static class ClosingBufferedOutputStream extends OutputStream {
+    static class ClosingBufferedOutputStream extends ResponseOutputStream {
 
         private final BlockingOutputStream delegate;
         private final BufferedOutputStream bufferedDelegate;
@@ -768,10 +786,12 @@ public void close() {
             }
         }
 
+        @Override
         long totalBytesWritten() {
             return delegate.totalBytesWritten();
         }
 
+        @Override
         void commit() {
             try {
                 flush();

webserver/webserver/src/test/java/io/helidon/webserver/http1/WriteBufferTest.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.webserver.http1;
+
+import io.helidon.webserver.WebServer;
+import io.helidon.webserver.http.Handler;
+import io.helidon.webserver.http1.Http1ServerResponse.BlockingOutputStream;
+import io.helidon.webserver.http1.Http1ServerResponse.ClosingBufferedOutputStream;
+import org.junit.jupiter.api.Test;
+
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+
+import io.helidon.webserver.http.ServerResponse;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+class WriteBufferTest {
+
+    /**
+     * Test that a simple response can be sent using the {@link ServerResponse#outputStream()} using the default
+     * (non-zero) write buffer.
+     */
+    @Test
+    void defaultWriteBufferTest() throws Exception {
+        String path = "/test";
+        String response = "Hello World!";
+        Handler handler = (req, res) -> {
+            try(OutputStream out = res.outputStream()) {
+                assertThat(out, instanceOf(ClosingBufferedOutputStream.class));
+                out.write(response.getBytes(StandardCharsets.UTF_8));
+            }
+        };
+        WebServer server = WebServer.builder().port(0).routing(it -> it.get(path, handler)).build().start();
+        try {
+            URL url = new URI("http://localhost:" + server.port() + path).toURL();
+            HttpURLConnection conn = (HttpURLConnection)url.openConnection();
+            conn.setRequestMethod("GET");
+            assertThat(conn.getResponseCode(), is(200));
+            String received = new String(conn.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
+            assertThat(received, is(response));
+        } finally {
+            server.stop();
+        }
+    }
+
+    /**
+     * Test that a simple response can be sent using the {@link ServerResponse#outputStream()} using no write buffer
+     * (i.e. the write buffer size was set to {@code 0}).
+     */
+    @Test
+    void noWriteBufferTest() throws Exception {
+        String path = "/test";
+        String response = "Hello World!";
+        Handler handler = (req, res) -> {
+            try(OutputStream out = res.outputStream()) {
+                assertThat(out, instanceOf(BlockingOutputStream.class));
+                out.write(response.getBytes(StandardCharsets.UTF_8));
+            }
+        };
+        WebServer server = WebServer.builder().port(0).writeBufferSize(0)
+                .routing(it -> it.get(path, handler)).build().start();
+        try {
+            URL url = new URI("http://localhost:" + server.port() + path).toURL();
+            HttpURLConnection conn = (HttpURLConnection)url.openConnection();
+            conn.setRequestMethod("GET");
+            assertThat(conn.getResponseCode(), is(200));
+            String received = new String(conn.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
+            assertThat(received, is(response));
+        } finally {
+            server.stop();
+        }
+    }
+
+}