7158796: Tighten properties checking in EnvHelp

Move getProperty call out of computeBooleanFromString

Reviewed-by: skoivu, sla

jdk/src/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java

@@ -68,9 +68,9 @@ public class ServerNotifForwarder {
         this.notifBuffer = notifBuffer;
         this.connectionId = connectionId;
         connectionTimeout = EnvHelp.getServerConnectionTimeout(env);
-        checkNotificationEmission = EnvHelp.computeBooleanFromString(
+
-            env,
+        String stringBoolean = (String) env.get("jmx.remote.x.check.notification.emission");
-            "jmx.remote.x.check.notification.emission",false);
+        checkNotificationEmission = EnvHelp.computeBooleanFromString( stringBoolean );
         notificationAccessController =
                 EnvHelp.getNotificationAccessController(env);
     }

jdk/src/share/classes/com/sun/jmx/remote/util/EnvHelp.java

@@ -665,97 +665,57 @@ public class EnvHelp {
      * Computes a boolean value from a string value retrieved from a
      * property in the given map.
      *
-     * @param env the environment map.
+     * @param stringBoolean the string value that must be converted
-     * @param prop the name of the property in the environment map whose
+     * into a boolean value.
-     * returned string value must be converted into a boolean value.
-     * @param systemProperty if true, consult a system property of the
-     * same name if there is no entry in the environment map.
      *
      * @return
      *   <ul>
-     *   <li>{@code false} if {@code env.get(prop)} is {@code null}</li>
+     *   <li>{@code false} if {@code stringBoolean} is {@code null}</li>
      *   <li>{@code false} if
-     *       {@code ((String)env.get(prop)).equalsIgnoreCase("false")}
+     *       {@code stringBoolean.equalsIgnoreCase("false")}
      *       is {@code true}</li>
      *   <li>{@code true} if
-     *       {@code ((String)env.get(prop)).equalsIgnoreCase("true")}
+     *       {@code stringBoolean.equalsIgnoreCase("true")}
      *       is {@code true}</li>
      *   </ul>
      *
-     * @throws IllegalArgumentException if {@code env} is {@code null} or
+     * @throws IllegalArgumentException if
-     * {@code env.get(prop)} is not {@code null} and
      * {@code ((String)env.get(prop)).equalsIgnoreCase("false")} and
      * {@code ((String)env.get(prop)).equalsIgnoreCase("true")} are
      * {@code false}.
-     * @throws ClassCastException if {@code env.get(prop)} cannot be cast
-     * to {@code String}.
      */
-    public static boolean computeBooleanFromString(
+    public static boolean computeBooleanFromString(String stringBoolean) {
-            Map<String, ?> env, String prop, boolean systemProperty) {
-
-        if (env == null)
-            throw new IllegalArgumentException("env map cannot be null");
-
         // returns a default value of 'false' if no property is found...
-        return computeBooleanFromString(env,prop,systemProperty,false);
+        return computeBooleanFromString(stringBoolean,false);
     }
 
     /**
      * Computes a boolean value from a string value retrieved from a
      * property in the given map.
      *
-     * @param env the environment map.
+     * @param stringBoolean the string value that must be converted
-     * @param prop the name of the property in the environment map whose
+     * into a boolean value.
-     * returned string value must be converted into a boolean value.
-     * @param systemProperty if true, consult a system property of the
-     * same name if there is no entry in the environment map.
      * @param defaultValue a default value to return in case no property
      *        was defined.
      *
      * @return
      *   <ul>
-     *   <li>{@code defaultValue} if {@code env.get(prop)} is {@code null}
+     *   <li>{@code defaultValue} if {@code stringBoolean}
-     *       and {@code systemProperty} is {@code false}</li>
+     *   is {@code null}</li>
-     *   <li>{@code defaultValue} if {@code env.get(prop)} is {@code null}
-     *       and {@code systemProperty} is {@code true} and
-     *       {@code System.getProperty(prop)} is {@code null}</li>
-     *   <li>{@code false} if {@code env.get(prop)} is {@code null}
-     *       and {@code systemProperty} is {@code true} and
-     *       {@code System.getProperty(prop).equalsIgnoreCase("false")}
-     *       is {@code true}</li>
-     *   <li>{@code true} if {@code env.get(prop)} is {@code null}
-     *       and {@code systemProperty} is {@code true} and
-     *       {@code System.getProperty(prop).equalsIgnoreCase("true")}
-     *       is {@code true}</li>
      *   <li>{@code false} if
-     *       {@code ((String)env.get(prop)).equalsIgnoreCase("false")}
+     *       {@code stringBoolean.equalsIgnoreCase("false")}
      *       is {@code true}</li>
      *   <li>{@code true} if
-     *       {@code ((String)env.get(prop)).equalsIgnoreCase("true")}
+     *       {@code stringBoolean.equalsIgnoreCase("true")}
      *       is {@code true}</li>
      *   </ul>
      *
-     * @throws IllegalArgumentException if {@code env} is {@code null} or
+     * @throws IllegalArgumentException if
-     * {@code env.get(prop)} is not {@code null} and
      * {@code ((String)env.get(prop)).equalsIgnoreCase("false")} and
      * {@code ((String)env.get(prop)).equalsIgnoreCase("true")} are
      * {@code false}.
-     * @throws ClassCastException if {@code env.get(prop)} cannot be cast
-     * to {@code String}.
      */
-    public static boolean computeBooleanFromString(
+    public static boolean computeBooleanFromString( String stringBoolean, boolean defaultValue) {
-            Map<String, ?> env, String prop,
-            boolean systemProperty, boolean defaultValue) {
-
-        if (env == null)
-            throw new IllegalArgumentException("env map cannot be null");
-
-        String stringBoolean = (String) env.get(prop);
-        if (stringBoolean == null && systemProperty) {
-            stringBoolean =
-                    AccessController.doPrivileged(new GetPropertyAction(prop));
-        }
-
         if (stringBoolean == null)
             return defaultValue;
         else if (stringBoolean.equalsIgnoreCase("true"))
@@ -763,8 +723,8 @@ public class EnvHelp {
         else if (stringBoolean.equalsIgnoreCase("false"))
             return false;
         else
-            throw new IllegalArgumentException(prop +
+            throw new IllegalArgumentException(
-                " must be \"true\" or \"false\" instead of \"" +
+                "Property value must be \"true\" or \"false\" instead of \"" +
                 stringBoolean + "\"");
     }
 

jdk/src/share/classes/javax/management/remote/rmi/RMIConnector.java

@@ -277,9 +277,9 @@ public class RMIConnector implements JMXConnector, Serializable, JMXAddressable
             // Check for secure RMIServer stub if the corresponding
             // client-side environment property is set to "true".
             //
-            boolean checkStub = EnvHelp.computeBooleanFromString(
+            String stringBoolean =  (String) usemap.get("jmx.remote.x.check.stub");
-                    usemap,
+            boolean checkStub = EnvHelp.computeBooleanFromString(stringBoolean);
-                    "jmx.remote.x.check.stub",false);
+
             if (checkStub) checkStub(stub, rmiServerImplStubClass);
 
             // Connect IIOP Stub if needed.

jdk/src/share/classes/javax/management/remote/rmi/RMIConnectorServer.java

@@ -412,9 +412,8 @@ public class RMIConnectorServer extends JMXConnectorServer {
                 if (tracing)
                     logger.trace("start", "Using external directory: " + jndiUrl);
 
-                final boolean rebind = EnvHelp.computeBooleanFromString(
+                String stringBoolean = (String) attributes.get(JNDI_REBIND_ATTRIBUTE);
-                    attributes,
+                final boolean rebind = EnvHelp.computeBooleanFromString( stringBoolean );
-                    JNDI_REBIND_ATTRIBUTE,false);
 
                 if (tracing)
                     logger.trace("start", JNDI_REBIND_ATTRIBUTE + "=" + rebind);