8180498: Remove httpclient internal APIs which supply ByteBuffers to read calls

Reviewed-by: chegar, dfuchs

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncSSLConnection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -189,11 +189,6 @@ class AsyncSSLConnection extends HttpConnection
         throw new UnsupportedOperationException("Not supported.");
     }
 
-    @Override
-    protected int readImpl(ByteBuffer buffer) throws IOException {
-        throw new UnsupportedOperationException("Not supported.");
-    }
-
     @Override
     CompletableFuture<Void> whenReceivingResponse() {
         throw new UnsupportedOperationException("Not supported.");

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java

@@ -33,7 +33,6 @@ import java.net.SocketPermission;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLPermission;
-import java.nio.ByteBuffer;
 import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -76,9 +75,6 @@ final class Exchange<T> {
     boolean upgrading; // to HTTP/2
     final PushGroup<?,T> pushGroup;
 
-    // buffer for receiving response headers
-    private volatile ByteBuffer rxBuffer;
-
     Exchange(HttpRequestImpl request, MultiExchange<?,T> multi) {
         this.request = request;
         this.upgrading = false;
@@ -121,17 +117,6 @@ final class Exchange<T> {
         return client;
     }
 
-    ByteBuffer getBuffer() {
-        if(rxBuffer == null) {
-            synchronized (this) {
-                if(rxBuffer == null) {
-                    rxBuffer = Utils.getExchangeBuffer();
-                }
-            }
-        }
-        return rxBuffer;
-    }
-
     public Response response() throws IOException, InterruptedException {
         return responseImpl(null);
     }

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Exchange.java

@@ -55,7 +55,7 @@ class Http1Exchange<T> extends ExchangeImpl<T> {
     final HttpConnection connection;
     final HttpClientImpl client;
     final Executor executor;
-    final ByteBuffer buffer; // used for receiving
+    volatile ByteBuffer buffer; // used for receiving
 
     @Override
     public String toString() {
@@ -74,7 +74,7 @@ class Http1Exchange<T> extends ExchangeImpl<T> {
         this.client = exchange.client();
         this.executor = exchange.executor();
         this.operations = new LinkedList<>();
-        this.buffer = exchange.getBuffer();
+        this.buffer = Utils.EMPTY_BYTEBUFFER;
         if (connection != null) {
             this.connection = connection;
         } else {
@@ -157,7 +157,9 @@ class Http1Exchange<T> extends ExchangeImpl<T> {
         try {
             response = new Http1Response<>(connection, this);
             response.readHeaders();
-            return response.response();
+            Response r = response.response();
+            buffer = response.getBuffer();
+            return r;
         } catch (Throwable t) {
             connection.close();
             throw t;
@@ -213,7 +215,9 @@ class Http1Exchange<T> extends ExchangeImpl<T> {
         return MinimalFuture.supply( () -> {
             response = new Http1Response<>(connection, Http1Exchange.this);
             response.readHeaders();
-            return response.response();
+            Response r = response.response();
+            buffer = response.getBuffer();
+            return r;
         }, executor);
     }
 

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Response.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -44,7 +44,7 @@ class Http1Response<T> {
     private final HttpConnection connection;
     private ResponseHeaders headers;
     private int responseCode;
-    private final ByteBuffer buffer; // same buffer used for reading status line and headers
+    private ByteBuffer buffer;
     private final Http1Exchange<T> exchange;
     private final boolean redirecting; // redirecting
     private boolean return2Cache; // return connection to cache when finished
@@ -96,6 +96,10 @@ class Http1Response<T> {
         return finished;
     }
 
+    ByteBuffer getBuffer() {
+        return buffer;
+    }
+
     int fixupContentLen(int clen) {
         if (request.method().equalsIgnoreCase("HEAD")) {
             return 0;
@@ -194,12 +198,15 @@ class Http1Response<T> {
     static final char CR = '\r';
     static final char LF = '\n';
 
-    private int getBuffer() throws IOException {
+    private int obtainBuffer() throws IOException {
         int n = buffer.remaining();
 
         if (n == 0) {
-            buffer.clear();
+            buffer = connection.read();
-            return connection.read(buffer);
+            if (buffer == null) {
+                return -1;
+            }
+            n = buffer.remaining();
         }
         return n;
     }
@@ -207,18 +214,17 @@ class Http1Response<T> {
     String readStatusLine() throws IOException {
         boolean cr = false;
         StringBuilder statusLine = new StringBuilder(128);
-        ByteBuffer b = buffer;
+        while ((obtainBuffer()) != -1) {
-        while (getBuffer() != -1) {
+            byte[] buf = buffer.array();
-            byte[] buf = b.array();
+            int offset = buffer.position();
-            int offset = b.position();
+            int len = buffer.limit() - offset;
-            int len = b.limit() - offset;
 
             for (int i = 0; i < len; i++) {
                 char c = (char) buf[i+offset];
 
                 if (cr) {
                     if (c == LF) {
-                        b.position(i + 1 + offset);
+                        buffer.position(i + 1 + offset);
                         return statusLine.toString();
                     } else {
                         throw new IOException("invalid status line");
@@ -231,7 +237,7 @@ class Http1Response<T> {
                 }
             }
             // unlikely, but possible, that multiple reads required
-            b.position(b.limit());
+            buffer.position(buffer.limit());
         }
         return null;
     }

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpConnection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -323,12 +323,9 @@ abstract class HttpConnection implements Closeable {
         }
     }
 
-    final int read(ByteBuffer buffer) throws IOException {
-        return readImpl(buffer);
-    }
-
     final ByteBuffer read() throws IOException {
-        return readImpl();
+        ByteBuffer b = readImpl();
+        return b;
     }
 
     /*
@@ -337,9 +334,6 @@ abstract class HttpConnection implements Closeable {
      */
     protected abstract ByteBuffer readImpl() throws IOException;
 
-    /** Reads as much as possible into given buffer and returns amount read. */
-    protected abstract int readImpl(ByteBuffer buffer) throws IOException;
-
     @Override
     public String toString() {
         return "HttpConnection: " + channel().toString();

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java

@@ -311,8 +311,7 @@ class PlainHttpConnection extends HttpConnection implements AsyncConnection {
         }
     }
 
-    @Override
+    private int readImpl(ByteBuffer buf) throws IOException {
-    protected int readImpl(ByteBuffer buf) throws IOException {
         int mark = buf.position();
         int n;
         // FIXME: this hack works in conjunction with the corresponding change

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainTunnelingConnection.java

@@ -156,11 +156,6 @@ class PlainTunnelingConnection extends HttpConnection {
         return delegate.readImpl();
     }
 
-    @Override
-    protected int readImpl(ByteBuffer buffer) throws IOException {
-        return delegate.readImpl(buffer);
-    }
-
     @Override
     boolean isSecure() {
         return false;

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseContent.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -148,11 +148,7 @@ class ResponseContent {
     // make sure we have at least 1 byte to look at
     private void getHunk() throws IOException {
         if (chunkbuf == null || !chunkbuf.hasRemaining()) {
-            if (chunkbuf == null) {
+            chunkbuf = connection.read();
-                chunkbuf = Utils.getBuffer();
-            }
-            chunkbuf.clear();
-            connection.read(chunkbuf);
         }
     }
 
@@ -256,7 +252,6 @@ class ResponseContent {
 
     private void pushBodyFixed(ByteBuffer b) throws IOException {
         int remaining = contentLength;
-        //lastBufferUsed = b;
         while (b.hasRemaining() && remaining > 0) {
             ByteBuffer buffer = Utils.getBuffer();
             int amount = Math.min(b.remaining(), remaining);
@@ -265,22 +260,14 @@ class ResponseContent {
             buffer.flip();
             dataConsumer.accept(Optional.of(buffer));
         }
-        //client.returnBuffer(b);
         while (remaining > 0) {
-            ByteBuffer buffer = Utils.getBuffer();
+            ByteBuffer buffer = connection.read();
-            int xx = connection.read(buffer);
+            if (buffer == null)
-            if (xx == -1)
                 throw new IOException("connection closed");
 
             int bytesread = buffer.remaining();
             // assume for now that pipelining not implemented
             if (bytesread > remaining) {
-                System.err.println("xx = " + xx);
-                System.err.println("bytesread = " + bytesread);
-                System.err.println("remaining = " + remaining);
-                for (int i=0; i<remaining; i++) {
-                    System.err.printf("%x ", buffer.get());
-                }
                 throw new IOException("too many bytes read");
             }
             remaining -= bytesread;

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseHeaders.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -72,7 +72,7 @@ final class ResponseHeaders implements HttpHeaders {
 
     static final class InputStreamWrapper extends InputStream {
         final HttpConnection connection;
-        final ByteBuffer buffer;
+        ByteBuffer buffer;
         int lastRead = -1; // last byte read from the buffer
         int consumed = 0; // number of bytes consumed.
         InputStreamWrapper(HttpConnection connection, ByteBuffer buffer) {
@@ -83,9 +83,8 @@ final class ResponseHeaders implements HttpHeaders {
         @Override
         public int read() throws IOException {
             if (!buffer.hasRemaining()) {
-                buffer.clear();
+                buffer = connection.read();
-                int n = connection.read(buffer);
+                if (buffer == null) {
-                if (n == -1) {
                     return lastRead = -1;
                 }
             }
@@ -97,6 +96,16 @@ final class ResponseHeaders implements HttpHeaders {
         }
     }
 
+    private static void display(Map<String, List<String>> map) {
+        map.forEach((k,v) -> {
+            System.out.print (k + ": ");
+            for (String val : v) {
+                System.out.print(val + ", ");
+            }
+            System.out.println("");
+        });
+    }
+
     private Map<String, List<String>> parse(InputStreamWrapper input)
          throws IOException
     {
@@ -114,7 +123,6 @@ final class ResponseHeaders implements HttpHeaders {
             // finds is CR. This only happens if there are no headers, and
             // only one byte will be consumed from the buffer. In this case
             // the next byte MUST be LF
-            //System.err.println("Last character read is: " + (byte)lastRead);
             if (input.read() != LF) {
                 throw new IOException("Unexpected byte sequence when no headers: "
                      + ((int)CR) + " " + input.lastRead

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLConnection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -162,10 +162,11 @@ class SSLConnection extends HttpConnection {
 
     @Override
     protected ByteBuffer readImpl() throws IOException {
-        ByteBuffer dst = ByteBuffer.allocate(8192);
+        WrapperResult r = sslDelegate.recvData(ByteBuffer.allocate(8192));
-        int n = readImpl(dst);
+        // TODO: check for closure
+        int n = r.result.bytesProduced();
         if (n > 0) {
-            return dst;
+            return r.buf;
         } else if (n == 0) {
             return Utils.EMPTY_BYTEBUFFER;
         } else {
@@ -173,19 +174,6 @@ class SSLConnection extends HttpConnection {
         }
     }
 
-    @Override
-    protected int readImpl(ByteBuffer buf) throws IOException {
-        // TODO: need to ensure that buf is big enough for application data
-        WrapperResult r = sslDelegate.recvData(buf);
-        // TODO: check for closure
-        String s = "Receive) ";
-        //debugPrint(s, r.buf);
-        if (r.result.bytesProduced() > 0) {
-            assert buf == r.buf;
-        }
-        return r.result.bytesProduced();
-    }
-
     @Override
     boolean connected() {
         return delegate.connected();

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLDelegate.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -104,9 +104,7 @@ class SSLDelegate {
         }
         SSLEngineResult result;
 
-        /* if passed in buffer was not big enough then the a reallocated buffer
+        ByteBuffer buf; // buffer containing result data
-         * is returned here  */
-        ByteBuffer buf;
     }
 
     int app_buf_size;

jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLTunnelConnection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -164,19 +164,10 @@ class SSLTunnelConnection extends HttpConnection {
 
     @Override
     protected ByteBuffer readImpl() throws IOException {
-        return sslDelegate.recvData(Utils.EMPTY_BYTEBUFFER).buf; // fix this, make the read normal
+        ByteBuffer buf = Utils.getBuffer();
-    }
 
-    @Override
-    protected int readImpl(ByteBuffer buf) throws IOException {
         WrapperResult r = sslDelegate.recvData(buf);
-        // TODO: check for closure
+        return r.buf;
-        String s = "Receive) ";
-        //debugPrint(s, r.buf);
-        if (r.result.bytesProduced() > 0) {
-            assert buf == r.buf;
-        }
-        return r.result.bytesProduced();
     }
 
     @Override

jdk/test/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/ResponseHeadersTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -214,10 +214,6 @@ public class ResponseHeadersTest {
         protected ByteBuffer readImpl() throws IOException {
             throw new AssertionError("Bad test assumption: should not have reached here!");
         }
-        @Override
-        protected int readImpl(ByteBuffer buffer) throws IOException {
-            throw new AssertionError("Bad test assumption: should not have reached here!");
-        }
     }
 
     public static HttpHeaders createResponseHeaders(ByteBuffer buffer)