8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream

Reviewed-by: dfuchs

test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java

@@ -23,8 +23,9 @@
 
 /*
  * @test
- * @bug 4533243
- * @summary Closing a keep alive stream gives NullPointerException
+ * @bug 4533243 8263364
+ * @summary Closing a keep alive stream should not give NullPointerException and should accept a connection from  a
+ *          client only from this test
  * @library /test/lib
  * @run main/othervm/timeout=30 KeepAliveStreamCloseWithWrongContentLength
  */
@@ -32,82 +33,152 @@
 import java.net.*;
 import java.io.*;
 import jdk.test.lib.net.URIBuilder;
+import java.nio.ByteBuffer;
+import java.nio.channels.ServerSocketChannel;
+import java.nio.channels.SocketChannel;
 
 public class KeepAliveStreamCloseWithWrongContentLength {
 
-    static class XServer extends Thread {
-        ServerSocket srv;
-        Socket s;
-        InputStream is;
-        OutputStream os;
+    private final static String path = "/KeepAliveStreamCloseWithWrongContentLength";
+    private final static String getRequest1stLine = "GET %s".formatted(path);
 
-        XServer (ServerSocket s) {
-            srv = s;
+    static class XServer extends Thread implements AutoCloseable {
+
+        final ServerSocket serverSocket;
+        volatile Socket clientSocket;
+
+        XServer (InetAddress address) throws IOException {
+            ServerSocketChannel serverSocketChannel = ServerSocketChannel.open();
+            ServerSocket serversocket = serverSocketChannel.socket();
+            serversocket.bind(new InetSocketAddress(address, 0));
+            this.serverSocket = serversocket;
+        }
+
+        public int getLocalPort() {
+            return serverSocket.getLocalPort();
         }
 
         public void run() {
+
             try {
-                s = srv.accept ();
-                // read HTTP request from client
-                InputStream is = s.getInputStream();
-                // read the first ten bytes
-                for (int i=0; i<10; i++) {
-                    is.read();
+                ByteArrayOutputStream clientBytes;
+                clientSocket = null;
+
+                // in a concurrent test environment it can happen that other rouge clients connect to this server
+                // so we need to identify and connect only to the client from this test
+                // if the rouge client sends as least bytes as there is in getRequest1stLine it will be discarded and
+                // the test should proceed otherwise it should timeout on readNBytes below
+                do {
+                    if (clientSocket != null) {
+                        final String client = "%s:%d".formatted(
+                                clientSocket.getInetAddress().getHostAddress(),
+                                clientSocket.getPort()
+                        );
+                        try {
+                            clientSocket.close();
+                        }
+                        catch (IOException ioe) {
+                            ioe.printStackTrace();
+                        }
+                        finally {
+                            System.err.println("rogue client (%s) connection attempt, ignoring".formatted(client));
+                        }
+                    }
+                    clientSocket = serverSocket.accept();
+                    // read HTTP request from client
+                    clientBytes = new ByteArrayOutputStream();
+                    clientBytes.write(clientSocket.getInputStream().readNBytes(getRequest1stLine.getBytes().length));
                 }
-                OutputStreamWriter ow =
-                    new OutputStreamWriter((os = s.getOutputStream()));
-                ow.write("HTTP/1.0 200 OK\n");
+                while(!getRequest1stLine.equals(clientBytes.toString()));
+            }
+            catch (Exception e) {
+                e.printStackTrace();
+            }
+            try  {
+                OutputStreamWriter outputStreamWriter = new OutputStreamWriter(clientSocket.getOutputStream());
+                outputStreamWriter.write("HTTP/1.0 200 OK\n");
 
                 // Note: The client expects 10 bytes.
-                ow.write("Content-Length: 10\n");
-                ow.write("Content-Type: text/html\n");
+                outputStreamWriter.write("Content-Length: 10\n");
+                outputStreamWriter.write("Content-Type: text/html\n");
 
                 // Note: If this line is missing, everything works fine.
-                ow.write("Connection: Keep-Alive\n");
-                ow.write("\n");
+                outputStreamWriter.write("Connection: Keep-Alive\n");
+                outputStreamWriter.write("\n");
 
                 // Note: The (buggy) server only sends 9 bytes.
-                ow.write("123456789");
-                ow.flush();
-            } catch (Exception e) {
-            } finally {
-                try {if (os != null) { os.close(); }} catch (IOException e) {}
+                outputStreamWriter.write("123456789");
+                outputStreamWriter.flush();
+                clientSocket.getChannel().shutdownOutput();
+            }
+            catch (Exception e) {
+                e.printStackTrace();
+            }
+        }
+
+        @Override
+        public void close() throws Exception {
+            final var clientSocket = this.clientSocket;
+            try {
+                long drained = drain(clientSocket.getChannel());
+                System.err.printf("Server drained %d bytes from the channel%n", drained);
+            } catch (Exception x) {
+                System.err.println("Server failed to drain client socket: " + x);
+                x.printStackTrace();
+            }
+            serverSocket.close();
+        }
+
+    }
+
+    static long drain(SocketChannel channel) throws IOException {
+        if (!channel.isOpen()) return 0;
+        System.err.println("Not reading server: draining socket");
+        var blocking = channel.isBlocking();
+        if (blocking) channel.configureBlocking(false);
+        long count = 0;
+        try {
+            ByteBuffer buffer = ByteBuffer.allocateDirect(8 * 1024);
+            int read;
+            while ((read = channel.read(buffer)) > 0) {
+                count += read;
+                buffer.clear();
+            }
+            return count;
+        } finally {
+            if (blocking != channel.isBlocking()) {
+                channel.configureBlocking(blocking);
             }
         }
     }
 
-    public static void main (String[] args) throws Exception {
-        final InetAddress loopback = InetAddress.getLoopbackAddress();
-        final ServerSocket serversocket = new ServerSocket();
-        serversocket.bind(new InetSocketAddress(loopback, 0));
 
-        try {
-            int port = serversocket.getLocalPort ();
-            XServer server = new XServer (serversocket);
-            server.start ();
+    public static void main (String[] args) throws Exception {
+
+        final InetAddress loopback = InetAddress.getLoopbackAddress();
+
+        try (XServer server = new XServer(loopback)) {
+            server.start();
             URL url = URIBuilder.newBuilder()
                 .scheme("http")
                 .loopback()
-                .port(port)
+                .path(path)
+                .port(server.getLocalPort())
                 .toURL();
             HttpURLConnection urlc = (HttpURLConnection)url.openConnection(Proxy.NO_PROXY);
-            InputStream is = urlc.getInputStream ();
+            InputStream is = urlc.getInputStream();
             int c = 0;
             while (c != -1) {
                 try {
                     c=is.read();
+                    System.out.println("client reads: "+c);
                 } catch (IOException ioe) {
                     is.read ();
                     break;
                 }
             }
             is.close();
-        } catch (IOException e) {
-            return;
-        } catch (NullPointerException e) {
-            throw new RuntimeException (e);
-        } finally {
-            serversocket.close();
         }
+
     }
 }