# HG changeset patch # User chegar # Date 1519669129 0 # Node ID 261b5b524dd334d854307ad78d6864f8a91fccda # Parent f195e5048667fb53f65e16c778468a2a4d47b338 http-client-branch: fix issues with redirect including Upgrade header when it should not, Expect containing more than one value, and SECURE redirect policy diff -r f195e5048667 -r 261b5b524dd3 src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java --- 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; diff -r f195e5048667 -r 261b5b524dd3 src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java --- 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; diff -r f195e5048667 -r 261b5b524dd3 src/java.net.http/share/classes/jdk/internal/net/http/RedirectFilter.java --- 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: diff -r f195e5048667 -r 261b5b524dd3 test/jdk/java/net/httpclient/DigestEchoServer.java --- 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 expectValues = he.getRequestHeaders().get("Expect"); + if (expectValues != null && expectValues.size() > 1) { + throw new IOException("Expect: " + expectValues); + } + if (!isAuthentified(he)) { try { requestAuthentication(he); diff -r f195e5048667 -r 261b5b524dd3 test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java --- 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(() -> {