You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2024/08/02 13:19:38 UTC
(maven-resolver) branch master updated: [MRESOLVER-584] JDK transport HTTP/2 GOAWAY improvement (#532)
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 78d8eb62 [MRESOLVER-584] JDK transport HTTP/2 GOAWAY improvement (#532)
78d8eb62 is described below
commit 78d8eb623efb9bbf918ae007eb05ce20dab8c816
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Fri Aug 2 15:19:33 2024 +0200
[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
---
.../aether/transport/jdk/JdkTransporter.java | 275 +++++++++------------
.../aether/transport/jdk/JdkTransporterCloser.java | 2 +-
.../maven-resolver-transport-jdk-21/pom.xml | 98 --------
.../aether/transport/jdk/JdkTransporterCloser.java | 32 ---
.../maven-resolver-transport-jdk/pom.xml | 18 --
maven-resolver-transport-jdk-parent/pom.xml | 1 -
6 files changed, 122 insertions(+), 304 deletions(-)
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 32da764e..15a37326 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 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
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 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
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 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
}
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,176 +412,134 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
@Override
protected void implClose() {
- // no-op
- }
-
- private InetAddress getHttpLocalAddress(RepositorySystemSession session, RemoteRepository repository) {
- String bindAddress = ConfigUtils.getString(
- session,
- null,
- ConfigurationProperties.HTTP_LOCAL_ADDRESS + "." + repository.getId(),
- ConfigurationProperties.HTTP_LOCAL_ADDRESS);
- if (bindAddress == null) {
- return null;
- }
- try {
- return InetAddress.getByName(bindAddress);
- } catch (UnknownHostException uhe) {
- throw new IllegalArgumentException(
- "Given bind address (" + bindAddress + ") cannot be resolved for remote repository " + repository,
- uhe);
+ if (client != null) {
+ JdkTransporterCloser.closer(client).run();
}
}
- /**
- * Visible for testing.
- */
- static final String HTTP_INSTANCE_KEY_PREFIX = JdkTransporterFactory.class.getName() + ".http.";
+ private static HttpClient createClient(
+ RepositorySystemSession session, RemoteRepository repository, boolean insecure) throws Exception {
- private HttpClient getOrCreateClient(RepositorySystemSession session, RemoteRepository repository, int javaVersion)
- throws NoTransporterException {
- final String instanceKey = HTTP_INSTANCE_KEY_PREFIX + repository.getId();
+ 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);
- final String httpsSecurityMode = ConfigUtils.getString(
- session,
- ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT,
- ConfigurationProperties.HTTPS_SECURITY_MODE + "." + repository.getId(),
- ConfigurationProperties.HTTPS_SECURITY_MODE);
+ String username = repoAuthContext.get(AuthenticationContext.USERNAME);
+ String password = repoAuthContext.get(AuthenticationContext.PASSWORD);
- if (!ConfigurationProperties.HTTPS_SECURITY_MODE_DEFAULT.equals(httpsSecurityMode)
- && !ConfigurationProperties.HTTPS_SECURITY_MODE_INSECURE.equals(httpsSecurityMode)) {
- throw new IllegalArgumentException("Unsupported '" + httpsSecurityMode + "' HTTPS security mode.");
+ authentications.put(
+ Authenticator.RequestorType.SERVER,
+ new PasswordAuthentication(username, password.toCharArray()));
+ }
}
- 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);
+ if (sslContext == null) {
+ if (insecure) {
+ sslContext = SSLContext.getInstance("TLS");
+ X509ExtendedTrustManager tm = new X509ExtendedTrustManager() {
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType) {}
- String username = repoAuthContext.get(AuthenticationContext.USERNAME);
- String password = repoAuthContext.get(AuthenticationContext.PASSWORD);
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType) {}
- authentications.put(
- Authenticator.RequestorType.SERVER,
- new PasswordAuthentication(username, password.toCharArray()));
- }
- }
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) {}
- 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();
- }
- }
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {}
- 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);
- }
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
- setLocalAddress(builder, () -> getHttpLocalAddress(session, repository));
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
- if (repository.getProxy() != null) {
- ProxySelector proxy = ProxySelector.of(new InetSocketAddress(
- repository.getProxy().getHost(),
- repository.getProxy().getPort()));
+ @Override
+ public X509Certificate[] getAcceptedIssuers() {
+ return null;
+ }
+ };
+ sslContext.init(null, new X509TrustManager[] {tm}, null);
+ } else {
+ sslContext = SSLContext.getDefault();
+ }
+ }
- 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);
+ 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);
+ }
- authentications.put(
- Authenticator.RequestorType.PROXY,
- new PasswordAuthentication(username, password.toCharArray()));
- }
- }
- }
+ setLocalAddress(builder, () -> getHttpLocalAddress(session, repository));
- if (!authentications.isEmpty()) {
- builder.authenticator(new Authenticator() {
- @Override
- protected PasswordAuthentication getPasswordAuthentication() {
- return authentications.get(getRequestorType());
- }
- });
- }
+ if (repository.getProxy() != null) {
+ ProxySelector proxy = ProxySelector.of(new InetSocketAddress(
+ repository.getProxy().getHost(), repository.getProxy().getPort()));
- 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.");
- }
+ 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);
- return result;
- } catch (Exception e) {
- throw new WrapperEx(e);
+ 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());
}
});
- } catch (WrapperEx e) {
- throw new NoTransporterException(repository, e.getCause());
}
+
+ return builder.build();
}
- private void setLocalAddress(HttpClient.Builder builder, Supplier<InetAddress> addressSupplier) {
+ private static InetAddress getHttpLocalAddress(RepositorySystemSession session, RemoteRepository repository) {
+ String bindAddress = ConfigUtils.getString(
+ session,
+ null,
+ ConfigurationProperties.HTTP_LOCAL_ADDRESS + "." + repository.getId(),
+ ConfigurationProperties.HTTP_LOCAL_ADDRESS);
+ if (bindAddress == null) {
+ return null;
+ }
+ try {
+ return InetAddress.getByName(bindAddress);
+ } catch (UnknownHostException uhe) {
+ throw new IllegalArgumentException(
+ "Given bind address (" + bindAddress + ") cannot be resolved for remote repository " + repository,
+ uhe);
+ }
+ }
+
+ private static void setLocalAddress(HttpClient.Builder builder, Supplier<InetAddress> addressSupplier) {
try {
final InetAddress address = addressSupplier.get();
if (address == null) {
@@ -586,10 +559,4 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
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 a273c014..69f839a7 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 @@ import java.net.http.HttpClient;
*/
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 cdf0e305..00000000
--- 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 b7da9a81..00000000
--- 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 7fe556a5..97f03208 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 83d26deb..16d4966e 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>