UNDERTOW-2207: UndertowOutputStream allows flush after writes

Previously, setting Content-Length would auto-close the stream
in such a way that a flush after the final write would throw
an IOException due to internal self-closure.

core/src/main/java/io/undertow/io/UndertowOutputStream.java

@@ -272,11 +272,18 @@ void updateWritten(final long len) throws IOException {
      * {@inheritDoc}
      */
     public void flush() throws IOException {
-        if (anyAreSet(state, FLAG_CLOSED)) {
-            throw UndertowMessages.MESSAGES.streamIsClosed();
-        }
+        boolean closed = anyAreSet(state, FLAG_CLOSED);
         if (buffer != null && buffer.position() != 0) {
+            if (closed) {
+                throw UndertowMessages.MESSAGES.streamIsClosed();
+            }
             writeBufferBlocking(false);
+        } else if (closed) {
+            // No-op if flush is called with no buffered data on a closed channel.
+            // This stream closes itself when a Content-Length is provided, once sufficient
+            // bytes have been written -- a flush following the last write is entirely
+            // reasonable.
+            return;
         }
         if (channel == null) {
             channel = exchange.getResponseChannel();

core/src/test/java/io/undertow/server/handlers/blocking/BlockingServerStreamClosureTestCase.java

@@ -0,0 +1,119 @@
+/*
+ * JBoss, Home of Professional Open Source.
+ * Copyright 2022 Red Hat, Inc., and individual contributors
+ * as indicated by the @author tags.
+ *
+ * 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.undertow.server.handlers.blocking;
+
+import io.undertow.server.handlers.BlockingHandler;
+import io.undertow.testutils.DefaultServer;
+import io.undertow.testutils.HttpClientUtils;
+import io.undertow.testutils.TestHttpClient;
+import io.undertow.util.Headers;
+import io.undertow.util.StatusCodes;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * @author Carter Kozak
+ */
+@RunWith(DefaultServer.class)
+public class BlockingServerStreamClosureTestCase {
+
+    @Test
+    public void testFlushAfterContentLengthReached() throws IOException {
+        AtomicReference<Throwable> thrown = new AtomicReference<>();
+        DefaultServer.setRootHandler(new BlockingHandler(exchange -> {
+            exchange.getResponseHeaders().put(Headers.CONTENT_LENGTH, "1");
+            try {
+                OutputStream out = exchange.getOutputStream();
+                out.write(65);
+                out.flush();
+                out.close();
+            } catch (Throwable t) {
+                thrown.set(t);
+                throw t;
+            }
+        }));
+        makeSuccessfulRequest("A");
+        Throwable maybeFailure = thrown.get();
+        if (maybeFailure != null) {
+            throw new AssertionError("Unexpected failure", maybeFailure);
+        }
+    }
+
+    @Test
+    public void testEmptyWriteAfterContentLength() throws IOException {
+        AtomicReference<Throwable> thrown = new AtomicReference<>();
+        DefaultServer.setRootHandler(new BlockingHandler(exchange -> {
+            exchange.getResponseHeaders().put(Headers.CONTENT_LENGTH, "1");
+            try {
+                OutputStream out = exchange.getOutputStream();
+                out.write(65);
+                out.write(new byte[10], 2, 0);
+                out.close();
+            } catch (Throwable t) {
+                thrown.set(t);
+                throw t;
+            }
+        }));
+        makeSuccessfulRequest("A");
+        Throwable maybeFailure = thrown.get();
+        if (maybeFailure != null) {
+            throw new AssertionError("Unexpected failure", maybeFailure);
+        }
+    }
+
+    @Test
+    public void testFlushAfterClose() throws IOException {
+        AtomicReference<Throwable> thrown = new AtomicReference<>();
+        DefaultServer.setRootHandler(new BlockingHandler(exchange -> {
+            try {
+                OutputStream out = exchange.getOutputStream();
+                out.write(65);
+                out.close();
+                out.flush();
+            } catch (Throwable t) {
+                thrown.set(t);
+                throw t;
+            }
+        }));
+        makeSuccessfulRequest("A");
+        Throwable maybeFailure = thrown.get();
+        if (maybeFailure != null) {
+            throw new AssertionError("Unexpected failure", maybeFailure);
+        }
+    }
+
+    private static void makeSuccessfulRequest(String expectedContent) throws IOException {
+        TestHttpClient client = new TestHttpClient();
+        try {
+            HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL());
+            HttpResponse result = client.execute(get);
+            Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
+            Assert.assertEquals(expectedContent, HttpClientUtils.readResponse(result));
+        } finally {
+            client.getConnectionManager().shutdown();
+        }
+    }
+}

servlet/src/test/java/io/undertow/servlet/test/streams/ServletOutputStreamClosureTestCase.java

@@ -0,0 +1,110 @@
+/*
+ * JBoss, Home of Professional Open Source.
+ * Copyright 2022 Red Hat, Inc., and individual contributors
+ * as indicated by the @author tags.
+ *
+ * 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.undertow.servlet.test.streams;
+
+import io.undertow.servlet.api.ServletInfo;
+import io.undertow.servlet.test.util.DeploymentUtils;
+import io.undertow.servlet.util.ImmediateInstanceFactory;
+import io.undertow.testutils.DefaultServer;
+import io.undertow.testutils.HttpClientUtils;
+import io.undertow.testutils.TestHttpClient;
+import io.undertow.util.Headers;
+import io.undertow.util.StatusCodes;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.servlet.ServletOutputStream;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * @author Carter Kozak
+ */
+@RunWith(DefaultServer.class)
+public class ServletOutputStreamClosureTestCase {
+
+    @Test
+    public void testFlushAfterContentLengthReached() throws IOException {
+        AtomicReference<Throwable> thrown = new AtomicReference<>();
+        DeploymentUtils.setupServlet(
+                new ServletInfo("servlet", HttpServlet.class, new ImmediateInstanceFactory<HttpServlet>(new HttpServlet() {
+                    @Override
+                    protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+                        resp.setHeader(Headers.CONTENT_LENGTH_STRING, "1");
+                        try {
+                            ServletOutputStream out = resp.getOutputStream();
+                            out.write(65);
+                            out.flush();
+                            out.close();
+                        } catch (Throwable t) {
+                            thrown.set(t);
+                            throw t;
+                        }
+                    }
+                })).addMapping("/*"));
+        makeSuccessfulRequest("A");
+        Throwable maybeFailure = thrown.get();
+        if (maybeFailure != null) {
+            throw new AssertionError("Unexpected failure", maybeFailure);
+        }
+    }
+
+    @Test
+    public void testFlushAfterClose() throws IOException {
+        AtomicReference<Throwable> thrown = new AtomicReference<>();
+        DeploymentUtils.setupServlet(
+                new ServletInfo("servlet", HttpServlet.class, new ImmediateInstanceFactory<HttpServlet>(new HttpServlet() {
+                    @Override
+                    protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+                        try {
+                            ServletOutputStream out = resp.getOutputStream();
+                            out.write(65);
+                            out.close();
+                            out.flush();
+                        } catch (Throwable t) {
+                            thrown.set(t);
+                            throw t;
+                        }
+                    }
+                })).addMapping("/*"));
+        makeSuccessfulRequest("A");
+        Throwable maybeFailure = thrown.get();
+        if (maybeFailure != null) {
+            throw new AssertionError("Unexpected failure", maybeFailure);
+        }
+    }
+
+    private static void makeSuccessfulRequest(String expectedContent) throws IOException {
+        TestHttpClient client = new TestHttpClient();
+        try {
+            HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext");
+            HttpResponse result = client.execute(get);
+            Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
+            Assert.assertEquals(expectedContent, HttpClientUtils.readResponse(result));
+        } finally {
+            client.getConnectionManager().shutdown();
+        }
+    }
+}