On Thu, 17 Jul 2025 16:11:00 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>>> 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. > > After you mentioned this detail, I read this doc in its entirety. Would > something like the following be a bit more clear: > > > > * <li> If this object's host (getLocation().getHost()) is not null, > * then the following checks are made in that order and if any > * of these checks are satisfied, then return true: > * <ol> > * <li> If this object's host was initialized with a single IP > * address then one of <i>codesource</i>'s IP addresses must be > * equal to this object's IP address. > * <li> If this object's host is a wildcard domain (such as > * *.example.com), then <i>codesource</i>'s canonical host name > * (the name without any preceding *) must end with this object's > * canonical host name. For example, *.example.com implies > * *.foo.example.com. > * <li> If this object's host was not initialized with a single > * IP address, then one of this object's IP addresses must equal > * one of <i>codesource</i>'s IP addresses. > * <li> This object's canonical host name must equal > <i>codesource</i>'s > * canonical host name. > > > > > Also note that, in the above text I used `<ol>` instead of `<ul>` to show the > ordering intent. However, if the use of `<ul>` was intentional for better > rendering, then that's fine too. It's not the same logic. Even if the checks above pass, `implies()` does not return true yet, it still has to process the rules after that. I think you could say "... and if any of these checks are not satisfied, then return false" but that is somewhat redundant with the first sentence of `implies()`: "More specifically, this method makes the following checks. If any fail, it returns false. If they all succeed, it returns true." ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26300#discussion_r2213851416
