OpenJDK / jdk / jdk

changeset 53865:c925e36a8e6d

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8168069: X509TrustManagerImpl causes ClassLoader leaks with unparseable extensions Reviewed-by: mullan
author xuelei
date Wed, 20 Feb 2019 18:46:30 -0800
parents 81a9748bc86c
children 880baf6db17b
files src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java src/java.base/share/classes/sun/security/x509/CertificateExtensions.java test/jdk/sun/security/ssl/SSLContextImpl/BadKSProvider.java test/jdk/sun/security/ssl/SSLContextImpl/BadTSProvider.java
diffstat 4 files changed, 56 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java	Wed Feb 20 19:41:43 2019 -0500
+++ b/src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java	Wed Feb 20 18:46:30 2019 -0800
@@ -921,29 +921,45 @@
 
         static {
             Exception reserved = null;
-            TrustManager[] tmMediator;
+            TrustManager[] tmMediator = null;
             try {
                 tmMediator = getTrustManagers();
             } catch (Exception e) {
                 reserved = e;
-                tmMediator = new TrustManager[0];
+                if (SSLLogger.isOn && SSLLogger.isOn("ssl,defaultctx")) {
+                    SSLLogger.warning(
+                            "Failed to load default trust managers", e);
+                }
             }
-            trustManagers = tmMediator;
 
+            KeyManager[] kmMediator = null;
             if (reserved == null) {
-                KeyManager[] kmMediator;
                 try {
                     kmMediator = getKeyManagers();
                 } catch (Exception e) {
                     reserved = e;
-                    kmMediator = new KeyManager[0];
+                    if (SSLLogger.isOn && SSLLogger.isOn("ssl,defaultctx")) {
+                        SSLLogger.warning(
+                                "Failed to load default key managers", e);
+                    }
                 }
-                keyManagers = kmMediator;
-            } else {
-                keyManagers = new KeyManager[0];
             }
 
-            reservedException = reserved;
+            if (reserved != null) {
+                trustManagers = new TrustManager[0];
+                keyManagers = new KeyManager[0];
+
+                // Important note: please don't reserve the original exception
+                // object, which may be not garbage collection friendly as
+                // 'reservedException' is a static filed.
+                reservedException =
+                        new KeyManagementException(reserved.getMessage());
+            } else {
+                trustManagers = tmMediator;
+                keyManagers = kmMediator;
+
+                reservedException = null;
+            }
         }
 
         private static TrustManager[] getTrustManagers() throws Exception {
@@ -1071,21 +1087,30 @@
     private static final class DefaultSSLContextHolder {
 
         private static final SSLContextImpl sslContext;
-        static Exception reservedException = null;
+        private static final Exception reservedException;
 
         static {
+            Exception reserved = null;
             SSLContextImpl mediator = null;
             if (DefaultManagersHolder.reservedException != null) {
-                reservedException = DefaultManagersHolder.reservedException;
+                reserved = DefaultManagersHolder.reservedException;
             } else {
                 try {
                     mediator = new DefaultSSLContext();
                 } catch (Exception e) {
-                    reservedException = e;
+                    // Important note: please don't reserve the original
+                    // exception object, which may be not garbage collection
+                    // friendly as 'reservedException' is a static filed.
+                    reserved = new KeyManagementException(e.getMessage());
+                    if (SSLLogger.isOn && SSLLogger.isOn("ssl,defaultctx")) {
+                        SSLLogger.warning(
+                                "Failed to load default SSLContext", e);
+                    }
                 }
             }
 
             sslContext = mediator;
+            reservedException = reserved;
         }
     }
 
--- a/src/java.base/share/classes/sun/security/x509/CertificateExtensions.java	Wed Feb 20 19:41:43 2019 -0500
+++ b/src/java.base/share/classes/sun/security/x509/CertificateExtensions.java	Wed Feb 20 18:46:30 2019 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -355,7 +355,7 @@
 
 class UnparseableExtension extends Extension {
     private String name;
-    private Throwable why;
+    private String exceptionDescription;
 
     public UnparseableExtension(Extension ext, Throwable why) {
         super(ext);
@@ -371,12 +371,13 @@
             // If we cannot find the name, just ignore it
         }
 
-        this.why = why;
+        this.exceptionDescription = why.toString();
     }
 
     @Override public String toString() {
         return super.toString() +
-                "Unparseable " + name + "extension due to\n" + why + "\n\n" +
+                "Unparseable " + name + "extension due to\n" +
+                exceptionDescription + "\n\n" +
                 new HexDumpEncoder().encodeBuffer(getExtensionValue());
     }
 }
--- a/test/jdk/sun/security/ssl/SSLContextImpl/BadKSProvider.java	Wed Feb 20 19:41:43 2019 -0500
+++ b/test/jdk/sun/security/ssl/SSLContextImpl/BadKSProvider.java	Wed Feb 20 18:46:30 2019 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -21,14 +21,16 @@
  * questions.
  */
 
+//
+// SunJSSE does not support dynamic system properties, no way to re-use
+// system properties in samevm/agentvm mode.
+//
+
 /*
  * @test
- * @bug 4919147
+ * @bug 4919147 8168069
  * @summary Support for token-based KeyStores
  * @run main/othervm BadKSProvider
- *
- *     SunJSSE does not support dynamic system properties, no way to re-use
- *     system properties in samevm/agentvm mode.
  */
 
 import java.io.*;
@@ -176,16 +178,16 @@
             // catching the exception is ok,
             // but let's try to confirm it is the right exception.
             //
-            // XXX this test must be updated if the exception message changes
+            // Note: this test must be updated if the exception message changes
 
             Throwable cause = se.getCause();
-            if (cause instanceof java.security.NoSuchAlgorithmException == false) {
+            if (!(cause instanceof java.security.NoSuchAlgorithmException)) {
                 se.printStackTrace();
                 throw new Exception("Unexpected exception" + se);
             }
 
             cause = cause.getCause();
-            if (cause instanceof java.security.NoSuchProviderException == false) {
+            if (!(cause instanceof java.security.KeyManagementException)) {
                 se.printStackTrace();
                 throw new Exception("Unexpected exception" + se);
             }
--- a/test/jdk/sun/security/ssl/SSLContextImpl/BadTSProvider.java	Wed Feb 20 19:41:43 2019 -0500
+++ b/test/jdk/sun/security/ssl/SSLContextImpl/BadTSProvider.java	Wed Feb 20 18:46:30 2019 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -28,7 +28,7 @@
 
 /*
  * @test
- * @bug 4919147
+ * @bug 4919147 8168069
  * @summary Support for token-based KeyStores
  * @run main/othervm BadTSProvider
  */
@@ -175,12 +175,10 @@
             new BadTSProvider();
             throw new SecurityException("expected no-such-provider exception");
         } catch (SocketException se) {
-
             // catching the exception is ok,
             // but let's try to confirm it is the right exception.
             //
-            // XXX this test must be updated if the exception message changes
-
+            // Note: this test must be updated if the exception message changes
             Throwable cause = se.getCause();
             if (!(cause instanceof NoSuchAlgorithmException)) {
                 se.printStackTrace();
@@ -188,13 +186,7 @@
             }
 
             cause = cause.getCause();
-            if (!(cause instanceof KeyStoreException)) {
-                se.printStackTrace();
-                throw new Exception("Unexpected exception" + se);
-            }
-
-            cause = cause.getCause();
-            if (!(cause instanceof NoSuchProviderException)) {
+            if (!(cause instanceof KeyManagementException)) {
                 se.printStackTrace();
                 throw new Exception("Unexpected exception" + se);
             }
© 2007, Oracle and/or its affiliates
Terms of Use · Privacy · Trademarks