From ef4ba224c4887b2e307937754064d3623a2d3de5 Mon Sep 17 00:00:00 2001
From: Weijun Wang <weijun@openjdk.org>
Date: Tue, 22 Jun 2021 02:06:59 +0000
Subject: [PATCH] 8268349: Provide clear run-time warnings about Security
 Manager deprecation

Reviewed-by: lancea, mullan, alanb
---
 .../share/classes/java/lang/System.java       |  46 ++++--
 .../LoggerFinderLoaderTest.java               |  16 +--
 .../lang/System/SecurityManagerWarnings.java  | 132 ++++++++++++------
 .../lambda/LogGeneratedClassesTest.java       |   9 +-
 .../spi/URLStreamHandlerProvider/Basic.java   |  10 +-
 .../ProtectionDomain/RecursionDebug.java      |   6 +-
 6 files changed, 144 insertions(+), 75 deletions(-)

diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java
index de8122dc074..5910ca62443 100644
--- a/src/java.base/share/classes/java/lang/System.java
+++ b/src/java.base/share/classes/java/lang/System.java
@@ -44,14 +44,16 @@ import java.lang.reflect.Executable;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.net.URI;
+import java.net.URL;
 import java.nio.charset.CharacterCodingException;
-import java.security.AccessControlContext;
-import java.security.ProtectionDomain;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.nio.channels.Channel;
 import java.nio.channels.spi.SelectorProvider;
 import java.nio.charset.Charset;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.CodeSource;
+import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -324,6 +326,16 @@ public final class System {
     private static native void setOut0(PrintStream out);
     private static native void setErr0(PrintStream err);
 
+    // Remember initial System.err. setSecurityManager() warning goes here
+    private static volatile @Stable PrintStream initialErrStream;
+
+    private static URL codeSource(Class<?> clazz) {
+        PrivilegedAction<ProtectionDomain> pa = clazz::getProtectionDomain;
+        @SuppressWarnings("removal")
+        CodeSource cs = AccessController.doPrivileged(pa).getCodeSource();
+        return (cs != null) ? cs.getLocation() : null;
+    }
+
     /**
      * Sets the system-wide security manager.
      *
@@ -362,16 +374,29 @@ public final class System {
      *       method.
      */
     @Deprecated(since="17", forRemoval=true)
+    @CallerSensitive
     public static void setSecurityManager(@SuppressWarnings("removal") SecurityManager sm) {
         if (allowSecurityManager()) {
-            System.err.println("WARNING: java.lang.System::setSecurityManager" +
-                    " is deprecated and will be removed in a future release.");
+            var callerClass = Reflection.getCallerClass();
+            URL url = codeSource(callerClass);
+            final String source;
+            if (url == null) {
+                source = callerClass.getName();
+            } else {
+                source = callerClass.getName() + " (" + url + ")";
+            }
+            initialErrStream.printf("""
+                    WARNING: A terminally deprecated method in java.lang.System has been called
+                    WARNING: System::setSecurityManager has been called by %s
+                    WARNING: Please consider reporting this to the maintainers of %s
+                    WARNING: System::setSecurityManager will be removed in a future release
+                    """, source, callerClass.getName());
             implSetSecurityManager(sm);
         } else {
             // security manager not allowed
             if (sm != null) {
                 throw new UnsupportedOperationException(
-                    "Runtime configured to disallow security manager");
+                    "The Security Manager is deprecated and will be removed in a future release");
             }
         }
     }
@@ -2191,10 +2216,13 @@ public final class System {
         }
 
         if (needWarning) {
-            System.err.println("WARNING: The Security Manager is deprecated" +
-                    " and will be removed in a future release.");
+            System.err.println("""
+                    WARNING: A command line option has enabled the Security Manager
+                    WARNING: The Security Manager is deprecated and will be removed in a future release""");
         }
 
+        initialErrStream = System.err;
+
         // initializing the system class loader
         VM.initLevel(3);
 
diff --git a/test/jdk/java/lang/System/LoggerFinder/internal/LoggerFinderLoaderTest/LoggerFinderLoaderTest.java b/test/jdk/java/lang/System/LoggerFinder/internal/LoggerFinderLoaderTest/LoggerFinderLoaderTest.java
index 82df1d55c33..8f1ce47fb07 100644
--- a/test/jdk/java/lang/System/LoggerFinder/internal/LoggerFinderLoaderTest/LoggerFinderLoaderTest.java
+++ b/test/jdk/java/lang/System/LoggerFinder/internal/LoggerFinderLoaderTest/LoggerFinderLoaderTest.java
@@ -36,6 +36,7 @@ import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.ResourceBundle;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -44,15 +45,11 @@ import java.util.function.Supplier;
 import java.lang.System.LoggerFinder;
 import java.lang.System.Logger;
 import java.lang.System.Logger.Level;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.Locale;
-import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 import java.util.concurrent.atomic.AtomicReference;
-import jdk.internal.logger.SimpleConsoleLogger;
 
 /**
  * @test
@@ -202,6 +199,10 @@ public class LoggerFinderLoaderTest {
         }
     }
 
+    private static String withoutWarning(String in) {
+        return in.lines().filter(s -> !s.startsWith("WARNING:")).collect(Collectors.joining());
+    }
+
     static LoggerFinder getLoggerFinder(Class<?> expectedClass,
             String errorPolicy, boolean singleton) {
         LoggerFinder provider = null;
@@ -235,12 +236,7 @@ public class LoggerFinderLoaderTest {
                     }
                 } else if ("QUIET".equals(errorPolicy.toUpperCase(Locale.ROOT))) {
                     String warning = ErrorStream.errorStream.peek();
-                    String smDeprecationWarning
-                            = "WARNING: java.lang.System::setSecurityManager is deprecated and will be removed in a future release."
-                            + System.getProperty("line.separator");
-                    if (warning.startsWith(smDeprecationWarning)) {
-                        warning = warning.substring(smDeprecationWarning.length());
-                    }
+                    warning = withoutWarning(warning);
                     if (!warning.isEmpty()) {
                         throw new RuntimeException("Unexpected error message found: "
                                 + ErrorStream.errorStream.peek());
diff --git a/test/jdk/java/lang/System/SecurityManagerWarnings.java b/test/jdk/java/lang/System/SecurityManagerWarnings.java
index b565ba80904..65e0af69922 100644
--- a/test/jdk/java/lang/System/SecurityManagerWarnings.java
+++ b/test/jdk/java/lang/System/SecurityManagerWarnings.java
@@ -23,70 +23,116 @@
 
 /*
  * @test
- * @bug 8266459
+ * @bug 8266459 8268349
  * @summary check various warnings
  * @library /test/lib
  */
 
+import jdk.test.lib.JDKToolFinder;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.util.JarUtils;
 
-import java.security.Permission;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
 
 public class SecurityManagerWarnings {
+
     public static void main(String args[]) throws Exception {
         if (args.length == 0) {
-            run(null)
-                    .shouldHaveExitValue(0)
-                    .shouldContain("SM is enabled: false")
-                    .shouldNotContain("Security Manager is deprecated")
-                    .shouldContain("setSecurityManager is deprecated");
-            run("allow")
-                    .shouldHaveExitValue(0)
-                    .shouldContain("SM is enabled: false")
-                    .shouldNotContain("Security Manager is deprecated")
-                    .shouldContain("setSecurityManager is deprecated");
-            run("disallow")
-                    .shouldNotHaveExitValue(0)
-                    .shouldContain("SM is enabled: false")
-                    .shouldNotContain("Security Manager is deprecated")
-                    .shouldContain("UnsupportedOperationException");
-            run("SecurityManagerWarnings$MySM")
-                    .shouldHaveExitValue(0)
-                    .shouldContain("SM is enabled: true")
-                    .shouldContain("Security Manager is deprecated")
-                    .shouldContain("setSecurityManager is deprecated");
-            run("")
-                    .shouldNotHaveExitValue(0)
-                    .shouldContain("SM is enabled: true")
-                    .shouldContain("Security Manager is deprecated")
-                    .shouldContain("AccessControlException");
-            run("default")
-                    .shouldNotHaveExitValue(0)
-                    .shouldContain("SM is enabled: true")
-                    .shouldContain("Security Manager is deprecated")
-                    .shouldContain("AccessControlException");
+            Files.writeString(Path.of("policy"), """
+                    grant {
+                        permission java.lang.RuntimePermission "setIO";
+                        permission java.lang.RuntimePermission "createSecurityManager";
+                        permission java.lang.RuntimePermission "setSecurityManager";
+                    };
+                    """);
+
+            String testClasses = System.getProperty("test.classes");
+
+            allowTest(null, testClasses);
+            allowTest("allow", testClasses);
+            disallowTest("disallow", testClasses);
+            enableTest("", testClasses);
+            enableTest("default", testClasses);
+            enableTest("java.lang.SecurityManager", testClasses);
+
+            JarUtils.createJarFile(Path.of("a.jar"),
+                    Path.of(testClasses),
+                    Path.of("SecurityManagerWarnings.class"));
+
+            allowTest(null, "a.jar");
         } else {
             System.out.println("SM is enabled: " + (System.getSecurityManager() != null));
-            System.setSecurityManager(new SecurityManager());
+            PrintStream oldErr = System.err;
+            // Modify System.err, thus make sure warnings are always printed
+            // to the original System.err and will not be swallowed.
+            System.setErr(new PrintStream(new ByteArrayOutputStream()));
+            try {
+                System.setSecurityManager(new SecurityManager());
+            } catch (Exception e) {
+                // Exception messages must show in original stderr
+                e.printStackTrace(oldErr);
+                throw e;
+            }
         }
     }
 
-    static OutputAnalyzer run(String prop) throws Exception {
+    // When SM is allowed, no startup warning, has setSM warning
+    static void allowTest(String prop, String cp) throws Exception {
+        checkInstallMessage(run(prop, cp), cp)
+                .shouldHaveExitValue(0)
+                .stdoutShouldContain("SM is enabled: false")
+                .shouldNotContain("A command line option");
+    }
+
+    // When SM is disallowed, no startup warning, setSM fails
+    static void disallowTest(String prop, String cp) throws Exception {
+        run(prop, cp)
+                .shouldNotHaveExitValue(0)
+                .stdoutShouldContain("SM is enabled: false")
+                .shouldNotContain("A command line option")
+                .shouldNotContain("A terminally deprecated method")
+                .stderrShouldContain("UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release");
+    }
+
+    // When SM is allowed, has startup warning, has setSM warning
+    static void enableTest(String prop, String cp) throws Exception {
+        checkInstallMessage(run(prop, cp), cp)
+                .shouldHaveExitValue(0)
+                .stdoutShouldContain("SM is enabled: true")
+                .stderrShouldContain("WARNING: A command line option has enabled the Security Manager")
+                .stderrShouldContain("WARNING: The Security Manager is deprecated and will be removed in a future release");
+    }
+
+    // Check the setSM warning
+    static OutputAnalyzer checkInstallMessage(OutputAnalyzer oa, String cp) {
+        String uri = new File(cp).toURI().toString();
+        return oa
+                .stderrShouldContain("WARNING: A terminally deprecated method in java.lang.System has been called")
+                .stderrShouldContain("WARNING: System::setSecurityManager has been called by SecurityManagerWarnings (" + uri + ")")
+                .stderrShouldContain("WARNING: Please consider reporting this to the maintainers of SecurityManagerWarnings")
+                .stderrShouldContain("WARNING: System::setSecurityManager will be removed in a future release");
+    }
+
+    static OutputAnalyzer run(String prop, String cp) throws Exception {
+        ProcessBuilder pb;
         if (prop == null) {
-            return ProcessTools.executeTestJvm(
+            pb = new ProcessBuilder(
+                    JDKToolFinder.getJDKTool("java"),
+                    "-cp", cp,
                     "SecurityManagerWarnings", "run");
         } else {
-            return ProcessTools.executeTestJvm(
+            pb = new ProcessBuilder(
+                    JDKToolFinder.getJDKTool("java"),
+                    "-cp", cp,
                     "-Djava.security.manager=" + prop,
+                    "-Djava.security.policy=policy",
                     "SecurityManagerWarnings", "run");
         }
-    }
-
-    // This SecurityManager allows everything!
-    public static class MySM extends SecurityManager {
-        @Override
-        public void checkPermission(Permission perm) {
-        }
+        return ProcessTools.executeProcess(pb);
     }
 }
diff --git a/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java b/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java
index 92dd5857aa4..d75e9b4d2a5 100644
--- a/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java
+++ b/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java
@@ -147,9 +147,8 @@ public class LogGeneratedClassesTest extends LUtils {
                                "-Djdk.internal.lambda.dumpProxyClasses=notExist",
                                "com.example.TestLambda");
         assertEquals(tr.testOutput.stream()
-                                  .filter(s -> !s.contains("setSecurityManager is deprecated"))
                                   .filter(s -> s.startsWith("WARNING"))
-                                  .peek(s -> assertTrue(s.contains("does not exist")))
+                                  .filter(s -> s.contains("does not exist"))
                                   .count(),
                      1, "only show error once");
         tr.assertZero("Should still return 0");
@@ -164,9 +163,8 @@ public class LogGeneratedClassesTest extends LUtils {
                                "-Djdk.internal.lambda.dumpProxyClasses=file",
                                "com.example.TestLambda");
         assertEquals(tr.testOutput.stream()
-                                  .filter(s -> !s.contains("setSecurityManager is deprecated"))
                                   .filter(s -> s.startsWith("WARNING"))
-                                  .peek(s -> assertTrue(s.contains("not a directory")))
+                                  .filter(s -> s.contains("not a directory"))
                                   .count(),
                      1, "only show error once");
         tr.assertZero("Should still return 0");
@@ -224,9 +222,8 @@ public class LogGeneratedClassesTest extends LUtils {
                                    "-Djdk.internal.lambda.dumpProxyClasses=readOnly",
                                    "com.example.TestLambda");
             assertEquals(tr.testOutput.stream()
-                                      .filter(s -> !s.contains("setSecurityManager is deprecated"))
                                       .filter(s -> s.startsWith("WARNING"))
-                                      .peek(s -> assertTrue(s.contains("not writable")))
+                                      .filter(s -> s.contains("not writable"))
                                       .count(),
                          1, "only show error once");
             tr.assertZero("Should still return 0");
diff --git a/test/jdk/java/net/spi/URLStreamHandlerProvider/Basic.java b/test/jdk/java/net/spi/URLStreamHandlerProvider/Basic.java
index c6183c8d7cc..d2c635fb9ec 100644
--- a/test/jdk/java/net/spi/URLStreamHandlerProvider/Basic.java
+++ b/test/jdk/java/net/spi/URLStreamHandlerProvider/Basic.java
@@ -86,10 +86,14 @@ public class Basic {
     static final String SECURITY_MANAGER_DEPRECATED
             = "WARNING: The Security Manager is deprecated and will be removed in a future release."
                     + System.getProperty("line.separator");
+
+    private static String withoutWarning(String in) {
+        return in.lines().filter(s -> !s.startsWith("WARNING:")).collect(Collectors.joining());
+    }
+
     static final Consumer<Result> KNOWN = r -> {
-        if (r.exitValue != 0 ||
-                (!r.output.isEmpty() && !r.output.equals(SECURITY_MANAGER_DEPRECATED)))
-            throw new RuntimeException(r.output);
+        if (r.exitValue != 0 || !withoutWarning(r.output).isEmpty())
+            throw new RuntimeException("[" + r.output + "]");
     };
     static final Consumer<Result> UNKNOWN = r -> {
         if (r.exitValue == 0 ||
diff --git a/test/jdk/java/security/ProtectionDomain/RecursionDebug.java b/test/jdk/java/security/ProtectionDomain/RecursionDebug.java
index 3d2cdb6f090..4b12a238098 100644
--- a/test/jdk/java/security/ProtectionDomain/RecursionDebug.java
+++ b/test/jdk/java/security/ProtectionDomain/RecursionDebug.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2004, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 4947618
+ * @bug 4947618 8268349
  * @summary Recursion problem in security manager and policy code
  *
  * @run main/othervm/policy=Recursion.policy -Djava.security.debug=domain RecursionDebug
@@ -87,7 +87,5 @@ public class RecursionDebug {
             throw new Exception
                 ("Test with custom non-bootclass SecurityManager failed");
         }
-
-        System.setSecurityManager(null);
     }
 }