On Thu, 17 Jul 2025 12:33:23 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Sean Mullan has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add @SuppressWarnings("removal") to SocketPermissionCollection. > > src/java.base/share/classes/java/security/CodeSource.java line 287: > >> 285: * one of <i>codesource</i>'s IP addresses or this >> object's >> 286: * canonical host name must equal <i>codesource</i>'s >> canonical >> 287: * host name. > > Hello Sean, the original text in `SocketPermission.implies()` lists these 2 > rules separately, as follows: > >> >> <li>If this object was not initialized with a single IP address, and one of >> this object's IP addresses equals one of <i>p</i>'s IP addresses. >> >> <li>If this canonical name equals <i>p</i>'s canonical name. > > Given that we state at the beginning of this text that `the following checks > are made in order:`, do you think we should continue to list these 2 rules > separately, in that order, instead of combining them into one, like what's > being proposed here? > Hello Sean, the original text in `SocketPermission.implies()` lists these 2 > rules separately, as follows: > > > <li>If this object was not initialized with a single IP address, and one of > > this object's IP addresses equals one of p's IP addresses. > > <li>If this canonical name equals p's canonical name. > > Given that we state at the beginning of this text that `the following checks > are made in order:`, do you think we should continue to list these 2 rules > separately, in that order, instead of combining them into one, like what's > being proposed here? This one was a little tricky. `CodeSource.implies` states the following at the beginning: "More specifically, this method makes the following checks. If any fail, it returns false. If they all succeed, it returns true." That logic made sense prior to this fix because the rule said: "If this object's host (getLocation().getHost()) is not null, then the SocketPermission constructed with this object's host must imply the SocketPermission constructed with codesource's host." But if I just copy over the relevant conditions as-is from `SocketPermission.implies`, the last two conditions are not logically the same: - If this object was not initialized with a single IP address, and one of this object's IP addresses equals one of p's IP addresses. - If this canonical name equals p's canonical name. If the first condition fails above, it doesn't bail out and return false, instead it checks the last condition. So this is why I combined those two conditions and added an "or". Let me know if this makes sense or if you think there is a better way to word this. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26300#discussion_r2213455927