On Sat, 16 Nov 2024 01:59:43 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.
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains two commits:
> 
>  - merge latest from master branch
>  - 8344233: Remove calls to SecurityManager and doPrivileged in 
> java.net.ProxySelector and sun.net.spi.DefaultProxySelector after JEP 486 
> integration

Marked as reviewed by dfuchs (Reviewer).

LGTM

-------------

PR Review: https://git.openjdk.org/jdk/pull/22145#pullrequestreview-2440348616
PR Comment: https://git.openjdk.org/jdk/pull/22145#issuecomment-2480524050

Reply via email to