On 7/19/19 7:14 AM, Alan Bateman wrote:
On 19/07/2019 15:00, Severin Gehwolf wrote:
:
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.
Using privilegedGetProperty is okay as long as the test classes are
granted permission to read this property. You should be able to verify
that quickly by running a few tests that come with their own policy
files.
Platform requires the tests using it to grant "PropertyPermission" to
the code source of test library. test/jdk/jdk/net/Sockets/policy.* files
are already doing that and so it's fine.
It'd be nice if @library would be able to augment the security policy
but it's a different issue.
In passing, the cast privilegedGetProperty methods is a bit ugly, this
might be a bit cleaner:
PrivilegedAction<String> pa = () -> System.getProperty(key,
defaultValue);
return AccessController.doPrivileged(pa);
The proposed patch with this reformat looks okay.
Mandy
