8266761: AssertionError in sun.net.httpserver.ServerImpl.responseCompleted

Reviewed-by: chegar

3 files changed, 241 insertions, 4 deletions

diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java
index 966a9b0932c..454b6cd435c 100644
--- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java
+++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java
@@ -201,6 +201,7 @@ class ExchangeImpl {
     public void sendResponseHeaders (int rCode, long contentLen)
     throws IOException
     {
+        final Logger logger = server.getLogger();
         if (sentHeaders) {
             throw new IOException ("headers already sent");
         }
@@ -220,7 +221,6 @@ class ExchangeImpl {
             ||(rCode == 304))          /* not modified */
         {
             if (contentLen != -1) {
-                Logger logger = server.getLogger();
                 String msg = "sendResponseHeaders: rCode = "+ rCode
                     + ": forcing contentLen = -1";
                 logger.log (Level.WARNING, msg);
@@ -234,7 +234,6 @@ class ExchangeImpl {
              * through this API, but should instead manually set the required
              * headers.*/
             if (contentLen >= 0) {
-                final Logger logger = server.getLogger();
                 String msg =
                     "sendResponseHeaders: being invoked with a content length for a HEAD request";
                 logger.log (Level.WARNING, msg);
@@ -271,7 +270,6 @@ class ExchangeImpl {
                     Optional.ofNullable(rspHdrs.get("Connection"))
                     .map(List::stream).orElse(Stream.empty());
             if (conheader.anyMatch("close"::equalsIgnoreCase)) {
-                Logger logger = server.getLogger();
                 logger.log (Level.DEBUG, "Connection: close requested by handler");
                 close = true;
             }
@@ -282,6 +280,7 @@ class ExchangeImpl {
         tmpout.flush() ;
         tmpout = null;
         sentHeaders = true;
+        logger.log(Level.TRACE, "Sent headers: noContentToSend=" + noContentToSend);
         if (noContentToSend) {
             WriteFinishedEvent e = new WriteFinishedEvent (this);
             server.addEvent (e);
diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
index ed12c78a3c2..75afa78844c 100644
--- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
+++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
@@ -290,15 +290,19 @@ class ServerImpl implements TimeSource {
             try {
                 if (r instanceof WriteFinishedEvent) {
 
+                    logger.log(Level.TRACE, "Write Finished");
                     int exchanges = endExchange();
                     if (terminating && exchanges == 0) {
                         finished = true;
                     }
-                    responseCompleted (c);
                     LeftOverInputStream is = t.getOriginalInputStream();
                     if (!is.isEOF()) {
                         t.close = true;
+                        if (c.getState() == State.REQUEST) {
+                            requestCompleted(c);
+                        }
                     }
+                    responseCompleted (c);
                     if (t.close || idleConnections.size() >= MAX_IDLE_CONNECTIONS) {
                         c.close();
                         allConnections.remove (c);
@@ -512,6 +516,7 @@ class ServerImpl implements TimeSource {
 
         public void run () {
             /* context will be null for new connections */
+            logger.log(Level.TRACE, "exchange started");
             context = connection.getHttpContext();
             boolean newconnection;
             SSLEngine engine = null;
@@ -678,6 +683,9 @@ class ServerImpl implements TimeSource {
             } catch (Exception e4) {
                 logger.log (Level.TRACE, "ServerImpl.Exchange (4)", e4);
                 closeConnection(connection);
+            } catch (Throwable t) {
+                logger.log(Level.TRACE, "ServerImpl.Exchange (5)", t);
+                throw t;
             }
         }
 
diff --git a/test/jdk/com/sun/net/httpserver/InputNotRead.java b/test/jdk/com/sun/net/httpserver/InputNotRead.java
new file mode 100644
index 00000000000..5b67f8d8b24
--- /dev/null
+++ b/test/jdk/com/sun/net/httpserver/InputNotRead.java
@@ -0,0 +1,230 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8266761
+ * @summary HttpServer can fail with an assertion error when a handler doesn't fully
+ *          read the request body and sends back a reply with no content, or when
+ *          the client closes its outputstream while the server tries to drains
+ *          its content.
+ * @run testng/othervm InputNotRead
+ * @run testng/othervm -Djava.net.preferIPv6Addresses=true InputNotRead
+ */
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+
+import org.testng.annotations.Test;
+
+import static java.nio.charset.StandardCharsets.*;
+
+public class InputNotRead {
+
+    private static final int msgCode = 200;
+    private static final String someContext = "/context";
+
+    static class ServerThreadFactory implements ThreadFactory {
+        static final AtomicLong tokens = new AtomicLong();
+        @Override
+        public Thread newThread(Runnable r) {
+            var thread = new Thread(r, "Server-" + tokens.incrementAndGet());
+            thread.setDaemon(true);
+            return thread;
+        }
+    }
+
+    static {
+        Logger.getLogger("").setLevel(Level.ALL);
+        Logger.getLogger("").getHandlers()[0].setLevel(Level.ALL);
+    }
+
+    @Test
+    public void testSendResponse() throws Exception {
+        System.out.println("testSendResponse()");
+        InetAddress loopback = InetAddress.getLoopbackAddress();
+        HttpServer server = HttpServer.create(new InetSocketAddress(loopback, 0), 0);
+        ExecutorService executor = Executors.newCachedThreadPool(new ServerThreadFactory());
+        server.setExecutor(executor);
+        try {
+            server.createContext(someContext, new HttpHandler() {
+                @Override
+                public void handle(HttpExchange msg) throws IOException {
+                    System.err.println("Handling request: " + msg.getRequestURI());
+                    byte[] reply = new byte[0];
+                    try {
+                        msg.getRequestBody().read();
+                        try {
+                            msg.sendResponseHeaders(msgCode, reply.length == 0 ? -1 : reply.length);
+                        } catch(IOException ioe) {
+                            ioe.printStackTrace();
+                        }
+                    } finally {
+                        // don't close the exchange and don't close any stream
+                        // to trigger the assertion.
+                        System.err.println("Request handled: " + msg.getRequestURI());
+                    }
+                }
+            });
+            server.start();
+            System.out.println("Server started at port "
+                               + server.getAddress().getPort());
+
+            runRawSocketHttpClient(loopback, server.getAddress().getPort(), -1);
+        } finally {
+            System.out.println("shutting server down");
+            executor.shutdown();
+            server.stop(0);
+        }
+        System.out.println("Server finished.");
+    }
+
+    @Test
+    public void testCloseOutputStream() throws Exception {
+        System.out.println("testCloseOutputStream()");
+        InetAddress loopback = InetAddress.getLoopbackAddress();
+        HttpServer server = HttpServer.create(new InetSocketAddress(loopback, 0), 0);
+        ExecutorService executor = Executors.newCachedThreadPool(new ServerThreadFactory());
+        server.setExecutor(executor);
+        try {
+            server.createContext(someContext, new HttpHandler() {
+                @Override
+                public void handle(HttpExchange msg) throws IOException {
+                    System.err.println("Handling request: " + msg.getRequestURI());
+                    byte[] reply = "Here is my reply!".getBytes(UTF_8);
+                    try {
+                        BufferedReader r = new BufferedReader(new InputStreamReader(msg.getRequestBody()));
+                        r.read();
+                        try {
+                            msg.sendResponseHeaders(msgCode, reply.length == 0 ? -1 : reply.length);
+                            msg.getResponseBody().write(reply);
+                            msg.getResponseBody().close();
+                            Thread.sleep(50);
+                        } catch(IOException | InterruptedException ie) {
+                            ie.printStackTrace();
+                        }
+                    } finally {
+                        System.err.println("Request handled: " + msg.getRequestURI());
+                    }
+                }
+            });
+            server.start();
+            System.out.println("Server started at port "
+                    + server.getAddress().getPort());
+
+            runRawSocketHttpClient(loopback, server.getAddress().getPort(), 64 * 1024 + 16);
+        } finally {
+            System.out.println("shutting server down");
+            executor.shutdown();
+            server.stop(0);
+        }
+        System.out.println("Server finished.");
+    }
+
+    static void runRawSocketHttpClient(InetAddress address, int port, int contentLength)
+        throws Exception
+    {
+        Socket socket = null;
+        PrintWriter writer = null;
+        BufferedReader reader = null;
+        final String CRLF = "\r\n";
+        try {
+            socket = new Socket(address, port);
+            writer = new PrintWriter(new OutputStreamWriter(
+                socket.getOutputStream()));
+            System.out.println("Client connected by socket: " + socket);
+            String body = "I will send all the data.";
+            if (contentLength <= 0)
+                contentLength = body.getBytes(UTF_8).length;
+
+            writer.print("GET " + someContext + "/ HTTP/1.1" + CRLF);
+            writer.print("User-Agent: Java/"
+                + System.getProperty("java.version")
+                + CRLF);
+            writer.print("Host: " + address.getHostName() + CRLF);
+            writer.print("Accept: */*" + CRLF);
+            writer.print("Content-Length: " + contentLength + CRLF);
+            writer.print("Connection: keep-alive" + CRLF);
+            writer.print(CRLF); // Important, else the server will expect that
+            // there's more into the request.
+            writer.flush();
+            System.out.println("Client wrote request to socket: " + socket);
+            writer.print(body);
+            writer.flush();
+
+            reader = new BufferedReader(new InputStreamReader(
+                socket.getInputStream()));
+            System.out.println("Client start reading from server:"  );
+            String line = reader.readLine();
+            for (; line != null; line = reader.readLine()) {
+                if (line.isEmpty()) {
+                    break;
+                }
+                System.out.println("\""  + line + "\"");
+            }
+            System.out.println("Client finished reading from server"  );
+        } finally {
+            // give time to the server to try & drain its input stream
+            Thread.sleep(500);
+            // closes the client outputstream while the server is draining
+            // it
+            if (writer != null) {
+                writer.close();
+            }
+            // give time to the server to trigger its assertion
+            // error before closing the connection
+            Thread.sleep(500);
+            if (reader != null)
+                try {
+                    reader.close();
+                } catch (IOException logOrIgnore) {
+                    logOrIgnore.printStackTrace();
+                }
+            if (socket != null) {
+                try {
+                    socket.close();
+                } catch (IOException logOrIgnore) {
+                    logOrIgnore.printStackTrace();
+                }
+            }
+        }
+        System.out.println("Client finished." );
+    }
+
+}