[MRESOLVER-584] JDK transport HTTP/2 GOAWAY improvement (#532)
Changes:
* drop 21 closer, it abruptly uses "shutdown now"
* in 11 make httpClient NOT stored per session, but per transport instance
* simplify and refactor, put things in places like insecure mode config
---
https://issues.apache.org/jira/browse/MRESOLVER-584
diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
index 32da764..15a3732 100644
--- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
+++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
@@ -197,6 +197,17 @@
javaVersion);
}
}
+ final String httpsSecurityMode = ConfigUtils.getString(
+ session,
+ ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT,
+ ConfigurationProperties.HTTPS_SECURITY_MODE + "." + repository.getId(),
+ ConfigurationProperties.HTTPS_SECURITY_MODE);
+
+ if (!ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT.equals(httpsSecurityMode)
+ && !ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(httpsSecurityMode)) {
+ throw new IllegalArgumentException("Unsupported '" + httpsSecurityMode + "' HTTPS security mode.");
+ }
+ final boolean insecure = ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(httpsSecurityMode);
this.maxConcurrentRequests = new Semaphore(ConfigUtils.getInteger(
session,
@@ -205,7 +216,11 @@
CONFIG_PROP_MAX_CONCURRENT_REQUESTS));
this.headers = headers;
- this.client = getOrCreateClient(session, repository, javaVersion);
+ try {
+ this.client = createClient(session, repository, insecure);
+ } catch (Exception e) {
+ throw new NoTransporterException(repository, e);
+ }
}
private URI resolve(TransportTask task) {
@@ -386,7 +401,7 @@
}
private <T> HttpResponse<T> send(HttpRequest request, HttpResponse.BodyHandler<T> responseBodyHandler)
- throws IOException, InterruptedException {
+ throws Exception {
maxConcurrentRequests.acquire();
try {
return client.send(request, responseBodyHandler);
@@ -397,10 +412,116 @@
@Override
protected void implClose() {
- // no-op
+ if (client != null) {
+ JdkTransporterCloser.closer(client).run();
+ }
}
- private InetAddress getHttpLocalAddress(RepositorySystemSession session, RemoteRepository repository) {
+ private static HttpClient createClient(
+ RepositorySystemSession session, RemoteRepository repository, boolean insecure) throws Exception {
+
+ HashMap<Authenticator.RequestorType, PasswordAuthentication> authentications = new HashMap<>();
+ SSLContext sslContext = null;
+ try (AuthenticationContext repoAuthContext = AuthenticationContext.forRepository(session, repository)) {
+ if (repoAuthContext != null) {
+ sslContext = repoAuthContext.get(AuthenticationContext.SSL_CONTEXT, SSLContext.class);
+
+ String username = repoAuthContext.get(AuthenticationContext.USERNAME);
+ String password = repoAuthContext.get(AuthenticationContext.PASSWORD);
+
+ authentications.put(
+ Authenticator.RequestorType.SERVER,
+ new PasswordAuthentication(username, password.toCharArray()));
+ }
+ }
+
+ if (sslContext == null) {
+ if (insecure) {
+ sslContext = SSLContext.getInstance("TLS");
+ X509ExtendedTrustManager tm = new X509ExtendedTrustManager() {
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType) {}
+
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType) {}
+
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) {}
+
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {}
+
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
+
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
+
+ @Override
+ public X509Certificate[] getAcceptedIssuers() {
+ return null;
+ }
+ };
+ sslContext.init(null, new X509TrustManager[] {tm}, null);
+ } else {
+ sslContext = SSLContext.getDefault();
+ }
+ }
+
+ int connectTimeout = ConfigUtils.getInteger(
+ session,
+ ConfigurationProperties.DEFAULT_CONNECT_TIMEOUT,
+ ConfigurationProperties.CONNECT_TIMEOUT + "." + repository.getId(),
+ ConfigurationProperties.CONNECT_TIMEOUT);
+
+ HttpClient.Builder builder = HttpClient.newBuilder()
+ .version(HttpClient.Version.valueOf(ConfigUtils.getString(
+ session,
+ DEFAULT_HTTP_VERSION,
+ CONFIG_PROP_HTTP_VERSION + "." + repository.getId(),
+ CONFIG_PROP_HTTP_VERSION)))
+ .followRedirects(HttpClient.Redirect.NORMAL)
+ .connectTimeout(Duration.ofMillis(connectTimeout))
+ .sslContext(sslContext);
+
+ if (insecure) {
+ SSLParameters sslParameters = sslContext.getDefaultSSLParameters();
+ sslParameters.setEndpointIdentificationAlgorithm(null);
+ builder.sslParameters(sslParameters);
+ }
+
+ setLocalAddress(builder, () -> getHttpLocalAddress(session, repository));
+
+ if (repository.getProxy() != null) {
+ ProxySelector proxy = ProxySelector.of(new InetSocketAddress(
+ repository.getProxy().getHost(), repository.getProxy().getPort()));
+
+ builder.proxy(proxy);
+ try (AuthenticationContext proxyAuthContext = AuthenticationContext.forProxy(session, repository)) {
+ if (proxyAuthContext != null) {
+ String username = proxyAuthContext.get(AuthenticationContext.USERNAME);
+ String password = proxyAuthContext.get(AuthenticationContext.PASSWORD);
+
+ authentications.put(
+ Authenticator.RequestorType.PROXY,
+ new PasswordAuthentication(username, password.toCharArray()));
+ }
+ }
+ }
+
+ if (!authentications.isEmpty()) {
+ builder.authenticator(new Authenticator() {
+ @Override
+ protected PasswordAuthentication getPasswordAuthentication() {
+ return authentications.get(getRequestorType());
+ }
+ });
+ }
+
+ return builder.build();
+ }
+
+ private static InetAddress getHttpLocalAddress(RepositorySystemSession session, RemoteRepository repository) {
String bindAddress = ConfigUtils.getString(
session,
null,
@@ -418,155 +539,7 @@
}
}
- /**
- * Visible for testing.
- */
- static final String HTTP_INSTANCE_KEY_PREFIX = JdkTransporterFactory.class.getName() + ".http.";
-
- private HttpClient getOrCreateClient(RepositorySystemSession session, RemoteRepository repository, int javaVersion)
- throws NoTransporterException {
- final String instanceKey = HTTP_INSTANCE_KEY_PREFIX + repository.getId();
-
- final String httpsSecurityMode = ConfigUtils.getString(
- session,
- ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT,
- ConfigurationProperties.HTTPS_SECURITY_MODE + "." + repository.getId(),
- ConfigurationProperties.HTTPS_SECURITY_MODE);
-
- if (!ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT.equals(httpsSecurityMode)
- && !ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(httpsSecurityMode)) {
- throw new IllegalArgumentException("Unsupported '" + httpsSecurityMode + "' HTTPS security mode.");
- }
- final boolean insecure = ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(httpsSecurityMode);
-
- // todo: normally a single client per JVM is sufficient - in particular cause part of the config
- // is global and not per instance so we should create a client only when conf changes for a repo
- // else fallback on a global client
- try {
- return (HttpClient) session.getData().computeIfAbsent(instanceKey, () -> {
- HashMap<Authenticator.RequestorType, PasswordAuthentication> authentications = new HashMap<>();
- SSLContext sslContext = null;
- try {
- try (AuthenticationContext repoAuthContext =
- AuthenticationContext.forRepository(session, repository)) {
- if (repoAuthContext != null) {
- sslContext = repoAuthContext.get(AuthenticationContext.SSL_CONTEXT, SSLContext.class);
-
- String username = repoAuthContext.get(AuthenticationContext.USERNAME);
- String password = repoAuthContext.get(AuthenticationContext.PASSWORD);
-
- authentications.put(
- Authenticator.RequestorType.SERVER,
- new PasswordAuthentication(username, password.toCharArray()));
- }
- }
-
- if (sslContext == null) {
- if (insecure) {
- sslContext = SSLContext.getInstance("TLS");
- X509ExtendedTrustManager tm = new X509ExtendedTrustManager() {
- @Override
- public void checkClientTrusted(X509Certificate[] chain, String authType) {}
-
- @Override
- public void checkServerTrusted(X509Certificate[] chain, String authType) {}
-
- @Override
- public void checkClientTrusted(
- X509Certificate[] chain, String authType, Socket socket) {}
-
- @Override
- public void checkServerTrusted(
- X509Certificate[] chain, String authType, Socket socket) {}
-
- @Override
- public void checkClientTrusted(
- X509Certificate[] chain, String authType, SSLEngine engine) {}
-
- @Override
- public void checkServerTrusted(
- X509Certificate[] chain, String authType, SSLEngine engine) {}
-
- @Override
- public X509Certificate[] getAcceptedIssuers() {
- return null;
- }
- };
- sslContext.init(null, new X509TrustManager[] {tm}, null);
- } else {
- sslContext = SSLContext.getDefault();
- }
- }
-
- int connectTimeout = ConfigUtils.getInteger(
- session,
- ConfigurationProperties.DEFAULT_CONNECT_TIMEOUT,
- ConfigurationProperties.CONNECT_TIMEOUT + "." + repository.getId(),
- ConfigurationProperties.CONNECT_TIMEOUT);
-
- HttpClient.Builder builder = HttpClient.newBuilder()
- .version(HttpClient.Version.valueOf(ConfigUtils.getString(
- session,
- DEFAULT_HTTP_VERSION,
- CONFIG_PROP_HTTP_VERSION + "." + repository.getId(),
- CONFIG_PROP_HTTP_VERSION)))
- .followRedirects(HttpClient.Redirect.NORMAL)
- .connectTimeout(Duration.ofMillis(connectTimeout))
- .sslContext(sslContext);
-
- if (insecure) {
- SSLParameters sslParameters = sslContext.getDefaultSSLParameters();
- sslParameters.setEndpointIdentificationAlgorithm(null);
- builder.sslParameters(sslParameters);
- }
-
- setLocalAddress(builder, () -> getHttpLocalAddress(session, repository));
-
- if (repository.getProxy() != null) {
- ProxySelector proxy = ProxySelector.of(new InetSocketAddress(
- repository.getProxy().getHost(),
- repository.getProxy().getPort()));
-
- builder.proxy(proxy);
- try (AuthenticationContext proxyAuthContext =
- AuthenticationContext.forProxy(session, repository)) {
- if (proxyAuthContext != null) {
- String username = proxyAuthContext.get(AuthenticationContext.USERNAME);
- String password = proxyAuthContext.get(AuthenticationContext.PASSWORD);
-
- authentications.put(
- Authenticator.RequestorType.PROXY,
- new PasswordAuthentication(username, password.toCharArray()));
- }
- }
- }
-
- if (!authentications.isEmpty()) {
- builder.authenticator(new Authenticator() {
- @Override
- protected PasswordAuthentication getPasswordAuthentication() {
- return authentications.get(getRequestorType());
- }
- });
- }
-
- HttpClient result = builder.build();
- if (!session.addOnSessionEndedHandler(JdkTransporterCloser.closer(javaVersion, result))) {
- LOGGER.warn(
- "Using Resolver 2 feature without Resolver 2 session handling, you may leak resources.");
- }
-
- return result;
- } catch (Exception e) {
- throw new WrapperEx(e);
- }
- });
- } catch (WrapperEx e) {
- throw new NoTransporterException(repository, e.getCause());
- }
- }
-
- private void setLocalAddress(HttpClient.Builder builder, Supplier<InetAddress> addressSupplier) {
+ private static void setLocalAddress(HttpClient.Builder builder, Supplier<InetAddress> addressSupplier) {
try {
final InetAddress address = addressSupplier.get();
if (address == null) {
@@ -586,10 +559,4 @@
throw new IllegalStateException(e);
}
}
-
- private static final class WrapperEx extends RuntimeException {
- private WrapperEx(Throwable cause) {
- super(cause);
- }
- }
}
diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java
index a273c01..69f839a 100644
--- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java
+++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java
@@ -27,7 +27,7 @@
*/
public final class JdkTransporterCloser {
@SuppressWarnings("checkstyle:MagicNumber")
- static Runnable closer(int javaVersion, HttpClient httpClient) {
+ static Runnable closer(HttpClient httpClient) {
return () -> {
if (httpClient instanceof AutoCloseable) {
try {
diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-21/pom.xml b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-21/pom.xml
deleted file mode 100644
index cdf0e30..0000000
--- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-21/pom.xml
+++ /dev/null
@@ -1,98 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!--
- Licensed to the Apache Software Foundation (ASF) under one
- or more contributor license agreements. See the NOTICE file
- distributed with this work for additional information
- regarding copyright ownership. The ASF licenses this file
- to you under the Apache License, Version 2.0 (the
- "License"); you may not use this file except in compliance
- with the License. You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing,
- software distributed under the License is distributed on an
- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- KIND, either express or implied. See the License for the
- specific language governing permissions and limitations
- under the License.
--->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
- <modelVersion>4.0.0</modelVersion>
-
- <parent>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-transport-jdk-parent</artifactId>
- <version>2.0.1-SNAPSHOT</version>
- </parent>
-
- <artifactId>maven-resolver-transport-jdk-21</artifactId>
- <packaging>jar</packaging>
-
- <name>Maven Artifact Resolver Transport JDK 21</name>
- <description>Maven Artifact Transport JDK Java 11+.</description>
-
- <properties>
- <javaVersion>21</javaVersion>
- </properties>
-
- <dependencies>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-api</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-api</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-spi</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-util</artifactId>
- </dependency>
- <dependency>
- <groupId>javax.inject</groupId>
- <artifactId>javax.inject</artifactId>
- <scope>provided</scope>
- <optional>true</optional>
- </dependency>
-
- <dependency>
- <groupId>org.junit.jupiter</groupId>
- <artifactId>junit-jupiter-api</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-test-util</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-test-http</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-impl</artifactId>
- <scope>test</scope>
- </dependency>
- </dependencies>
-
- <build>
- <plugins>
- <plugin>
- <groupId>org.eclipse.sisu</groupId>
- <artifactId>sisu-maven-plugin</artifactId>
- </plugin>
- </plugins>
- </build>
-</project>
diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-21/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-21/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java
deleted file mode 100644
index b7da9a8..0000000
--- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-21/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterCloser.java
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.eclipse.aether.transport.jdk;
-
-import java.net.http.HttpClient;
-
-/**
- * JDK Transport that properly closes {@link HttpClient} on Java 21+.
- *
- * @since 2.0.0
- */
-public final class JdkTransporterCloser {
- static Runnable closer(int javaVersion, HttpClient httpClient) {
- return httpClient::shutdownNow;
- }
-}
diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk/pom.xml b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk/pom.xml
index 7fe556a..97f0320 100644
--- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk/pom.xml
+++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk/pom.xml
@@ -53,12 +53,6 @@
<version>${project.version}</version>
<optional>true</optional>
</dependency>
- <dependency>
- <groupId>org.apache.maven.resolver</groupId>
- <artifactId>maven-resolver-transport-jdk-21</artifactId>
- <version>${project.version}</version>
- <optional>true</optional>
- </dependency>
<dependency>
<groupId>org.slf4j</groupId>
@@ -135,18 +129,6 @@
<includes>**/*.class</includes>
</configuration>
</execution>
- <execution>
- <id>java21</id>
- <goals>
- <goal>unpack-dependencies</goal>
- </goals>
- <phase>generate-resources</phase>
- <configuration>
- <includeArtifactIds>maven-resolver-transport-jdk-21</includeArtifactIds>
- <outputDirectory>${project.build.directory}/generated-resources/META-INF/versions/21</outputDirectory>
- <includes>**/*.class</includes>
- </configuration>
- </execution>
</executions>
</plugin>
</plugins>
diff --git a/maven-resolver-transport-jdk-parent/pom.xml b/maven-resolver-transport-jdk-parent/pom.xml
index 83d26de..16d4966 100644
--- a/maven-resolver-transport-jdk-parent/pom.xml
+++ b/maven-resolver-transport-jdk-parent/pom.xml
@@ -35,7 +35,6 @@
<modules>
<module>maven-resolver-transport-jdk-8</module>
<module>maven-resolver-transport-jdk-11</module>
- <module>maven-resolver-transport-jdk-21</module>
<module>maven-resolver-transport-jdk</module>
</modules>
</project>
