Skip to content

Commit 38199ba

Browse files
andrewazoresandrewazores
authored
chore(httpclient): upgrade to Apache httpclient5 (#767)
* chore(httpclient): upgrade to Apache httpclient5 * configurable HTTP retry time * refactor, configurable preemptive auth
1 parent 48638ad commit 38199ba

File tree

13 files changed

+176
-126
lines changed

13 files changed

+176
-126
lines changed

‎pom.xml‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@
8181

8282
<org.apache.commons.lang3.version>3.18.0</org.apache.commons.lang3.version>
8383
<org.apache.commons.io.version>2.21.0</org.apache.commons.io.version>
84-
<org.apache.httpcomponents.httpclient.version>4.5.14</org.apache.httpcomponents.httpclient.version>
85-
<org.apache.httpcomponents.httpmime.version>${org.apache.httpcomponents.httpclient.version}</org.apache.httpcomponents.httpmime.version>
84+
<org.apache.httpcomponents.httpclient.version>5.4.4</org.apache.httpcomponents.httpclient.version>
8685
<com.fasterxml.jackson.version>2.20.0</com.fasterxml.jackson.version>
8786
<io.smallrye.config.version>3.10.2</io.smallrye.config.version>
8887
<org.slf4j.version>2.0.17</org.slf4j.version>
@@ -208,15 +207,10 @@
208207
<version>${org.apache.commons.io.version}</version>
209208
</dependency>
210209
<dependency>
211-
<groupId>org.apache.httpcomponents</groupId>
212-
<artifactId>httpclient</artifactId>
210+
<groupId>org.apache.httpcomponents.client5</groupId>
211+
<artifactId>httpclient5</artifactId>
213212
<version>${org.apache.httpcomponents.httpclient.version}</version>
214213
</dependency>
215-
<dependency>
216-
<groupId>org.apache.httpcomponents</groupId>
217-
<artifactId>httpmime</artifactId>
218-
<version>${org.apache.httpcomponents.httpmime.version}</version>
219-
</dependency>
220214
<dependency>
221215
<groupId>com.fasterxml.jackson.core</groupId>
222216
<artifactId>jackson-databind</artifactId>

‎src/main/java/io/cryostat/agent/CallbackResolver.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
import io.cryostat.agent.ConfigModule.CallbackCandidate;
3333

34-
import org.apache.http.client.utils.URIBuilder;
34+
import org.apache.hc.core5.net.URIBuilder;
3535
import org.projectnessie.cel.extension.StringsLib;
3636
import org.projectnessie.cel.tools.Script;
3737
import org.projectnessie.cel.tools.ScriptException;

‎src/main/java/io/cryostat/agent/ConfigModule.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ public abstract class ConfigModule {
137137
"cryostat.agent.webclient.tls.client-auth.key-manager.type";
138138
public static final String CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT =
139139
"cryostat.agent.webclient.response.retry-count";
140+
public static final String CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_TIME =
141+
"cryostat.agent.webclient.response.retry-time-seconds";
142+
public static final String CRYOSTAT_AGENT_WEBCLIENT_HTTP_USE_PREEMPTIVE_AUTHENTICATION =
143+
"cryostat.agent.webclient.http.use-preemptive-authentication";
140144

141145
public static final String CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED =
142146
"cryostat.agent.webclient.tls.required";
@@ -650,6 +654,22 @@ public static int provideCryostatAgentWebclientResponseRetryCount(Config config)
650654
return config.getValue(CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT, int.class);
651655
}
652656

657+
@Provides
658+
@Singleton
659+
@Named(CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_TIME)
660+
public static int provideCryostatAgentWebclientResponseRetryTime(Config config) {
661+
return config.getValue(CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_TIME, int.class);
662+
}
663+
664+
@Provides
665+
@Singleton
666+
@Named(CRYOSTAT_AGENT_WEBCLIENT_HTTP_USE_PREEMPTIVE_AUTHENTICATION)
667+
public static boolean provideCryostatAgentWebclientHttpUsePreemptiveAuthentication(
668+
Config config) {
669+
return config.getValue(
670+
CRYOSTAT_AGENT_WEBCLIENT_HTTP_USE_PREEMPTIVE_AUTHENTICATION, boolean.class);
671+
}
672+
653673
@Provides
654674
@Singleton
655675
@Named(CRYOSTAT_AGENT_WEBSERVER_HOST)

‎src/main/java/io/cryostat/agent/CryostatClient.java‎

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import java.util.concurrent.ExecutionException;
3636
import java.util.concurrent.Executor;
3737
import java.util.function.Function;
38-
import java.util.function.Supplier;
3938

4039
import io.cryostat.agent.FlightRecorderHelper.ConfigurationInfo;
4140
import io.cryostat.agent.FlightRecorderHelper.TemplatedRecording;
@@ -52,21 +51,22 @@
5251
import jdk.jfr.Recording;
5352
import org.apache.commons.io.FileUtils;
5453
import org.apache.commons.io.input.CountingInputStream;
55-
import org.apache.http.HttpHeaders;
56-
import org.apache.http.HttpResponse;
57-
import org.apache.http.client.HttpClient;
58-
import org.apache.http.client.methods.HttpDelete;
59-
import org.apache.http.client.methods.HttpGet;
60-
import org.apache.http.client.methods.HttpPost;
61-
import org.apache.http.client.methods.HttpRequestBase;
62-
import org.apache.http.client.methods.HttpUriRequest;
63-
import org.apache.http.entity.ContentType;
64-
import org.apache.http.entity.StringEntity;
65-
import org.apache.http.entity.mime.FormBodyPartBuilder;
66-
import org.apache.http.entity.mime.MultipartEntityBuilder;
67-
import org.apache.http.entity.mime.content.ByteArrayBody;
68-
import org.apache.http.entity.mime.content.InputStreamBody;
69-
import org.apache.http.entity.mime.content.StringBody;
54+
import org.apache.hc.client5.http.classic.HttpClient;
55+
import org.apache.hc.client5.http.classic.methods.HttpDelete;
56+
import org.apache.hc.client5.http.classic.methods.HttpGet;
57+
import org.apache.hc.client5.http.classic.methods.HttpPost;
58+
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
59+
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
60+
import org.apache.hc.client5.http.entity.mime.ByteArrayBody;
61+
import org.apache.hc.client5.http.entity.mime.FormBodyPartBuilder;
62+
import org.apache.hc.client5.http.entity.mime.InputStreamBody;
63+
import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder;
64+
import org.apache.hc.client5.http.entity.mime.StringBody;
65+
import org.apache.hc.core5.http.ClassicHttpResponse;
66+
import org.apache.hc.core5.http.ContentType;
67+
import org.apache.hc.core5.http.HttpHeaders;
68+
import org.apache.hc.core5.http.HttpHost;
69+
import org.apache.hc.core5.http.io.entity.StringEntity;
7070
import org.slf4j.Logger;
7171
import org.slf4j.LoggerFactory;
7272

@@ -81,8 +81,8 @@ public class CryostatClient {
8181

8282
private final Executor executor;
8383
private final ObjectMapper mapper;
84+
private final HttpHost host;
8485
private final HttpClient http;
85-
private final Supplier<Optional<String>> authorizationSupplier;
8686

8787
private final String appName;
8888
private final String instanceId;
@@ -94,16 +94,15 @@ public class CryostatClient {
9494
Executor executor,
9595
ObjectMapper mapper,
9696
HttpClient http,
97-
Supplier<Optional<String>> authorizationSupplier,
9897
String instanceId,
9998
String jvmId,
10099
String appName,
101100
URI baseUri,
102101
String realm) {
103102
this.executor = executor;
104103
this.mapper = mapper;
104+
this.host = HttpHost.create(baseUri);
105105
this.http = http;
106-
this.authorizationSupplier = authorizationSupplier;
107106
this.instanceId = instanceId;
108107
this.jvmId = jvmId;
109108
this.appName = appName;
@@ -286,7 +285,7 @@ private CompletableFuture<Integer> submitCredentials(
286285
res -> {
287286
if (!isOkStatus(res)) {
288287
try {
289-
if (res.getStatusLine().getStatusCode() == 409) {
288+
if (res.getCode() == 409) {
290289
int queried = queryExistingCredentials(callback).get();
291290
if (queried >= 0) {
292291
return queried;
@@ -393,9 +392,9 @@ public CompletableFuture<Void> pushHeapDump(Path heapDump, String requestId)
393392
log.trace(
394393
"{} {} ({} -> {}): {}/{}",
395394
req.getMethod(),
396-
res.getStatusLine().getStatusCode(),
395+
res.getCode(),
397396
heapDump.getFileName().toString(),
398-
req.getURI(),
397+
req.getRequestUri(),
399398
FileUtils.byteCountToDisplaySize(is.getByteCount()),
400399
Duration.between(start, finish));
401400
assertOkStatus(req, res);
@@ -485,9 +484,9 @@ public CompletableFuture<Void> upload(
485484
log.trace(
486485
"{} {} ({} -> {}): {}/{}",
487486
req.getMethod(),
488-
res.getStatusLine().getStatusCode(),
487+
res.getCode(),
489488
fileName,
490-
req.getURI(),
489+
req.getRequestUri(),
491490
FileUtils.byteCountToDisplaySize(is.getByteCount()),
492491
Duration.between(start, finish));
493492
assertOkStatus(req, res);
@@ -496,24 +495,19 @@ public CompletableFuture<Void> upload(
496495
.whenComplete((v, t) -> req.reset());
497496
}
498497

499-
private HttpResponse logResponse(HttpRequestBase req, HttpResponse res) {
500-
log.trace("{} {} : {}", req.getMethod(), req.getURI(), res.getStatusLine().getStatusCode());
498+
private ClassicHttpResponse logResponse(HttpUriRequestBase req, ClassicHttpResponse res) {
499+
log.trace("{} {} : {}", req.getMethod(), req.getRequestUri(), res.getCode());
501500
return res;
502501
}
503502

504-
private <T> CompletableFuture<T> supply(HttpRequestBase req, Function<HttpResponse, T> fn) {
505-
// FIXME Apache httpclient 4 does not support Bearer token auth easily, so we explicitly set
506-
// the header here. This is a form of preemptive auth - the token is always sent with the
507-
// request. It would be better to attempt to send the request to the server first and see if
508-
// it responds with an auth challenge, and then send the auth information we have, and use
509-
// the client auth cache. This flow is supported for Bearer tokens in httpclient 5.
510-
authorizationSupplier.get().ifPresent(v -> req.addHeader(HttpHeaders.AUTHORIZATION, v));
503+
private <T> CompletableFuture<T> supply(
504+
HttpUriRequestBase req, Function<ClassicHttpResponse, T> fn) {
511505
return CompletableFuture.supplyAsync(() -> fn.apply(executeQuiet(req)), executor);
512506
}
513507

514-
private HttpResponse executeQuiet(HttpUriRequest req) {
508+
private ClassicHttpResponse executeQuiet(HttpUriRequest req) {
515509
try {
516-
return http.execute(req);
510+
return http.execute(host, req);
517511
} catch (IOException ioe) {
518512
throw new CompletionException(ioe);
519513
}
@@ -530,18 +524,18 @@ private String selfMatchExpression(URI callback) {
530524
callback, instanceId);
531525
}
532526

533-
private boolean isOkStatus(HttpResponse res) {
534-
int sc = res.getStatusLine().getStatusCode();
527+
private boolean isOkStatus(ClassicHttpResponse res) {
528+
int sc = res.getCode();
535529
// 2xx is OK, 3xx is redirect range so allow those too
536530
return 200 <= sc && sc < 400;
537531
}
538532

539-
private HttpResponse assertOkStatus(HttpRequestBase req, HttpResponse res) {
540-
int sc = res.getStatusLine().getStatusCode();
533+
private ClassicHttpResponse assertOkStatus(HttpUriRequestBase req, ClassicHttpResponse res) {
534+
int sc = res.getCode();
541535
if (!isOkStatus(res)) {
542-
URI uri = req.getURI();
543-
log.error("Non-OK response ({}) on HTTP API {}", sc, uri);
544536
try {
537+
URI uri = req.getUri();
538+
log.error("Non-OK response ({}) on HTTP API {}", sc, uri);
545539
throw new HttpException(
546540
sc,
547541
new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), null, null));

0 commit comments

Comments
 (0)