Skip to content

Commit

Permalink
[MRESOLVER-584] JDK transport HTTP/2 GOAWAY improvement (#532)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cstamas committed Aug 2, 2024
1 parent 4a9f6e8 commit 78d8eb6
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 304 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -386,7 +401,7 @@ protected void implPut(PutTask task) throws Exception {
}

private <T> HttpResponse<T> send(HttpRequest request, HttpResponse.BodyHandler<T> responseBodyHandler)
throws IOException, InterruptedException {
throws Exception {
maxConcurrentRequests.acquire();
try {
return client.send(request, responseBodyHandler);
Expand All @@ -397,176 +412,134 @@ private <T> HttpResponse<T> send(HttpRequest request, HttpResponse.BodyHandler<T

@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) {
Expand All @@ -586,10 +559,4 @@ private void setLocalAddress(HttpClient.Builder builder, Supplier<InetAddress> a
throw new IllegalStateException(e);
}
}

private static final class WrapperEx extends RuntimeException {
private WrapperEx(Throwable cause) {
super(cause);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 78d8eb6

Please sign in to comment.