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

[Mandy Chung]

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