8232625: HttpClient redirect policy should be more conservative
authordfuchs
Wed, 23 Oct 2019 15:54:39 +0100
changeset 58758 2b13d126a2d8
parent 58756 b7aa58d7f5aa
child 58759 4242e35767b5
8232625: HttpClient redirect policy should be more conservative Summary: When enabled, HttpClient redirect is fixed to drop the body when the request method is changed, and to relay any redirection code it does not understand to the caller. Reviewed-by: chegar
src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java
src/java.net.http/share/classes/jdk/internal/net/http/RedirectFilter.java
test/jdk/java/net/httpclient/HttpRedirectTest.java
test/jdk/java/net/httpclient/http2/RedirectTest.java
test/jdk/java/net/httpclient/http2/server/Http2RedirectHandler.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java	Wed Oct 23 15:48:11 2019 +0200
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java	Wed Oct 23 15:54:39 2019 +0100
@@ -41,6 +41,7 @@
 import java.net.http.HttpClient;
 import java.net.http.HttpHeaders;
 import java.net.http.HttpRequest;
+
 import jdk.internal.net.http.common.HttpHeadersBuilder;
 import jdk.internal.net.http.common.Utils;
 import jdk.internal.net.http.websocket.OpeningHandshake;
@@ -152,13 +153,14 @@
     /** Returns a new instance suitable for redirection. */
     public static HttpRequestImpl newInstanceForRedirection(URI uri,
                                                             String method,
-                                                            HttpRequestImpl other) {
-        return new HttpRequestImpl(uri, method, other);
+                                                            HttpRequestImpl other,
+                                                            boolean mayHaveBody) {
+        return new HttpRequestImpl(uri, method, other, mayHaveBody);
     }
 
     /** Returns a new instance suitable for authentication. */
     public static HttpRequestImpl newInstanceForAuthentication(HttpRequestImpl other) {
-        HttpRequestImpl request = new HttpRequestImpl(other.uri(), other.method(), other);
+        HttpRequestImpl request = new HttpRequestImpl(other.uri(), other.method(), other, true);
         if (request.isWebSocket()) {
             Utils.setWebSocketUpgradeHeaders(request);
         }
@@ -171,7 +173,8 @@
      */
     private HttpRequestImpl(URI uri,
                             String method,
-                            HttpRequestImpl other) {
+                            HttpRequestImpl other,
+                            boolean mayHaveBody) {
         assert method == null || Utils.isValidName(method);
         this.method = method == null? "GET" : method;
         this.userHeaders = other.userHeaders;
@@ -184,13 +187,21 @@
         this.proxy = other.proxy;
         this.expectContinue = other.expectContinue;
         this.secure = uri.getScheme().toLowerCase(Locale.US).equals("https");
-        this.requestPublisher = other.requestPublisher;  // may be null
+        this.requestPublisher = mayHaveBody ? publisher(other) : null; // may be null
         this.acc = other.acc;
         this.timeout = other.timeout;
         this.version = other.version();
         this.authority = null;
     }
 
+    private BodyPublisher publisher(HttpRequestImpl other) {
+        BodyPublisher res = other.requestPublisher;
+        if (!Objects.equals(method, other.method)) {
+            res = null;
+        }
+        return res;
+    }
+
     /* used for creating CONNECT requests  */
     HttpRequestImpl(String method, InetSocketAddress authority, HttpHeaders headers) {
         // TODO: isWebSocket flag is not specified, but the assumption is that
--- a/src/java.net.http/share/classes/jdk/internal/net/http/RedirectFilter.java	Wed Oct 23 15:48:11 2019 +0200
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/RedirectFilter.java	Wed Oct 23 15:54:39 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -89,6 +89,31 @@
         }
     }
 
+    private static boolean isRedirecting(int statusCode) {
+        // < 300: not a redirect codes
+        if (statusCode < 300) return false;
+        // 309-399 Unassigned => don't follow
+        // > 399: not a redirect code
+        if (statusCode > 308) return false;
+        switch (statusCode) {
+            // 300: MultipleChoice => don't follow
+            case 300:
+                return false;
+            // 304: Not Modified => don't follow
+            case 304:
+                return false;
+            // 305: Proxy Redirect => don't follow.
+            case 305:
+                return false;
+            // 306: Unused => don't follow
+            case 306:
+                return false;
+            // 301, 302, 303, 307, 308: OK to follow.
+            default:
+                return true;
+        }
+    }
+
     /**
      * Checks to see if a new request is needed and returns it.
      * Null means response is ok to return to user.
@@ -102,13 +127,13 @@
         if (rcode == HTTP_NOT_MODIFIED)
             return null;
 
-        if (rcode >= 300 && rcode <= 399) {
+        if (isRedirecting(rcode)) {
             URI redir = getRedirectedURI(r.headers());
             String newMethod = redirectedMethod(rcode, method);
             Log.logTrace("response code: {0}, redirected URI: {1}", rcode, redir);
             if (canRedirect(redir) && ++exchange.numberOfRedirects < max_redirects) {
                 Log.logTrace("redirect to: {0} with method: {1}", redir, newMethod);
-                return HttpRequestImpl.newInstanceForRedirection(redir, newMethod, request);
+                return HttpRequestImpl.newInstanceForRedirection(redir, newMethod, request, rcode != 303);
             } else {
                 Log.logTrace("not redirecting");
                 return null;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/HttpRedirectTest.java	Wed Oct 23 15:54:39 2019 +0100
@@ -0,0 +1,457 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+import com.sun.net.httpserver.HttpServer;
+import com.sun.net.httpserver.HttpsConfigurator;
+import com.sun.net.httpserver.HttpsServer;
+import jdk.test.lib.net.SimpleSSLContext;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+import javax.net.ssl.SSLContext;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.Proxy;
+import java.net.ProxySelector;
+import java.net.SocketAddress;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * @test
+ * @bug 8232625
+ * @summary This test verifies that the HttpClient works correctly when redirecting a post request.
+ * @library /test/lib http2/server
+ * @build jdk.test.lib.net.SimpleSSLContext HttpServerAdapters DigestEchoServer HttpRedirectTest
+ * @modules java.net.http/jdk.internal.net.http.common
+ *          java.net.http/jdk.internal.net.http.frame
+ *          java.net.http/jdk.internal.net.http.hpack
+ *          java.logging
+ *          java.base/sun.net.www.http
+ *          java.base/sun.net.www
+ *          java.base/sun.net
+ * @run testng/othervm -Dtest.requiresHost=true
+ *                   -Djdk.httpclient.HttpClient.log=headers
+ *                   -Djdk.internal.httpclient.debug=false
+ *                   HttpRedirectTest
+ *
+ */
+public class HttpRedirectTest implements HttpServerAdapters {
+    static final String GET_RESPONSE_BODY = "Lorem ipsum dolor sit amet";
+    static final String REQUEST_BODY = "Here it goes";
+    static final SSLContext context;
+    static {
+        try {
+            context = new SimpleSSLContext().get();
+            SSLContext.setDefault(context);
+        } catch (Exception x) {
+            throw new ExceptionInInitializerError(x);
+        }
+    }
+
+    final AtomicLong requestCounter = new AtomicLong();
+    final AtomicLong responseCounter = new AtomicLong();
+    HttpTestServer http1Server;
+    HttpTestServer http2Server;
+    HttpTestServer https1Server;
+    HttpTestServer https2Server;
+    DigestEchoServer.TunnelingProxy proxy;
+
+    URI http1URI;
+    URI https1URI;
+    URI http2URI;
+    URI https2URI;
+    InetSocketAddress proxyAddress;
+    ProxySelector proxySelector;
+    HttpClient client;
+    List<CompletableFuture<?>>  futures = new CopyOnWriteArrayList<>();
+    Set<URI> pending = new CopyOnWriteArraySet<>();
+
+    final ExecutorService executor = new ThreadPoolExecutor(12, 60, 10,
+            TimeUnit.SECONDS, new LinkedBlockingQueue<>()); // Shared by HTTP/1.1 servers
+    final ExecutorService clientexec = new ThreadPoolExecutor(6, 12, 1,
+            TimeUnit.SECONDS, new LinkedBlockingQueue<>()); // Used by the client
+
+    public HttpClient newHttpClient(ProxySelector ps) {
+        HttpClient.Builder builder = HttpClient
+                .newBuilder()
+                .sslContext(context)
+                .executor(clientexec)
+                .followRedirects(HttpClient.Redirect.ALWAYS)
+                .proxy(ps);
+        return builder.build();
+    }
+
+    @DataProvider(name="uris")
+    Object[][] testURIs() throws URISyntaxException {
+        List<URI> uris = List.of(
+                http1URI.resolve("direct/orig/"),
+                https1URI.resolve("direct/orig/"),
+                https1URI.resolve("proxy/orig/"),
+                http2URI.resolve("direct/orig/"),
+                https2URI.resolve("direct/orig/"),
+                https2URI.resolve("proxy/orig/"));
+        List<Map.Entry<Integer, String>> redirects = List.of(
+                Map.entry(301, "GET"),
+                Map.entry(308, "POST"),
+                Map.entry(302, "GET"),
+                Map.entry(303, "GET"),
+                Map.entry(307, "POST"),
+                Map.entry(300, "DO_NOT_FOLLOW"),
+                Map.entry(304, "DO_NOT_FOLLOW"),
+                Map.entry(305, "DO_NOT_FOLLOW"),
+                Map.entry(306, "DO_NOT_FOLLOW"),
+                Map.entry(309, "DO_NOT_FOLLOW"),
+                Map.entry(new Random().nextInt(90) + 310, "DO_NOT_FOLLOW")
+        );
+        Object[][] tests = new Object[redirects.size() * uris.size()][3];
+        int count = 0;
+        for (int i=0; i < uris.size(); i++) {
+            URI u = uris.get(i);
+            for (int j=0; j < redirects.size() ; j++) {
+                int code = redirects.get(j).getKey();
+                String m = redirects.get(j).getValue();
+                tests[count][0] = u.resolve(code +"/");
+                tests[count][1] = code;
+                tests[count][2] = m;
+                count++;
+            }
+        }
+        return tests;
+    }
+
+    @BeforeClass
+    public void setUp() throws Exception {
+        try {
+            InetSocketAddress sa = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
+
+            // HTTP/1.1
+            HttpServer server1 = HttpServer.create(sa, 0);
+            server1.setExecutor(executor);
+            http1Server = HttpTestServer.of(server1);
+            http1Server.addHandler(new HttpTestRedirectHandler("http", http1Server),
+                    "/HttpRedirectTest/http1/");
+            http1Server.start();
+            http1URI = new URI("http://" + http1Server.serverAuthority() + "/HttpRedirectTest/http1/");
+
+
+            // HTTPS/1.1
+            HttpsServer sserver1 = HttpsServer.create(sa, 100);
+            sserver1.setExecutor(executor);
+            sserver1.setHttpsConfigurator(new HttpsConfigurator(context));
+            https1Server = HttpTestServer.of(sserver1);
+            https1Server.addHandler(new HttpTestRedirectHandler("https", https1Server),
+                    "/HttpRedirectTest/https1/");
+            https1Server.start();
+            https1URI = new URI("https://" + https1Server.serverAuthority() + "/HttpRedirectTest/https1/");
+
+            // HTTP/2.0
+            http2Server = HttpTestServer.of(
+                    new Http2TestServer("localhost", false, 0));
+            http2Server.addHandler(new HttpTestRedirectHandler("http", http2Server),
+                    "/HttpRedirectTest/http2/");
+            http2Server.start();
+            http2URI = new URI("http://" + http2Server.serverAuthority() + "/HttpRedirectTest/http2/");
+
+            // HTTPS/2.0
+            https2Server = HttpTestServer.of(
+                    new Http2TestServer("localhost", true, 0));
+            https2Server.addHandler(new HttpTestRedirectHandler("https", https2Server),
+                    "/HttpRedirectTest/https2/");
+            https2Server.start();
+            https2URI = new URI("https://" + https2Server.serverAuthority() + "/HttpRedirectTest/https2/");
+
+            proxy = DigestEchoServer.createHttpsProxyTunnel(
+                    DigestEchoServer.HttpAuthSchemeType.NONE);
+            proxyAddress = proxy.getProxyAddress();
+            proxySelector = new HttpProxySelector(proxyAddress);
+            client = newHttpClient(proxySelector);
+            System.out.println("Setup: done");
+        } catch (Exception x) {
+            tearDown(); throw x;
+        } catch (Error e) {
+            tearDown(); throw e;
+        }
+    }
+
+    private void testNonIdempotent(URI u, HttpRequest request,
+                                   int code, String method) throws Exception {
+        System.out.println("Testing with " + u);
+        CompletableFuture<HttpResponse<String>> respCf =
+                client.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+        HttpResponse<String> resp = respCf.join();
+        if (method.equals("DO_NOT_FOLLOW")) {
+            assertEquals(resp.statusCode(), code, u + ": status code");
+        } else {
+            assertEquals(resp.statusCode(), 200, u + ": status code");
+        }
+        if (method.equals("POST")) {
+            assertEquals(resp.body(), REQUEST_BODY, u + ": body");
+        } else if (code == 304) {
+            assertEquals(resp.body(), "", u + ": body");
+        } else if (method.equals("DO_NOT_FOLLOW")) {
+            assertNotEquals(resp.body(), GET_RESPONSE_BODY, u + ": body");
+            assertNotEquals(resp.body(), REQUEST_BODY, u + ": body");
+        } else {
+            assertEquals(resp.body(), GET_RESPONSE_BODY, u + ": body");
+        }
+    }
+
+    public void testIdempotent(URI u, HttpRequest request,
+                               int code, String method) throws Exception {
+        CompletableFuture<HttpResponse<String>> respCf =
+                client.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+        HttpResponse<String> resp = respCf.join();
+        if (method.equals("DO_NOT_FOLLOW")) {
+            assertEquals(resp.statusCode(), code, u + ": status code");
+        } else {
+            assertEquals(resp.statusCode(), 200, u + ": status code");
+        }
+        if (method.equals("POST")) {
+            assertEquals(resp.body(), REQUEST_BODY, u + ": body");
+        } else if (code == 304) {
+            assertEquals(resp.body(), "", u + ": body");
+        } else if (method.equals("DO_NOT_FOLLOW")) {
+            assertNotEquals(resp.body(), GET_RESPONSE_BODY, u + ": body");
+            assertNotEquals(resp.body(), REQUEST_BODY, u + ": body");
+        } else if (code == 303) {
+            assertEquals(resp.body(), GET_RESPONSE_BODY, u + ": body");
+        } else {
+            assertEquals(resp.body(), REQUEST_BODY, u + ": body");
+        }
+    }
+
+    @Test(dataProvider = "uris")
+    public void testPOST(URI uri, int code, String method) throws Exception {
+        URI u = uri.resolve("foo?n=" + requestCounter.incrementAndGet());
+        HttpRequest request = HttpRequest.newBuilder(u)
+                .POST(HttpRequest.BodyPublishers.ofString(REQUEST_BODY)).build();
+        // POST is not considered idempotent.
+        testNonIdempotent(u, request, code, method);
+    }
+
+    @Test(dataProvider = "uris")
+    public void testPUT(URI uri, int code, String method) throws Exception {
+        URI u = uri.resolve("foo?n=" + requestCounter.incrementAndGet());
+        System.out.println("Testing with " + u);
+        HttpRequest request = HttpRequest.newBuilder(u)
+                .PUT(HttpRequest.BodyPublishers.ofString(REQUEST_BODY)).build();
+        // PUT is considered idempotent.
+        testIdempotent(u, request, code, method);
+    }
+
+    @Test(dataProvider = "uris")
+    public void testFoo(URI uri, int code, String method) throws Exception {
+        URI u = uri.resolve("foo?n=" + requestCounter.incrementAndGet());
+        System.out.println("Testing with " + u);
+        HttpRequest request = HttpRequest.newBuilder(u)
+                .method("FOO",
+                        HttpRequest.BodyPublishers.ofString(REQUEST_BODY)).build();
+        // FOO is considered idempotent.
+        testIdempotent(u, request, code, method);
+    }
+
+    @Test(dataProvider = "uris")
+    public void testGet(URI uri, int code, String method) throws Exception {
+        URI u = uri.resolve("foo?n=" + requestCounter.incrementAndGet());
+        System.out.println("Testing with " + u);
+        HttpRequest request = HttpRequest.newBuilder(u)
+                .method("GET",
+                        HttpRequest.BodyPublishers.ofString(REQUEST_BODY)).build();
+        CompletableFuture<HttpResponse<String>> respCf =
+                client.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+        HttpResponse<String> resp = respCf.join();
+        // body will be preserved except for 304 and 303: this is a GET.
+        if (method.equals("DO_NOT_FOLLOW")) {
+            assertEquals(resp.statusCode(), code, u + ": status code");
+        } else {
+            assertEquals(resp.statusCode(), 200, u + ": status code");
+        }
+        if (code == 304) {
+            assertEquals(resp.body(), "", u + ": body");
+        } else if (method.equals("DO_NOT_FOLLOW")) {
+            assertNotEquals(resp.body(), GET_RESPONSE_BODY, u + ": body");
+            assertNotEquals(resp.body(), REQUEST_BODY, u + ": body");
+        } else if (code == 303) {
+            assertEquals(resp.body(), GET_RESPONSE_BODY, u + ": body");
+        } else {
+            assertEquals(resp.body(), REQUEST_BODY, u + ": body");
+        }
+    }
+
+    @AfterClass
+    public void tearDown() {
+        proxy = stop(proxy, DigestEchoServer.TunnelingProxy::stop);
+        http1Server = stop(http1Server, HttpTestServer::stop);
+        https1Server = stop(https1Server, HttpTestServer::stop);
+        http2Server = stop(http2Server, HttpTestServer::stop);
+        https2Server = stop(https2Server, HttpTestServer::stop);
+        client = null;
+        try {
+            executor.awaitTermination(2000, TimeUnit.MILLISECONDS);
+        } catch (Throwable x) {
+        } finally {
+            executor.shutdownNow();
+        }
+        try {
+            clientexec.awaitTermination(2000, TimeUnit.MILLISECONDS);
+        } catch (Throwable x) {
+        } finally {
+            clientexec.shutdownNow();
+        }
+        System.out.println("Teardown: done");
+    }
+
+    private interface Stoppable<T> { public void stop(T service) throws Exception; }
+
+    static <T>  T stop(T service, Stoppable<T> stop) {
+        try { if (service != null) stop.stop(service); } catch (Throwable x) { };
+        return null;
+    }
+
+    static class HttpProxySelector extends ProxySelector {
+        private static final List<Proxy> NO_PROXY = List.of(Proxy.NO_PROXY);
+        private final List<Proxy> proxyList;
+        HttpProxySelector(InetSocketAddress proxyAddress) {
+            proxyList = List.of(new Proxy(Proxy.Type.HTTP, proxyAddress));
+        }
+
+        @Override
+        public List<Proxy> select(URI uri) {
+            // our proxy only supports tunneling
+            if (uri.getScheme().equalsIgnoreCase("https")) {
+                if (uri.getPath().contains("/proxy/")) {
+                    return proxyList;
+                }
+            }
+            return NO_PROXY;
+        }
+
+        @Override
+        public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
+            System.err.println("Connection to proxy failed: " + ioe);
+            System.err.println("Proxy: " + sa);
+            System.err.println("\tURI: " + uri);
+            ioe.printStackTrace();
+        }
+    }
+
+    public static class HttpTestRedirectHandler implements HttpTestHandler {
+        static final AtomicLong respCounter = new AtomicLong();
+        final String scheme;
+        final HttpTestServer server;
+        HttpTestRedirectHandler(String scheme, HttpTestServer server) {
+            this.scheme = scheme;
+            this.server = server;
+        }
+
+        @Override
+        public void handle(HttpTestExchange t) throws IOException {
+            try (InputStream is = t.getRequestBody()) {
+                byte[] bytes = is.readAllBytes();
+                URI u = t.getRequestURI();
+                long responseID = Long.parseLong(u.getQuery().substring(2));
+                String path = u.getPath();
+                int i = path.lastIndexOf('/');
+                String file = path.substring(i+1);
+                String parent =  path.substring(0, i);
+                int code = 200;
+                if (file.equals("foo")) {
+                    i = parent.lastIndexOf("/");
+                    code = Integer.parseInt(parent.substring(i+1));
+                }
+                String response;
+                if (code == 200) {
+                    if (t.getRequestMethod().equals("GET")) {
+                        if (bytes.length == 0) {
+                            response = GET_RESPONSE_BODY;
+                        } else {
+                            response = new String(bytes, StandardCharsets.UTF_8);
+                        }
+                    } else if (t.getRequestMethod().equals("POST")) {
+                        response = new String(bytes, StandardCharsets.UTF_8);
+                    } else {
+                        response = new String(bytes, StandardCharsets.UTF_8);
+                    }
+                } else if (code < 300 || code > 399) {
+                    response = "Unexpected code: " + code;
+                    code = 400;
+                } else {
+                    try {
+                        URI reloc = new URI(scheme, server.serverAuthority(), parent + "/bar", u.getQuery(), null);
+                        t.getResponseHeaders().addHeader("Location", reloc.toASCIIString());
+                        if (code != 304) {
+                            response = "Code: " + code;
+                        } else response = null;
+                    } catch (URISyntaxException x) {
+                        x.printStackTrace();
+                        x.printStackTrace(System.out);
+                        code = 400;
+                        response = x.toString();
+                    }
+                }
+
+                System.out.println("Server " + t.getRequestURI() + " sending response " + responseID);
+                System.out.println("code: " + code + " body: " + response);
+                t.sendResponseHeaders(code, code == 304 ? 0: -1);
+                if (code != 304) {
+                    try (OutputStream os = t.getResponseBody()) {
+                        bytes = response.getBytes(StandardCharsets.UTF_8);
+                        os.write(bytes);
+                        os.flush();
+                    }
+                } else {
+                    bytes = new byte[0];
+                }
+
+                System.out.println("\tresp:" + responseID + ": wrote " + bytes.length + " bytes");
+            } catch (Throwable e) {
+                e.printStackTrace();
+                e.printStackTrace(System.out);
+                throw e;
+            }
+        }
+    }
+
+}
--- a/test/jdk/java/net/httpclient/http2/RedirectTest.java	Wed Oct 23 15:48:11 2019 +0200
+++ b/test/jdk/java/net/httpclient/http2/RedirectTest.java	Wed Oct 23 15:54:39 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -87,6 +87,11 @@
             }
         }
 
+        @Override
+        protected int redirectCode() {
+            return 308; // we need to use a code that preserves the body
+        }
+
         public synchronized boolean error() {
             return error;
         }
--- a/test/jdk/java/net/httpclient/http2/server/Http2RedirectHandler.java	Wed Oct 23 15:48:11 2019 +0200
+++ b/test/jdk/java/net/httpclient/http2/server/Http2RedirectHandler.java	Wed Oct 23 15:54:39 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -42,11 +42,11 @@
             is.readAllBytes();
             String location = supplier.get();
             System.err.printf("RedirectHandler request to %s from %s\n",
-                t.getRequestURI().toString(), t.getRemoteAddress().toString());
+                    t.getRequestURI().toString(), t.getRemoteAddress().toString());
             System.err.println("Redirecting to: " + location);
             HttpHeadersBuilder headersBuilder = t.getResponseHeaders();
             headersBuilder.addHeader("Location", location);
-            t.sendResponseHeaders(301, 1024);
+            t.sendResponseHeaders(redirectCode(), 1024);
             byte[] bb = new byte[1024];
             OutputStream os = t.getResponseBody();
             os.write(bb);
@@ -55,6 +55,10 @@
         }
     }
 
+    protected int redirectCode() {
+        return 301;
+    }
+
     // override in sub-class to examine the exchange, but don't
     // alter transaction state by reading the request body etc.
     protected void examineExchange(Http2TestExchange t) {