From f2cab46fa8a6470ad51d5e9a580de3c37e8c9d5c Mon Sep 17 00:00:00 2001 From: Andrew Lu Date: Thu, 1 Aug 2024 02:19:26 +0000 Subject: [PATCH 1/8] 8298873: Update IllegalRecordVersion.java for changes to TLS implementation 8301189: validate-source fails after JDK-8298873 Backport-of: 28adafcb524a043eca0fc6e7f9a1bb2a5490d723 --- test/jdk/ProblemList.txt | 1 - .../HandshakeWithInvalidRecordVersion.java | 236 ++++++++++++++++++ .../ssl/SSLEngine/IllegalRecordVersion.java | 77 ------ 3 files changed, 236 insertions(+), 78 deletions(-) create mode 100644 test/jdk/javax/net/ssl/SSLEngine/HandshakeWithInvalidRecordVersion.java delete mode 100644 test/jdk/javax/net/ssl/SSLEngine/IllegalRecordVersion.java diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index 4d1fdf42cb8..283db3d269f 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -609,7 +609,6 @@ sun/security/pkcs11/KeyStore/ClientAuth.sh 8254806 solaris- sun/security/pkcs11/sslecc/ClientJSSEServerJSSE.java 8161536 generic-all javax/net/ssl/SSLEngine/TestAllSuites.java 8298874 generic-all -javax/net/ssl/SSLEngine/IllegalRecordVersion.java 8298873 generic-all javax/net/ssl/SSLEngine/EngineCloseOnAlert.java 8298868 generic-all javax/net/ssl/SSLEngine/ConnectionTest.java 8298869 generic-all javax/net/ssl/SSLEngine/CheckStatus.java 8298872 generic-all diff --git a/test/jdk/javax/net/ssl/SSLEngine/HandshakeWithInvalidRecordVersion.java b/test/jdk/javax/net/ssl/SSLEngine/HandshakeWithInvalidRecordVersion.java new file mode 100644 index 00000000000..71119489b85 --- /dev/null +++ b/test/jdk/javax/net/ssl/SSLEngine/HandshakeWithInvalidRecordVersion.java @@ -0,0 +1,236 @@ +/* + * Copyright (c) 2014, 2023, 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. + */ + +/* + * @test + * @bug 8042449 8299870 + * @library /javax/net/ssl/templates + * @summary Verify successful handshake ignores invalid record version + * + * @run main/timeout=300 HandshakeWithInvalidRecordVersion + */ + +import javax.net.ssl.*; +import javax.net.ssl.SSLEngineResult.*; +import java.io.*; +import java.security.*; +import java.nio.*; +import java.util.Arrays; + +public class HandshakeWithInvalidRecordVersion implements SSLContextTemplate { + private static final boolean DEBUG = Boolean.getBoolean("test.debug"); + + private static final String PATH_TO_STORES = "../etc"; + private static final String KEYSTORE_FILE = "keystore"; + private static final String TRUSTSTORE_FILE = "truststore"; + + private static final String KEYSTORE_PATH = + System.getProperty("test.src", "./") + "/" + PATH_TO_STORES + + "/" + KEYSTORE_FILE; + private static final String TRUSTSTORE_PATH = + System.getProperty("test.src", "./") + "/" + PATH_TO_STORES + + "/" + TRUSTSTORE_FILE; + + public static void main(String [] args) throws Exception { + var runner = new HandshakeWithInvalidRecordVersion(); + runner.executeTest("TLSv1.2", + new String[]{"TLSv1.2"}, new String[]{"TLSv1.3", "TLSv1.2"}); + + runner.executeTest("TLSv1.2", + new String[]{"TLSv1.3", "TLSv1.2"}, new String[]{"TLSv1.2"}); + + runner.executeTest("TLSv1.3", + new String[]{"TLSv1.2", "TLSv1.3"}, new String[]{"TLSv1.3"}); + + runner.executeTest("TLSv1.3", + new String[]{"TLSv1.3"}, new String[]{"TLSv1.2", "TLSv1.3"}); + } + + + private void executeTest(String expectedProtocol, String[] clientProtocols, + String[] serverProtocols) throws Exception { + System.out.printf("Executing test%n" + + "Client protocols: %s%nServer protocols: %s%nExpected negotiated: %s%n", + Arrays.toString(clientProtocols), Arrays.toString(serverProtocols), + expectedProtocol); + + SSLEngine cliEngine = createClientSSLContext().createSSLEngine(); + cliEngine.setUseClientMode(true); + cliEngine.setEnabledProtocols(clientProtocols); + SSLEngine srvEngine = createServerSSLContext().createSSLEngine(); + srvEngine.setUseClientMode(false); + srvEngine.setEnabledProtocols(serverProtocols); + + SSLSession session = cliEngine.getSession(); + int netBufferMax = session.getPacketBufferSize(); + int appBufferMax = session.getApplicationBufferSize(); + + ByteBuffer cliToSrv = ByteBuffer.allocateDirect(netBufferMax); + ByteBuffer srvIBuff = ByteBuffer.allocateDirect(appBufferMax + 50); + ByteBuffer cliOBuff = ByteBuffer.wrap("I'm client".getBytes()); + + + System.out.println("Generating ClientHello"); + SSLEngineResult cliRes = cliEngine.wrap(cliOBuff, cliToSrv); + checkResult(cliRes, HandshakeStatus.NEED_UNWRAP); + log("Client wrap result: " + cliRes); + cliToSrv.flip(); + if (cliToSrv.limit() > 5) { + System.out.println("Setting record version to (0xa9, 0xa2)"); + cliToSrv.put(1, (byte)0xa9); + cliToSrv.put(2, (byte)0xa2); + } else { + throw new RuntimeException("ClientHello message is only " + + cliToSrv.limit() + "bytes. Expecting at least 6 bytes. "); + } + + System.out.println("Processing ClientHello"); + SSLEngineResult srv = srvEngine.unwrap(cliToSrv, srvIBuff); + checkResult(srv, HandshakeStatus.NEED_TASK); + runDelegatedTasks(srvEngine); + + finishHandshake(cliEngine, srvEngine); + + if (!cliEngine.getSession().getProtocol() + .equals(srvEngine.getSession().getProtocol()) + || !cliEngine.getSession().getProtocol().equals(expectedProtocol)) { + throw new RuntimeException("Client and server did not negotiate protocol. " + + "Expected: " + expectedProtocol + ". Negotiated: " + + cliEngine.getSession().getProtocol()); + } + } + private boolean isHandshaking(SSLEngine e) { + return (e.getHandshakeStatus() != HandshakeStatus.NOT_HANDSHAKING); + } + + private void finishHandshake(SSLEngine client, SSLEngine server) throws Exception { + boolean clientDone = false; + boolean serverDone = false; + SSLEngineResult serverResult; + SSLEngineResult clientResult; + int capacity = client.getSession().getPacketBufferSize(); + ByteBuffer emptyBuffer = ByteBuffer.allocate(capacity); + ByteBuffer serverToClient = ByteBuffer.allocate(capacity); + ByteBuffer clientToServer = ByteBuffer.allocate(capacity); + + System.out.println("Finishing handshake..."); + while (isHandshaking(client) || + isHandshaking(server)) { + + log("================"); + + clientResult = client.wrap(emptyBuffer, clientToServer); + serverResult = server.wrap(emptyBuffer, serverToClient); + + if (clientResult.getHandshakeStatus() == HandshakeStatus.FINISHED) { + clientDone = true; + } + + if (serverResult.getHandshakeStatus() == HandshakeStatus.FINISHED) { + serverDone = true; + } + + log("wrap1 = " + clientResult); + log("wrap2 = " + serverResult); + + if (clientResult.getHandshakeStatus() == HandshakeStatus.NEED_TASK) { + Runnable runnable; + while ((runnable = client.getDelegatedTask()) != null) { + runnable.run(); + } + } + + if (serverResult.getHandshakeStatus() == HandshakeStatus.NEED_TASK) { + Runnable runnable; + while ((runnable = server.getDelegatedTask()) != null) { + runnable.run(); + } + } + + clientToServer.flip(); + serverToClient.flip(); + + log("----"); + + clientResult = client.unwrap(serverToClient, emptyBuffer); + serverResult = server.unwrap(clientToServer, emptyBuffer); + + if (clientResult.getHandshakeStatus() == HandshakeStatus.FINISHED) { + clientDone = true; + } + + if (serverResult.getHandshakeStatus() == HandshakeStatus.FINISHED) { + serverDone = true; + } + + log("unwrap1 = " + clientResult); + log("unwrap2 = " + serverResult); + + if (clientResult.getHandshakeStatus() == HandshakeStatus.NEED_TASK) { + Runnable runnable; + while ((runnable = client.getDelegatedTask()) != null) { + runnable.run(); + } + } + + if (serverResult.getHandshakeStatus() == HandshakeStatus.NEED_TASK) { + Runnable runnable; + while ((runnable = server.getDelegatedTask()) != null) { + runnable.run(); + } + } + + clientToServer.clear(); + serverToClient.clear(); + } + + System.out.println("Handshake complete"); + + if (!clientDone || !serverDone) { + throw new RuntimeException("Both should be true:\n" + + " clientDone = " + clientDone + " serverDone = " + serverDone); + } + } + + private static void runDelegatedTasks(SSLEngine engine) { + Runnable runnable; + while ((runnable = engine.getDelegatedTask()) != null) { + log("\trunning delegated task..."); + runnable.run(); + } + } + + private static void checkResult(SSLEngineResult result, HandshakeStatus expectedStatus) { + if(result.getHandshakeStatus() != expectedStatus) { + throw new RuntimeException(String.format( + "Handshake status %s does not match expected status of %s", + result.getHandshakeStatus(), expectedStatus)); + } + } + + private static void log(Object msg) { + if (DEBUG) { + System.out.println(msg); + } + } +} diff --git a/test/jdk/javax/net/ssl/SSLEngine/IllegalRecordVersion.java b/test/jdk/javax/net/ssl/SSLEngine/IllegalRecordVersion.java deleted file mode 100644 index cab26528790..00000000000 --- a/test/jdk/javax/net/ssl/SSLEngine/IllegalRecordVersion.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (c) 2014, 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. - */ - -// This test case relies on updated static security property, no way to re-use -// security property in samevm/agentvm mode. - -/* - * @test - * @bug 8042449 - * @summary Issue for negative byte major record version - * - * @run main/othervm IllegalRecordVersion - */ - -import javax.net.ssl.*; -import javax.net.ssl.SSLEngineResult.*; -import java.io.*; -import java.security.*; -import java.nio.*; - -public class IllegalRecordVersion { - - public static void main(String args[]) throws Exception { - SSLContext context = SSLContext.getDefault(); - - SSLEngine cliEngine = context.createSSLEngine(); - cliEngine.setUseClientMode(true); - SSLEngine srvEngine = context.createSSLEngine(); - srvEngine.setUseClientMode(false); - - SSLSession session = cliEngine.getSession(); - int netBufferMax = session.getPacketBufferSize(); - int appBufferMax = session.getApplicationBufferSize(); - - ByteBuffer cliToSrv = ByteBuffer.allocateDirect(netBufferMax); - ByteBuffer srvIBuff = ByteBuffer.allocateDirect(appBufferMax + 50); - ByteBuffer cliOBuff = ByteBuffer.wrap("I'm client".getBytes()); - - - System.out.println("client hello (record version(0xa9, 0xa2))"); - SSLEngineResult cliRes = cliEngine.wrap(cliOBuff, cliToSrv); - System.out.println("Client wrap result: " + cliRes); - cliToSrv.flip(); - if (cliToSrv.limit() > 5) { - cliToSrv.put(1, (byte)0xa9); - cliToSrv.put(2, (byte)0xa2); - } - - try { - srvEngine.unwrap(cliToSrv, srvIBuff); - throw new Exception( - "Cannot catch the unsupported record version issue"); - } catch (SSLException e) { - // get the expected exception - } - } -} From 4f62849d1fd5629358540bbcf678682419874dad Mon Sep 17 00:00:00 2001 From: Andrew Lu Date: Thu, 1 Aug 2024 02:19:51 +0000 Subject: [PATCH 2/8] 8259274: Increase timeout duration in sun/nio/ch/TestMaxCachedBufferSize.java Backport-of: 2659bc449c8f09a1b9a5e336bd437e8427979fb8 --- test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java b/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java index 67520f27989..930e0408e6c 100644 --- a/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java +++ b/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2021, 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,11 +44,11 @@ * @modules java.management * @library /test/lib * @build TestMaxCachedBufferSize - * @run main/othervm TestMaxCachedBufferSize - * @run main/othervm -Djdk.nio.maxCachedBufferSize=0 TestMaxCachedBufferSize - * @run main/othervm -Djdk.nio.maxCachedBufferSize=2000 TestMaxCachedBufferSize - * @run main/othervm -Djdk.nio.maxCachedBufferSize=100000 TestMaxCachedBufferSize - * @run main/othervm -Djdk.nio.maxCachedBufferSize=10000000 TestMaxCachedBufferSize + * @run main/othervm/timeout=150 TestMaxCachedBufferSize + * @run main/othervm/timeout=150 -Djdk.nio.maxCachedBufferSize=0 TestMaxCachedBufferSize + * @run main/othervm/timeout=150 -Djdk.nio.maxCachedBufferSize=2000 TestMaxCachedBufferSize + * @run main/othervm/timeout=150 -Djdk.nio.maxCachedBufferSize=100000 TestMaxCachedBufferSize + * @run main/othervm/timeout=150 -Djdk.nio.maxCachedBufferSize=10000000 TestMaxCachedBufferSize * @summary Test the implementation of the jdk.nio.maxCachedBufferSize property * (use -Dseed=X to set PRNG seed) * @key randomness From fe0c28354405784f6c8ce8d9ef03380fd1d80717 Mon Sep 17 00:00:00 2001 From: Andrew Lu Date: Thu, 1 Aug 2024 02:20:19 +0000 Subject: [PATCH 3/8] 8255913: Decrease number of iterations in TestMaxCachedBufferSize Backport-of: d6f094040085fb16306fd1e0eea7d8d7969cdb4d --- test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java b/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java index 930e0408e6c..ef73a15256a 100644 --- a/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java +++ b/test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java @@ -54,7 +54,7 @@ * @key randomness */ public class TestMaxCachedBufferSize { - private static final int DEFAULT_ITERS = 10 * 1000; + private static final int DEFAULT_ITERS = 5 * 1000; private static final int DEFAULT_THREAD_NUM = 4; private static final int SMALL_BUFFER_MIN_SIZE = 4 * 1024; From 555a529969ab82aac69319440c25016aa756b8e1 Mon Sep 17 00:00:00 2001 From: Alexander Pepin <89089666+sashapepin@users.noreply.github.com> Date: Thu, 1 Aug 2024 09:47:01 +0000 Subject: [PATCH 4/8] 8334711: [TEST_BUG] Compilation failed of MimeFormatsTest/MimeFormatsTest.java Reviewed-by: yan --- test/jdk/java/awt/datatransfer/MimeFormatsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jdk/java/awt/datatransfer/MimeFormatsTest.java b/test/jdk/java/awt/datatransfer/MimeFormatsTest.java index a22cc47877d..6f4f1e592de 100644 --- a/test/jdk/java/awt/datatransfer/MimeFormatsTest.java +++ b/test/jdk/java/awt/datatransfer/MimeFormatsTest.java @@ -103,7 +103,7 @@ public void childRun() { ClipboardOwner owner = new ClipboardOwner() { public void lostOwnership(Clipboard clipboard, Transferable contents) { - System.err.println("%d exit".formatted( + System.err.println(String.format("%d exit", System.currentTimeMillis())); System.err.println("Exiting"); System.exit(0); @@ -115,7 +115,7 @@ public void lostOwnership(Clipboard clipboard, synchronized (lock) { // Wait to let the parent retrieve the contents. try { - System.err.println("%d wait".formatted( + System.err.println(String.format("%d wait", System.currentTimeMillis())); lock.wait(); } catch (InterruptedException e) { From 99dc660cf6194ca8a59e750e2404d334e9831ed2 Mon Sep 17 00:00:00 2001 From: Andrew Lu Date: Mon, 5 Aug 2024 02:13:33 +0000 Subject: [PATCH 5/8] 8317343: GC: Make TestHeapFreeRatio use createTestJvm Backport-of: c64bd3d6715304accd9a1e3266edd9d3d2353273 --- test/hotspot/jtreg/gc/arguments/TestHeapFreeRatio.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/hotspot/jtreg/gc/arguments/TestHeapFreeRatio.java b/test/hotspot/jtreg/gc/arguments/TestHeapFreeRatio.java index ad33f5b7a7f..137172509ba 100644 --- a/test/hotspot/jtreg/gc/arguments/TestHeapFreeRatio.java +++ b/test/hotspot/jtreg/gc/arguments/TestHeapFreeRatio.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2023, 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 @@ -28,6 +28,7 @@ * @key gc * @bug 8025661 * @summary Test parsing of -Xminf and -Xmaxf + * @requires vm.opt.x.Xminf == null & vm.opt.x.Xmaxf == null & vm.opt.MinHeapFreeRatio == null & vm.opt.MaxHeapFreeRatio == null * @library /test/lib * @library / * @modules java.base/jdk.internal.misc @@ -49,7 +50,7 @@ enum Validation { } private static void testMinMaxFreeRatio(String min, String max, Validation type) throws Exception { - ProcessBuilder pb = GCArguments.createJavaProcessBuilder( + ProcessBuilder pb = GCArguments.createTestJvm( "-Xminf" + min, "-Xmaxf" + max, "-version"); From 078a32378e9f2b21673ce4f813f2747bb2a7344c Mon Sep 17 00:00:00 2001 From: Andrew Lu Date: Tue, 6 Aug 2024 02:13:24 +0000 Subject: [PATCH 6/8] 8317358: G1: Make TestMaxNewSize use createTestJvm Backport-of: 1a098356dd3a157b12c2b5c527e61c8a628bdb2d --- .../jtreg/gc/arguments/TestMaxNewSize.java | 40 +++++-------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/test/hotspot/jtreg/gc/arguments/TestMaxNewSize.java b/test/hotspot/jtreg/gc/arguments/TestMaxNewSize.java index 7c10e1e27a2..cdfd0e829f8 100644 --- a/test/hotspot/jtreg/gc/arguments/TestMaxNewSize.java +++ b/test/hotspot/jtreg/gc/arguments/TestMaxNewSize.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2023, 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 @@ -29,7 +29,8 @@ * @bug 7057939 * @summary Make sure that MaxNewSize always has a useful value after argument * processing. - * @requires vm.gc.Serial + * @key flag-sensitive + * @requires vm.gc.Serial & vm.opt.MaxNewSize == null & vm.opt.NewRatio == null & vm.opt.NewSize == null & vm.opt.OldSize == null & vm.opt.x.Xms == null & vm.opt.x.Xmx == null * @library /test/lib * @library / * @modules java.base/jdk.internal.misc @@ -44,7 +45,8 @@ * @bug 7057939 * @summary Make sure that MaxNewSize always has a useful value after argument * processing. - * @requires vm.gc.Parallel + * @key flag-sensitive + * @requires vm.gc.Parallel & vm.opt.MaxNewSize == null & vm.opt.NewRatio == null & vm.opt.NewSize == null & vm.opt.OldSize == null & vm.opt.x.Xms == null & vm.opt.x.Xmx == null * @library /test/lib * @library / * @modules java.base/jdk.internal.misc @@ -59,7 +61,8 @@ * @bug 7057939 * @summary Make sure that MaxNewSize always has a useful value after argument * processing. - * @requires vm.gc.G1 + * @key flag-sensitive + * @requires vm.gc.G1 & vm.opt.MaxNewSize == null & vm.opt.NewRatio == null & vm.opt.NewSize == null & vm.opt.OldSize == null & vm.opt.x.Xms == null & vm.opt.x.Xmx == null * @library /test/lib * @library / * @modules java.base/jdk.internal.misc @@ -96,46 +99,23 @@ public class TestMaxNewSize { private static void checkMaxNewSize(String[] flags, int heapsize) throws Exception { BigInteger actual = new BigInteger(getMaxNewSize(flags)); - System.out.println(actual); - if (actual.compareTo(new BigInteger((new Long(heapsize)).toString())) == 1) { + System.out.println("asserting: " + actual + " <= " + heapsize); + if (actual.compareTo(new BigInteger("" + heapsize)) > 0) { throw new RuntimeException("MaxNewSize value set to \"" + actual + "\", expected otherwise when running with the following flags: " + Arrays.asList(flags).toString()); } } - private static void checkIncompatibleNewSize(String[] flags) throws Exception { - ArrayList finalargs = new ArrayList(); - finalargs.addAll(Arrays.asList(flags)); - finalargs.add("-version"); - - ProcessBuilder pb = GCArguments.createJavaProcessBuilder(finalargs); - OutputAnalyzer output = new OutputAnalyzer(pb.start()); - output.shouldContain("Initial young gen size set larger than the maximum young gen size"); - } - - private static boolean isRunningG1(String[] args) { - for (int i = 0; i < args.length; i++) { - if (args[i].contains("+UseG1GC")) { - return true; - } - } - return false; - } - private static String getMaxNewSize(String[] flags) throws Exception { ArrayList finalargs = new ArrayList(); finalargs.addAll(Arrays.asList(flags)); - if (isRunningG1(flags)) { - finalargs.add("-XX:G1HeapRegionSize=1M"); - } finalargs.add("-XX:+PrintFlagsFinal"); finalargs.add("-version"); - ProcessBuilder pb = GCArguments.createJavaProcessBuilder(finalargs); + ProcessBuilder pb = GCArguments.createTestJvm(finalargs); OutputAnalyzer output = new OutputAnalyzer(pb.start()); output.shouldHaveExitValue(0); String stdout = output.getStdout(); - //System.out.println(stdout); return getFlagValue("MaxNewSize", stdout); } From 5cedb3d16efe5dea6cdcf4d8f6cd864a2e867d08 Mon Sep 17 00:00:00 2001 From: Andrew Lu Date: Tue, 6 Aug 2024 02:13:49 +0000 Subject: [PATCH 7/8] 8336301: test/jdk/java/nio/channels/AsyncCloseAndInterrupt.java leaves around a FIFO file upon test completion Backport-of: ae9f318fc35eeab497e546ebab9faed6ec774ec5 --- test/jdk/java/nio/channels/AsyncCloseAndInterrupt.java | 1 + 1 file changed, 1 insertion(+) diff --git a/test/jdk/java/nio/channels/AsyncCloseAndInterrupt.java b/test/jdk/java/nio/channels/AsyncCloseAndInterrupt.java index 31f87137838..6bf46a8aa56 100644 --- a/test/jdk/java/nio/channels/AsyncCloseAndInterrupt.java +++ b/test/jdk/java/nio/channels/AsyncCloseAndInterrupt.java @@ -129,6 +129,7 @@ private static void initFile() throws Exception { return; } fifoFile = new File("x.fifo"); + fifoFile.deleteOnExit(); if (fifoFile.exists()) { if (!fifoFile.delete()) throw new IOException("Cannot delete existing fifo " + fifoFile); From bf9ba35e05bcbf9d4c1c8041d6be46d0092fdfc2 Mon Sep 17 00:00:00 2001 From: Matthias Baesken Date: Tue, 6 Aug 2024 07:05:56 +0000 Subject: [PATCH 8/8] 8229822: ThrowingPushPromises tests sometimes fail due to EOF SocketTube is fixed to cater for errors caused by pausing/resuming events on an asynchronously closed connection, from within the selector's manager thread. Http2Connection and Stream are fixed to prevent sending a DataFrame on a stream after Reset has been sent. Backport-of: 77c46ea9112b0c2632b4af1d899d59a132878da3 --- .../internal/net/http/Http2Connection.java | 44 ++++-- .../jdk/internal/net/http/SocketTube.java | 13 +- .../classes/jdk/internal/net/http/Stream.java | 134 +++++++++++++++--- ...rowingPushPromisesAsInputStreamCustom.java | 1 + .../ThrowingPushPromisesAsInputStreamIO.java | 1 + .../ThrowingPushPromisesAsLinesCustom.java | 1 + .../ThrowingPushPromisesAsLinesIO.java | 1 + .../ThrowingPushPromisesAsStringCustom.java | 1 + .../ThrowingPushPromisesAsStringIO.java | 1 + .../ThrowingPushPromisesSanity.java | 1 + 10 files changed, 169 insertions(+), 29 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java index c5975666407..100b6a2bc2e 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java @@ -329,7 +329,11 @@ private Http2Connection(HttpConnection connection, Log.logTrace("Connection send window size {0} ", windowController.connectionWindowSize()); Stream initialStream = createStream(exchange); - initialStream.registerStream(1); + boolean opened = initialStream.registerStream(1, true); + if (debug.on() && !opened) { + debug.log("Initial stream was cancelled - but connection is maintained: " + + "reset frame will need to be sent later"); + } windowController.registerStream(1, getInitialSendWindowSize()); initialStream.requestSent(); // Upgrading: @@ -338,6 +342,11 @@ private Http2Connection(HttpConnection connection, this.initial = initial; connectFlows(connection); sendConnectionPreface(); + if (!opened) { + debug.log("ensure reset frame is sent to cancel initial stream"); + initialStream.sendCancelStreamFrame(); + } + } // Used when upgrading an HTTP/1.1 connection to HTTP/2 after receiving @@ -847,7 +856,7 @@ private void handlePushPromise(Stream parent, PushPromiseFrame pp) Exchange pushExch = new Exchange<>(pushReq, parent.exchange.multi); Stream.PushedStream pushStream = createPushStream(parent, pushExch); pushExch.exchImpl = pushStream; - pushStream.registerStream(promisedStreamid); + pushStream.registerStream(promisedStreamid, true); parent.incoming_pushPromise(pushReq, pushStream); } @@ -872,7 +881,7 @@ private void handleConnectionFrame(Http2Frame frame) } } - void resetStream(int streamid, int code) throws IOException { + void resetStream(int streamid, int code) { try { if (connection.channel().isOpen()) { // no need to try & send a reset frame if the @@ -880,6 +889,7 @@ void resetStream(int streamid, int code) throws IOException { Log.logError( "Resetting stream {0,number,integer} with error code {1,number,integer}", streamid, code); + markStream(streamid, code); ResetFrame frame = new ResetFrame(streamid, code); sendFrame(frame); } else if (debug.on()) { @@ -892,6 +902,11 @@ void resetStream(int streamid, int code) throws IOException { } } + private void markStream(int streamid, int code) { + Stream s = streams.get(streamid); + if (s != null) s.markStream(code); + } + // reduce count of streams by 1 if stream still exists synchronized void decrementStreamsCount(int streamid) { Stream s = streams.get(streamid); @@ -1193,12 +1208,19 @@ private Stream registerNewStream(OutgoingHeaders> oh) { Stream stream = oh.getAttachment(); assert stream.streamid == 0; int streamid = nextstreamid; - nextstreamid += 2; - stream.registerStream(streamid); - // set outgoing window here. This allows thread sending - // body to proceed. - windowController.registerStream(streamid, getInitialSendWindowSize()); - return stream; + if (stream.registerStream(streamid, false)) { + // set outgoing window here. This allows thread sending + // body to proceed. + nextstreamid += 2; + windowController.registerStream(streamid, getInitialSendWindowSize()); + return stream; + } else { + stream.cancelImpl(new IOException("Request cancelled")); + if (finalStream() && streams.isEmpty()) { + close(); + } + return null; + } } private final Object sendlock = new Object(); @@ -1212,7 +1234,9 @@ void sendFrame(Http2Frame frame) { OutgoingHeaders> oh = (OutgoingHeaders>) frame; Stream stream = registerNewStream(oh); // provide protection from inserting unordered frames between Headers and Continuation - publisher.enqueue(encodeHeaders(oh, stream)); + if (stream != null) { + publisher.enqueue(encodeHeaders(oh, stream)); + } } else { publisher.enqueue(encodeFrame(frame)); } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java b/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java index 608c572722a..ecb3badde71 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2020, 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 @@ -246,7 +246,7 @@ public final void handle() { } @Override public final void abort(IOException error) { - debug().log(() -> "abort: " + error); + debug().log(() -> this.getClass().getSimpleName() + " abort: " + error); pause(); // pause, then signal signalError(error); // should not be resumed after abort (not checked) } @@ -724,10 +724,12 @@ public final void request(long n) { @Override public final void cancel() { pauseReadEvent(); + if (debug.on()) debug.log("Read subscription cancelled"); if (Log.channel()) { Log.logChannel("Read subscription cancelled for channel {0}", channelDescr()); } + if (debug.on()) debug.log("Stopping read scheduler"); readScheduler.stop(); } @@ -748,6 +750,7 @@ final void handleError() { } final void signalError(Throwable error) { + if (debug.on()) debug.log("signal read error: " + error); if (!errorRef.compareAndSet(null, error)) { return; } @@ -808,6 +811,7 @@ final void read() { } current.errorRef.compareAndSet(null, error); current.signalCompletion(); + if (debug.on()) debug.log("Stopping read scheduler"); readScheduler.stop(); debugState("leaving read() loop with error: "); return; @@ -831,6 +835,7 @@ final void read() { // anyway. pauseReadEvent(); current.signalCompletion(); + if (debug.on()) debug.log("Stopping read scheduler"); readScheduler.stop(); } debugState("leaving read() loop after EOF: "); @@ -850,6 +855,7 @@ final void read() { // waiting for this event to terminate. // So resume the read event and return now... resumeReadEvent(); + if (errorRef.get() != null) continue; debugState("leaving read() loop after onNext: "); return; } else { @@ -861,6 +867,7 @@ final void read() { // readable again. demand.increase(1); resumeReadEvent(); + if (errorRef.get() != null) continue; debugState("leaving read() loop with no bytes"); return; } @@ -879,6 +886,7 @@ final void read() { // Trying to pause the event here would actually // introduce a race condition between this loop and // request(n). + if (errorRef.get() != null) continue; debugState("leaving read() loop with no demand"); break; } @@ -946,6 +954,7 @@ protected final void signalEvent() { @Override protected final void signalError(Throwable error) { + if (debug.on()) debug.log("signalError to %s (%s)", sub, error); sub.signalError(error); } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java b/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java index a33b2c8b5f3..0d890944f9d 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java @@ -28,6 +28,8 @@ import java.io.EOFException; import java.io.IOException; import java.io.UncheckedIOException; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.net.URI; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -134,12 +136,18 @@ class Stream extends ExchangeImpl { private volatile boolean remotelyClosed; private volatile boolean closed; private volatile boolean endStreamSent; - - final AtomicBoolean deRegistered = new AtomicBoolean(false); + // Indicates the first reason that was invoked when sending a ResetFrame + // to the server. A streamState of 0 indicates that no reset was sent. + // (see markStream(int code) + private volatile int streamState; // assigned using STREAM_STATE varhandle. + private volatile boolean deRegistered; // assigned using DEREGISTERED varhandle. // state flags private boolean requestSent, responseReceived; + // send lock: prevent sending DataFrames after reset occurred. + private final Object sendLock = new Object(); + /** * A reference to this Stream's connection Send Window controller. The * stream MUST acquire the appropriate amount of Send Window before @@ -293,7 +301,7 @@ private boolean consumed(DataFrame df) { } boolean deRegister() { - return deRegistered.compareAndSet(false, true); + return DEREGISTERED.compareAndSet(this, false, true); } @Override @@ -336,6 +344,36 @@ private void receiveResetFrame(ResetFrame frame) { sched.runOrSchedule(); } + /** + * Records the first reason which was invoked when sending a ResetFrame + * to the server in the streamState, and return the previous value + * of the streamState. This is an atomic operation. + * A possible use of this method would be to send a ResetFrame only + * if no previous reset frame has been sent. + * For instance: 
{@code
+     *  if (markStream(ResetFrame.CANCEL) == 0) {
+     *      connection.sendResetFrame(streamId, ResetFrame.CANCEL);
+     *  }
+     *  }
+ * @param code the reason code as per HTTP/2 protocol + * @return the previous value of the stream state. + */ + int markStream(int code) { + if (code == 0) return streamState; + synchronized (sendLock) { + return (int) STREAM_STATE.compareAndExchange(this, 0, code); + } + } + + private void sendDataFrame(DataFrame frame) { + synchronized (sendLock) { + // must not send DataFrame after reset. + if (streamState == 0) { + connection.sendDataFrame(frame); + } + } + } + // pushes entire response body into response subscriber // blocking when required by local or remote flow control CompletableFuture receiveData(BodySubscriber bodySubscriber, Executor executor) { @@ -392,6 +430,7 @@ CompletableFuture> sendBodyAsync() { */ void incoming(Http2Frame frame) throws IOException { if (debug.on()) debug.log("incoming: %s", frame); + var cancelled = closed || streamState != 0; if ((frame instanceof HeaderFrame)) { HeaderFrame hframe = (HeaderFrame)frame; if (hframe.endHeaders()) { @@ -403,9 +442,10 @@ void incoming(Http2Frame frame) throws IOException { receiveDataFrame(new DataFrame(streamid, DataFrame.END_STREAM, List.of())); } } else if (frame instanceof DataFrame) { - receiveDataFrame((DataFrame)frame); + if (cancelled) connection.dropDataFrame((DataFrame) frame); + else receiveDataFrame((DataFrame) frame); } else { - otherFrame(frame); + if (!cancelled) otherFrame(frame); } } @@ -734,6 +774,16 @@ CompletableFuture> sendHeadersAsync() { } else { requestContentLen = 0; } + + // At this point the stream doesn't have a streamid yet. + // It will be allocated if we send the request headers. + Throwable t = errorRef.get(); + if (t != null) { + if (debug.on()) debug.log("stream already cancelled, headers not sent: %s", (Object)t); + return MinimalFuture.failedFuture(t); + } + + // sending the headers will cause the allocation of the stream id OutgoingHeaders> f = headerFrame(requestContentLen); connection.sendFrame(f); CompletableFuture> cf = new MinimalFuture<>(); @@ -759,10 +809,17 @@ void completed() { // been already closed (or will be closed shortly after). } - void registerStream(int id) { - this.streamid = id; - connection.putStream(this, streamid); - if (debug.on()) debug.log("Registered stream %d", id); + boolean registerStream(int id, boolean registerIfCancelled) { + boolean cancelled = closed; + if (!cancelled || registerIfCancelled) { + this.streamid = id; + connection.putStream(this, streamid); + if (debug.on()) { + debug.log("Stream %d registered (cancelled: %b, registerIfCancelled: %b)", + streamid, cancelled, registerIfCancelled); + } + } + return !cancelled; } void signalWindowUpdate() { @@ -867,6 +924,7 @@ void trySend() { cancelImpl(t); return; } + int state = streamState; do { // handle COMPLETED; @@ -879,7 +937,7 @@ else if (item == COMPLETED) { } // handle bytes to send downstream - while (item.hasRemaining()) { + while (item.hasRemaining() && state == 0) { if (debug.on()) debug.log("trySend: %d", item.remaining()); DataFrame df = getDataFrame(item); if (df == null) { @@ -897,6 +955,7 @@ else if (item == COMPLETED) { + "Too many bytes in request body. Expected: " + contentLength + ", got: " + (contentLength - remainingContentLength); + assert streamid > 0; connection.resetStream(streamid, ResetFrame.PROTOCOL_ERROR); throw new IOException(msg); } else if (remainingContentLength == 0) { @@ -907,15 +966,26 @@ else if (item == COMPLETED) { } else { assert !endStreamSent : "internal error, send data after END_STREAM flag"; } + if ((state = streamState) != 0) { + if (debug.on()) debug.log("trySend: cancelled: %s", String.valueOf(t)); + break; + } if (debug.on()) debug.log("trySend: sending: %d", df.getDataLength()); - connection.sendDataFrame(df); + sendDataFrame(df); } + if (state != 0) break; assert !item.hasRemaining(); ByteBuffer b = outgoing.removeFirst(); assert b == item; } while (outgoing.peekFirst() != null); + if (state != 0) { + t = errorRef.get(); + if (t == null) t = new IOException(ResetFrame.stringForCode(streamState)); + throw t; + } + if (debug.on()) debug.log("trySend: request 1"); subscription.request(1); } catch (Throwable ex) { @@ -1118,7 +1188,11 @@ CompletableFuture sendBodyImpl() { @Override void cancel() { - cancel(new IOException("Stream " + streamid + " cancelled")); + if ((streamid == 0)) { + cancel(new IOException("Stream cancelled before streamid assigned")); + } else { + cancel(new IOException("Stream " + streamid + " cancelled")); + } } void onSubscriptionError(Throwable t) { @@ -1151,9 +1225,13 @@ void connectionClosing(Throwable cause) { // This method sends a RST_STREAM frame void cancelImpl(Throwable e) { errorRef.compareAndSet(null, e); - if (debug.on()) debug.log("cancelling stream {0}: {1}", streamid, e); + if (debug.on()) { + if (streamid == 0) debug.log("cancelling stream: %s", (Object)e); + else debug.log("cancelling stream %d: %s", streamid, e); + } if (Log.trace()) { - Log.logTrace("cancelling stream {0}: {1}\n", streamid, e); + if (streamid == 0) Log.logTrace("cancelling stream: {0}\n", e); + else Log.logTrace("cancelling stream {0}: {1}\n", streamid, e); } boolean closing; if (closing = !closed) { // assigning closing to !closed @@ -1176,14 +1254,15 @@ void cancelImpl(Throwable e) { } try { // will send a RST_STREAM frame - if (streamid != 0) { - connection.decrementStreamsCount(streamid); + if (streamid != 0 && streamState == 0) { e = Utils.getCompletionCause(e); if (e instanceof EOFException) { // read EOF: no need to try & send reset + connection.decrementStreamsCount(streamid); connection.closeStream(streamid); } else { - connection.resetStream(streamid, ResetFrame.CANCEL); + // no use to send CANCEL if already closed. + sendCancelStreamFrame(); } } } catch (Throwable ex) { @@ -1191,6 +1270,14 @@ void cancelImpl(Throwable e) { } } + void sendCancelStreamFrame() { + // do not reset a stream until it has a streamid. + if (streamid > 0 && markStream(ResetFrame.CANCEL) == 0) { + connection.resetStream(streamid, ResetFrame.CANCEL); + } + close(); + } + // This method doesn't send any frame void close() { if (closed) return; @@ -1406,4 +1493,17 @@ public void onDecoded(CharSequence name, CharSequence value) } } } + + private static final VarHandle STREAM_STATE; + private static final VarHandle DEREGISTERED; + static { + try { + STREAM_STATE = MethodHandles.lookup() + .findVarHandle(Stream.class, "streamState", int.class); + DEREGISTERED = MethodHandles.lookup() + .findVarHandle(Stream.class, "deRegistered", boolean.class); + } catch (Exception x) { + throw new ExceptionInInitializerError(x); + } + } } diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamCustom.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamCustom.java index c3801a0c429..95671d6a3ff 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamCustom.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamCustom.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamIO.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamIO.java index f11c31421a1..7f22bdf4961 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamIO.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsInputStreamIO.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesCustom.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesCustom.java index 847bc382838..e17a72a21dc 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesCustom.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesCustom.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesIO.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesIO.java index 7b7ffab49a2..678a58530c1 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesIO.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsLinesIO.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringCustom.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringCustom.java index eb6f2a7c8f9..be94c2fa4ff 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringCustom.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringCustom.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringIO.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringIO.java index 24d38f1a6df..a7120964869 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringIO.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesAsStringIO.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server diff --git a/test/jdk/java/net/httpclient/ThrowingPushPromisesSanity.java b/test/jdk/java/net/httpclient/ThrowingPushPromisesSanity.java index b2ffd374d2e..d064c322599 100644 --- a/test/jdk/java/net/httpclient/ThrowingPushPromisesSanity.java +++ b/test/jdk/java/net/httpclient/ThrowingPushPromisesSanity.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8229822 * @summary Tests what happens when push promise handlers and their * response body handlers and subscribers throw unexpected exceptions. * @library /test/lib http2/server