On Thu, 14 Nov 2024 20:40:46 GMT, Daniel Fuchs <dfu...@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
