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.