On Fri, 15 Nov 2024 10:31:11 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which removes calls to > `SecurityManager` and `AccessController.doPrivileged()` from the > `ProxySelector` and `DefaultProxySelector` classes? > > Apart from the trivial removing of those calls, the commit in this PR also > removes the `getProxySelector` and `setProxySelector` named `NetPermission`s. > These 2 named permissions, were previously only documented on the > `ProxySelector.getDefault()` and `ProxySelector.setDefault()` methods. With > the removal of SecurityManager, the specification of these named properties > ceases to exist. Having said that, would removal of these named > NetPermission(s) require a CSR linked to this current issue? > > The other major chunk of this change is moving the proxy determination logic > in `DefaultProxySelector` into a private method from the previous > `doPriveleged()` call block. > > No new tests have been introduced. Existing tests in tier1 and tier2 continue > to pass with this change. LGTM. No CSR needed for removing those permissions. There's no visible change to the public APIs, and no behavior change that was not already covered by JEP 486 CSR, since permissions checks have now become deadcode. Or am I missing something? ------------- Marked as reviewed by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22145#pullrequestreview-2438351071
