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.
