http-client-branch: fix issues with redirect including Upgrade header when it should not, Expect containing more than one value, and SECURE redirect policy
--- a/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java Mon Feb 26 18:16:46 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java Mon Feb 26 18:18:49 2018 +0000
@@ -305,6 +305,7 @@
} else {
exchange.serverauth = au;
}
+ req = HttpRequestImpl.newInstanceForAuthentication(req);
addBasicCredentials(req, proxy, pw);
return req;
} else if (au.retries > retry_limit) {
@@ -330,6 +331,7 @@
} else {
exchange.serverauth = au;
}
+ req = HttpRequestImpl.newInstanceForAuthentication(req);
addBasicCredentials(req, proxy, au.credentials);
au.retries++;
return req;
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java Mon Feb 26 18:16:46 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java Mon Feb 26 18:18:49 2018 +0000
@@ -121,14 +121,30 @@
this.authority = null;
}
- /** Creates a HttpRequestImpl using fields of an existing request impl. */
- public HttpRequestImpl(URI uri,
- String method,
- HttpRequestImpl other) {
+ /** Returns a new instance suitable for redirection. */
+ public static HttpRequestImpl newInstanceForRedirection(URI uri,
+ String method,
+ HttpRequestImpl other) {
+ return new HttpRequestImpl(uri, method, other);
+ }
+
+ /** Returns a new instance suitable for authentication. */
+ public static HttpRequestImpl newInstanceForAuthentication(HttpRequestImpl other) {
+ return new HttpRequestImpl(other.uri(), other.method(), other);
+ }
+
+ /**
+ * Creates a HttpRequestImpl using fields of an existing request impl.
+ * The newly created HttpRequestImpl does not copy the system headers.
+ */
+ private HttpRequestImpl(URI uri,
+ String method,
+ HttpRequestImpl other) {
this.method = method == null? "GET" : method;
this.userHeaders = other.userHeaders;
this.isWebSocket = other.isWebSocket;
- this.systemHeaders = other.systemHeaders;
+ this.systemHeaders = new HttpHeadersImpl();
+ this.systemHeaders.setHeader("User-Agent", USER_AGENT);
this.uri = uri;
this.proxy = other.proxy;
this.expectContinue = other.expectContinue;
--- a/src/java.net.http/share/classes/jdk/internal/net/http/RedirectFilter.java Mon Feb 26 18:16:46 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/RedirectFilter.java Mon Feb 26 18:18:49 2018 +0000
@@ -30,6 +30,7 @@
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpHeaders;
+import jdk.internal.net.http.common.Log;
import jdk.internal.net.http.common.Utils;
class RedirectFilter implements HeaderFilter {
@@ -66,7 +67,7 @@
}
/**
- * checks to see if new request needed and returns it.
+ * Checks to see if a new request is needed and returns it.
* Null means response is ok to return to user.
*/
private HttpRequestImpl handleResponse(Response r) {
@@ -76,11 +77,12 @@
}
if (rcode >= 300 && rcode <= 399) {
URI redir = getRedirectedURI(r.headers());
+ Log.logTrace("response code: {0}, redirected URI: {1}", rcode, redir);
if (canRedirect(redir) && ++exchange.numberOfRedirects < max_redirects) {
- //System.out.println("Redirecting to: " + redir);
- return new HttpRequestImpl(redir, method, request);
+ Log.logTrace("redirecting to: {0}", redir);
+ return HttpRequestImpl.newInstanceForRedirection(redir, method, request);
} else {
- //System.out.println("Redirect: giving up");
+ Log.logTrace("not redirecting");
return null;
}
}
@@ -109,7 +111,8 @@
case NEVER:
return false;
case SECURE:
- return newScheme.equalsIgnoreCase("https");
+ return newScheme.equalsIgnoreCase(oldScheme)
+ || newScheme.equalsIgnoreCase("https");
case SAME_PROTOCOL:
return newScheme.equalsIgnoreCase(oldScheme);
default:
--- a/test/jdk/java/net/httpclient/DigestEchoServer.java Mon Feb 26 18:16:46 2018 +0000
+++ b/test/jdk/java/net/httpclient/DigestEchoServer.java Mon Feb 26 18:18:49 2018 +0000
@@ -742,6 +742,14 @@
System.out.println(type + ": Got " + he.getRequestMethod()
+ ": " + he.getRequestURI()
+ "\n" + DigestEchoServer.toString(he.getRequestHeaders()));
+
+ // Assert only a single value for Expect. Not directly related
+ // to digest authentication, but verifies good client behaviour.
+ List<String> expectValues = he.getRequestHeaders().get("Expect");
+ if (expectValues != null && expectValues.size() > 1) {
+ throw new IOException("Expect: " + expectValues);
+ }
+
if (!isAuthentified(he)) {
try {
requestAuthentication(he);
--- a/test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java Mon Feb 26 18:16:46 2018 +0000
+++ b/test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java Mon Feb 26 18:18:49 2018 +0000
@@ -534,6 +534,18 @@
}
boolean endStreamReceived = endStream;
HttpHeadersImpl headers = decodeHeaders(frames);
+
+ // Strict to assert Client correctness. Not all servers are as strict,
+ // but some are known to be.
+ Optional<?> disallowedHeader = headers.firstValue("Upgrade");
+ if (disallowedHeader.isPresent()) {
+ throw new IOException("Unexpected Upgrade in headers:" + headers);
+ }
+ disallowedHeader = headers.firstValue("HTTP2-Settings");
+ if (disallowedHeader.isPresent())
+ throw new IOException("Unexpected HTTP2-Settings in headers:" + headers);
+
+
Queue q = new Queue(sentinel);
streams.put(streamid, q);
exec.submit(() -> {