8180498: Remove httpclient internal APIs which supply ByteBuffers to read calls
Reviewed-by: chegar, dfuchs
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncSSLConnection.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncSSLConnection.java Mon May 22 17:31:07 2017 +0100
@@ -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
@@ -190,11 +190,6 @@
}
@Override
- protected int readImpl(ByteBuffer buffer) throws IOException {
- throw new UnsupportedOperationException("Not supported.");
- }
-
- @Override
CompletableFuture<Void> whenReceivingResponse() {
throw new UnsupportedOperationException("Not supported.");
}
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java Mon May 22 17:31:07 2017 +0100
@@ -33,7 +33,6 @@
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 @@
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 @@
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);
}
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Exchange.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Exchange.java Mon May 22 17:31:07 2017 +0100
@@ -55,7 +55,7 @@
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 @@
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 @@
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 @@
return MinimalFuture.supply( () -> {
response = new Http1Response<>(connection, Http1Exchange.this);
response.readHeaders();
- return response.response();
+ Response r = response.response();
+ buffer = response.getBuffer();
+ return r;
}, executor);
}
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Response.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Response.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
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 @@
return finished;
}
+ ByteBuffer getBuffer() {
+ return buffer;
+ }
+
int fixupContentLen(int clen) {
if (request.method().equalsIgnoreCase("HEAD")) {
return 0;
@@ -194,12 +198,15 @@
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();
- return connection.read(buffer);
+ buffer = connection.read();
+ if (buffer == null) {
+ return -1;
+ }
+ n = buffer.remaining();
}
return n;
}
@@ -207,18 +214,17 @@
String readStatusLine() throws IOException {
boolean cr = false;
StringBuilder statusLine = new StringBuilder(128);
- ByteBuffer b = buffer;
- while (getBuffer() != -1) {
- byte[] buf = b.array();
- int offset = b.position();
- int len = b.limit() - offset;
+ while ((obtainBuffer()) != -1) {
+ byte[] buf = buffer.array();
+ int offset = buffer.position();
+ int len = buffer.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 @@
}
}
// unlikely, but possible, that multiple reads required
- b.position(b.limit());
+ buffer.position(buffer.limit());
}
return null;
}
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpConnection.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpConnection.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
}
}
- 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 @@
*/
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();
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java Mon May 22 17:31:07 2017 +0100
@@ -311,8 +311,7 @@
}
}
- @Override
- protected int readImpl(ByteBuffer buf) throws IOException {
+ private int readImpl(ByteBuffer buf) throws IOException {
int mark = buf.position();
int n;
// FIXME: this hack works in conjunction with the corresponding change
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainTunnelingConnection.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainTunnelingConnection.java Mon May 22 17:31:07 2017 +0100
@@ -157,11 +157,6 @@
}
@Override
- protected int readImpl(ByteBuffer buffer) throws IOException {
- return delegate.readImpl(buffer);
- }
-
- @Override
boolean isSecure() {
return false;
}
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseContent.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseContent.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
// 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 = Utils.getBuffer();
- }
- chunkbuf.clear();
- connection.read(chunkbuf);
+ chunkbuf = connection.read();
}
}
@@ -256,7 +252,6 @@
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 @@
buffer.flip();
dataConsumer.accept(Optional.of(buffer));
}
- //client.returnBuffer(b);
while (remaining > 0) {
- ByteBuffer buffer = Utils.getBuffer();
- int xx = connection.read(buffer);
- if (xx == -1)
+ ByteBuffer buffer = connection.read();
+ if (buffer == null)
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;
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseHeaders.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseHeaders.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
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 @@
@Override
public int read() throws IOException {
if (!buffer.hasRemaining()) {
- buffer.clear();
- int n = connection.read(buffer);
- if (n == -1) {
+ buffer = connection.read();
+ if (buffer == null) {
return lastRead = -1;
}
}
@@ -97,6 +96,16 @@
}
}
+ 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 @@
// 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
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLConnection.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLConnection.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
@Override
protected ByteBuffer readImpl() throws IOException {
- ByteBuffer dst = ByteBuffer.allocate(8192);
- int n = readImpl(dst);
+ WrapperResult r = sslDelegate.recvData(ByteBuffer.allocate(8192));
+ // 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 {
@@ -174,19 +175,6 @@
}
@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();
}
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLDelegate.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLDelegate.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
}
SSLEngineResult result;
- /* if passed in buffer was not big enough then the a reallocated buffer
- * is returned here */
- ByteBuffer buf;
+ ByteBuffer buf; // buffer containing result data
}
int app_buf_size;
--- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLTunnelConnection.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLTunnelConnection.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
@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
- String s = "Receive) ";
- //debugPrint(s, r.buf);
- if (r.result.bytesProduced() > 0) {
- assert buf == r.buf;
- }
- return r.result.bytesProduced();
+ return r.buf;
}
@Override
--- a/jdk/test/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/ResponseHeadersTest.java Sun May 21 10:52:36 2017 +0200
+++ b/jdk/test/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/ResponseHeadersTest.java Mon May 22 17:31:07 2017 +0100
@@ -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 @@
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)