RFR: JDK-8243376: java.net.SocketPermission.implies(Permission p) spec is mismatching with implementation [v6]

[Michael McMahon]

On Tue, 9 Feb 2021 19:38:58 GMT, Jayashree S Kumar <github.com+31532647+jaysk1 at openjdk.org> wrote:

>> Issue
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8243376
>> 
>> Problem
>> 
>> The scenario is: 
>> - Some specified target hostname resolves to two IP addresses (always the same address pair). 
>> - The DNS resolved order of the two ip addresses changes (a usual LoadBalancer type behavior). 
>> - The CNAME of the two ip addresses differ. 
>> 
>> In SocketPermission class(void getIP() method), it internally resolves and saves only the first IP address resolved, not all the IP addresses resolved. 
>> - Depending on when the implier/implied SocketPermission hostname is resolved, the resolved addresses order differs, and the internally saved IP address mismatches, resulting on SocketPermission#implies() false. 
>> 
>> 
>> Michael McMahon kindly reviewed and suggested changes: https://mail.openjdk.java.net/pipermail/net-dev/2020-May/014001.html
>
> Jayashree S Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Code Review: Incorporated Indention correction, added getIP and updated try-catch inside for loop

Changes requested by michaelm (Reviewer).

Changes requested by michaelm (Reviewer).

test/jdk/java/net/SocketPermission/SocketPermissionIm.java line 6:

> 4:  * @summary SocketPermission implies(Permission p) spec allows, If the object was initialized with a single IP address and one of p's IP addresses is equal to this object's IP addr
> 5:  * @run java -Dsun.net.inetaddr.ttl=0 SocketPermissionIm
> 6:  */

Instead of:
@run java
you need:
@run main/othervm

Also see further comment below.

test/jdk/java/net/SocketPermission/SocketPermissionIm.java line 18:

> 16:         String hostsFileName = System.getProperty("test.src", ".") + File.separator + "Host.txt";
> 17:         System.setProperty("jdk.net.hosts.file", hostsFileName);
> 18: 

It's better to specify this system property on the command line because (we discovered recently) there are some environments where the name service gets initialized before main() is called in which case setting the property here is too late. So, you need to add a -Djdk.net.hosts.file=${test.src}....

test/jdk/java/net/SocketPermission/SocketPermissionIm.java line 32:

> 30:                } while (testPass <= 2);
> 31:     }
> 32: 

Formatting looks off in the file. The do statement in 22 should be aligned with SocketPermission in line 120.

The implementation changes look fine.

src/java.base/share/classes/java/net/SocketPermission.java line 706:

> 704:         for (String cname : cnames) {
> 705:             if (Objects.nonNull(cname) && match(cname, hname)) {
> 706:                 return true;

Sorry, forgot to mention, I think I'd prefer just to have:
if (cname != null && match(..)))

I think Objects.nonNull exists more for use as a method reference.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1916