Daniel, Chris, Alan,
thank you for your review.
I've pushed it w/ Platfform::privilegedGetProperty method added to shorten
lines in Platform.java
Cheers,
-- Igor
> On Sep 4, 2018, at 6:08 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>
> Hi Igor,
>
> Nit: You could have introduced a
> private static String getProperty(String name) {
> return AccessController.doP....
> }
> in Platform.java to avoid the long lines.
>
> Otherwise looks good to me too!
>
> best regards,
>
> -- daniel
>
> On 31/08/2018 19:42, Igor Ignatyev wrote:
>> Alan, Chris,
>> thanks for looking at it, I went w/ the alternative suggested by Chris. that
>> required a sprinkle of doPrivileged in the testlibrary, but now
>> Sockets/policy.* files grant the minimal required permissions to the test
>> code.
>> the incremental webrev can found here[1], please let me know if you need the
>> whole webrev.
>> [1] http://cr.openjdk.java.net/~iignatyev//8210039/webrev.0-1/index.html
>> Thanks,
>> -- Igor
>>> On Aug 30, 2018, at 3:28 AM, Chris Hegarty <chris.hega...@oracle.com> wrote:
>>>
>>>>
>>>> On 30 Aug 2018, at 08:51, Alan Bateman <alan.bate...@oracle.com> wrote:
>>>>
>>>> On 28/08/2018 17:50, Igor Ignatyev wrote:
>>>>> http://cr.openjdk.java.net/~iignatyev//8210039/webrev.00/index.html
>>>>>> 698 lines changed: 114 ins; 240 del; 344 mod
>>>>> Hi all,
>>>>>
>>>>> could you please review this clean up of jdk testlibrary?
>>>>> the patch updates the tests to use jdk.test.lib.Platform instead of
>>>>> jdk.testlibrary.OSInfo.OSType, cleans up OSInfo and renames it to
>>>>> jdk.test.lib.OSVersion.
>>>>>
>>>>> testing: changed tests + :jdk_desktop test group
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~iignatyev//8210039/webrev.00/index.html
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8210039
>>>>>
>>>> The updates to the Sockets policy file suggests using this
>>>> jdk.test.lib.Platform/OSVersion requires permissions that the test
>>>> infrastructure needs, not the test. It's not wrong but it's always a
>>>> concern when tests running with a security manager are granted non-obvious
>>>> permissions.
>>>
>>> The uses of test libraries with security manager is a little
>>> cumbersome, and usually ends up with the test code being
>>> granted more permissions than is necessary. I share Alan’s
>>> concern.
>>>
>>> Another alternative, that we used in other areas, is to grant
>>> the test library only minimal permissions, separate to the
>>> actual test code. For example:
>>>
>>> http://hg.openjdk.java.net/jdk/jdk/file/9183040e34d8/test/jdk/java/net/httpclient/AsFileDownloadTest.policy#l24
>>>
>>> <http://hg.openjdk.java.net/jdk/jdk/file/9183040e34d8/test/jdk/java/net/httpclient/AsFileDownloadTest.policy#l24>
>>>
>>> -Chris.
>
