RFR: 8228434: jdk/net/Sockets/Test.java fails after JDK-8227642

[Severin Gehwolf]

Hi Claes,

On Fri, 2019-07-19 at 15:44 +0200, Claes Redestad wrote:
> Hi,
> 
> why not just use the Platform.privilegedGetProperty method, which seems
> purpose built for avoiding issues with reading properties like these
> in tests running with a security manager?

Sure. By adding a method also accepting a default value it would work
as well. If that's preferred, I can change it:

diff --git a/test/lib/jdk/test/lib/Platform.java b/test/lib/jdk/test/lib/Platform.java
--- a/test/lib/jdk/test/lib/Platform.java
+++ b/test/lib/jdk/test/lib/Platform.java
@@ -37,8 +37,7 @@
     // E.g.: "/usr/local/bin/docker". We define this constant here so
     // that it can be used in VMProps as well which checks docker support
     // via this command
-    public static final String DOCKER_COMMAND =
-        System.getProperty("jdk.test.docker.command", "docker");
+    public  static final String DOCKER_COMMAND = privilegedGetProperty("jdk.test.docker.command", "docker");
     public  static final String vmName      = privilegedGetProperty("java.vm.name");
     public  static final String vmInfo      = privilegedGetProperty("java.vm.info");
     private static final String osVersion   = privilegedGetProperty("os.version");
@@ -57,6 +56,11 @@
                 PrivilegedAction<String>) () -> System.getProperty(key));
     }
 
+    private static String privilegedGetProperty(String key, String defVal) {
+        return AccessController.doPrivileged((
+                PrivilegedAction<String>) () -> System.getProperty(key, defVal));
+    }
+
     public static boolean isClient() {
         return vmName.endsWith(" Client VM");
     }

Alan suggested the approach in webrev 01.

Thanks,
Severin

> /Claes
> 
> On 2019-07-19 15:40, Severin Gehwolf wrote:
> > Hi,
> > 
> > Please review this fix for an issue introduced by JDK-8227642 pointed
> > out by Alan Bateman. Thanks, Alan! That enhancement introduced a new
> > constant in the test libraries' Platform class which can cause issues
> > in tests which run with a security manager. One such instance is
> > jdk/net/Sockets/Test.java.
> > 
> > The proposed webrev addresses this issue by delaying loading of the
> > DOCKER_COMMAND constant (which in turn triggers System.getProperty
> > call).
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8228434
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8228434/01/webrev/
> > 
> > Testing: tier1 on Linux x86_64, jdk/net jtreg tests. Docker/podman testing.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> >