RFR(M) : 8210039 : move OSInfo to top level testlibrary

Igor Ignatyev igor.ignatyev at oracle.com
Tue Sep 4 22:02:17 UTC 2018


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.fuchs at 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.hegarty at oracle.com> wrote:
>>> 
>>>> 
>>>> On 30 Aug 2018, at 08:51, Alan Bateman <Alan.Bateman at 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.
> 



More information about the core-libs-dev mailing list