RFR: 8344228: Revisit SecurityManager usage in java.net.http after JEP 486 integration

[Michael McMahon]

On Thu, 14 Nov 2024 20:40:46 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> Please find here a patch that cleans up the java.net.http module code to remove permission checks and doPriviliged calls.
> This was mostly mechanical.

Great cleanup! Good to see a lot of complicated cruft being removed.

src/java.net.http/share/classes/jdk/internal/net/http/HttpClientBuilderImpl.java line 49:

> 47:     HttpClient.Version version;
> 48:     Executor executor;
> 49:     SSLContext sslContext;

I think this line was referring to the SSLContext and SSLParameters rather than the security manager

src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line 323:

> 321:     private final DelegatingExecutor delegatingExecutor;
> 322:     private final boolean isDefaultExecutor;
> 323:     private final SSLContext sslContext;

Same here. I guess it's okay to remove them either way

src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java line 245:

> 243: 
> 244:             try {
> 245:                 pathForDefaultFSCheck(path);

Is this intended, calling the method but ignoring the return value? If so, a comment might be in order.

src/java.net.http/share/classes/jdk/internal/net/http/ResponseBodyHandlers.java line 55:

> 53:     private ResponseBodyHandlers() { }
> 54: 
> 55:     private static final String pathForDefaultFSCheck(Path path) {

maybe this method and the equivalent in the previous file could be renamed `checkPathForDefaultFS` which would suggest an exception being thrown in error cases.

test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/SimpleSSLContext.java line 45:

> 43:  * Creates a simple usable SSLContext for SSLSocketFactory
> 44:  * or a HttpsServer using a default keystore in the test tree.
> 45:  */

Can the securityExceptions local variable be removed? And the catch (SecurityException) {} block?

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

PR Review: https://git.openjdk.org/jdk/pull/22118#pullrequestreview-2437178625
PR Review Comment: https://git.openjdk.org/jdk/pull/22118#discussion_r1842874479
PR Review Comment: https://git.openjdk.org/jdk/pull/22118#discussion_r1842878174
PR Review Comment: https://git.openjdk.org/jdk/pull/22118#discussion_r1842886201
PR Review Comment: https://git.openjdk.org/jdk/pull/22118#discussion_r1842891211
PR Review Comment: https://git.openjdk.org/jdk/pull/22118#discussion_r1842898432