Hi Patrick, This looks good to me..
Best Lance > On Apr 4, 2020, at 11:26 AM, Patrick Concannon <patrick.concan...@oracle.com> > wrote: > > Hi Chris and Daniel, > > > Well spotted, Chris. Thanks for that! > > Thanks too Daniel, that's a good idea. I've made those changes and included > them in a new webrev, which you can find below. > > http://cr.openjdk.java.net/~pconcannon/8240533/webrevs/webrev.02/ > > > Kind regards, > > Patrick > > On 03/04/2020 17:16, Daniel Fuchs wrote: >> Hi Patrick, >> >> 120 { perms.add(new SocketPermission("127.0.0.1:0", >> 121 "connect,accept")); } >> 122 { perms.add(new SocketPermission("0.0.0.0:0", >> 123 "connect,accept")); } >> >> there in other tests - I think a single permission: >> >> { perms.add(new SocketPermission("*:0")); } >> >> would be more robust as it would take care of both IPv6 and IPv4 in one >> go. We should strive to avoid to hard-code 127.0.0.1 and 0.0.0.0 >> in tests. >> >> best regards, >> >> -- daniel >> >> On 03/04/2020 14:47, Patrick Concannon wrote: >>> Hi, >>> >>> Thanks for the feedback. > > >>> >>> Lance - I swapped out expectThrows for assertThrows, as requested. >>> >>> Chris - I put in an extra check in the tests to ensure that the new code >>> doesn’t interfere with the Security Manager checks already present in the >>> source. >>> >>> The new webrev can be found here: >>> http://cr.openjdk.java.net/~pconcannon/8240533/webrevs/webrev.01/ > >>> <http://cr.openjdk.java.net/~pconcannon/8240533/webrevs/webrev.01/> >>> >>> >>> Kind regards, >>> >>> Patrick >>> >>> On 31/03/2020 15:33, Chris Hegarty wrote: >>>> Patrick, >>>> >>>>> On 31 Mar 2020, at 15:08, Daniel Fuchs<daniel.fu...@oracle.com> wrote: >>>>> >>>>>> .. >>>>>> bug:https://bugs.openjdk.java.net/browse/JDK-8240533 >>>>>> webrev:http://cr.openjdk.java.net/~pconcannon/8240533/webrevs/webrev.00/ >>>> Look good Patrick. >>>> >>>> The check is deliberately performed after the security manager checks, >>>> right? If so, it is worth asserting this in a test. >>>> >>>> -Chris. >>>> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
