On Thu, 17 Jul 2025 14:08:57 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> 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. 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26300#discussion_r2213764079
