On Tue, 15 Oct 2024 16:14:58 GMT, Sean Mullan <mul...@openjdk.org> wrote:
> While making `LogManager.checkAccess` be a no-op might be more convenient, it > could unconditionally permit operations that formerly required a permission > check: clearly a bad idea. Always throwing a `SecurityException` is the > safest option. It's not about convenience _or_ safety; this part of the change has a provably flawed logical basis. These methods would no longer called from within the JDK after this change. All three of these methods were already previously defined to be a no-op when no security manager was installed (specifically when `System.getSecurityManager() == null`). Since no security manager may be installed after this change, this method will always return `null`. Thus, a no-op is still the most correct behavior and does not permit any operation that previously required a permission check (since it was already a no-op any time no security manager was installed, which will now be the only possible scenario). Therefore it is provably no safer to throw `SecurityException` here, since this will only prompt library developers to introduce the workaround I posted above when their tests fail, yielding the exact same result (except with a minor inconvenience to library developers). Either way is fine (as I said, the workaround is trivial), but IMO it's best to be conscious of the correct reasoning lest flawed assumptions _do_ end up enabling the introduction of unsafe changes elsewhere in the code. We don't have to make any assumptions about safety or previous behavior because it's all statically provable. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1801549133
