Skip to content

chore: Migrate connectivity-destination-service to HttpClient5 #1028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CharlesDuboisSAP wants to merge 5 commits into
base: main
Choose a base branch
from
Open

chore: Migrate connectivity-destination-service to HttpClient5 #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5355c37
chore: Migrate `connectivity-destination-service` to HttpClient5
CharlesDuboisSAP Dec 10, 2025
40d74d7
formatting
CharlesDuboisSAP Dec 16, 2025
60e749e
Merge branch 'main' into httpclient5-destination
CharlesDuboisSAP Jan 21, 2026
1ec4801
Merge branch 'main' into httpclient5-destination
CharlesDuboisSAP Jan 30, 2026
a9c1b64
TransparentProxy
CharlesDuboisSAP Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 12 additions & 24 deletions cloudplatform/connectivity-destination-service/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@
<groupId>com.sap.cloud.sdk.cloudplatform</groupId>
<artifactId>cloudplatform-connectivity</artifactId>
</dependency>
<dependency>
<groupId>com.sap.cloud.sdk.cloudplatform</groupId>
<artifactId>connectivity-apache-httpclient4</artifactId>
</dependency>
<dependency>
<groupId>com.sap.cloud.sdk.cloudplatform</groupId>
<artifactId>connectivity-apache-httpclient5</artifactId>
</dependency>
<!-- Included by default to serve as the default implementation -->
<dependency>
<groupId>com.sap.cloud.sdk.cloudplatform</groupId>
Expand Down Expand Up @@ -97,26 +97,14 @@
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpcore</artifactId>
<exclusions>
<exclusion>
<artifactId>commons-logging</artifactId>
<groupId>commons-logging</groupId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<exclusions>
<exclusion>
<artifactId>commons-logging</artifactId>
<groupId>commons-logging</groupId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents.core5</groupId>
<artifactId>httpcore5</artifactId>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
</dependency>
<dependency>
<groupId>io.vavr</groupId>
<artifactId>vavr</artifactId>
Expand Down
14 changes: 5 additions & 9 deletions ...e/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AuthTokenHeaderProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import javax.annotation.Nonnull;

import org.apache.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHeaders;

import com.google.common.base.Strings;
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
Expand Down Expand Up @@ -96,13 +96,9 @@ private static List<Header> getDestinationHeaders( @Nonnull final List<Destinati

private static boolean isAuthTokenExpected( @Nonnull final AuthenticationType authType )
{
switch( authType ) {
case NO_AUTHENTICATION:
case PRINCIPAL_PROPAGATION:
case CLIENT_CERTIFICATE_AUTHENTICATION:
return false;
default:
return true;
}
return switch( authType ) {
case NO_AUTHENTICATION, PRINCIPAL_PROPAGATION, CLIENT_CERTIFICATE_AUTHENTICATION -> false;
default -> true;
};
}
}
84 changes: 47 additions & 37 deletions ...src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.impl.classic.BasicHttpClientResponseHandler;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpStatus;

import com.google.common.base.Strings;
import com.sap.cloud.environment.servicebinding.api.DefaultServiceBindingAccessor;
Expand All @@ -32,6 +32,7 @@
import io.vavr.control.Try;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Slf4j
Expand Down Expand Up @@ -126,53 +127,62 @@ String getConfigurationAsJson(
serviceDestinationLoader.apply(strategy.behalf()),
() -> "Destination for Destination Service on behalf of " + strategy.behalf() + " not found.");

final HttpUriRequest request = prepareRequest(servicePath, strategy);
final ClassicHttpRequest request = prepareRequest(servicePath, strategy);

final HttpResponse response;
try {
response = HttpClientAccessor.getHttpClient(serviceDestination).execute(request);
return ApacheHttpClient5Accessor
.getHttpClient(serviceDestination)
.execute(request, new DestinationHttpClientResponseHandler(request));
}
catch( final IOException e ) {
throw new DestinationAccessException(e);
}
return handleResponse(request, response);
}

@Nonnull
private static String handleResponse( final HttpUriRequest request, final HttpResponse response )
@RequiredArgsConstructor( access = AccessLevel.PRIVATE )
static class DestinationHttpClientResponseHandler extends BasicHttpClientResponseHandler
{
final StatusLine status = response.getStatusLine();
final int statusCode = status.getStatusCode();
final String reasonPhrase = status.getReasonPhrase();
final ClassicHttpRequest request;

log.debug("Destination service returned HTTP status {} ({})", statusCode, reasonPhrase);
@Override
public String handleResponse( final ClassicHttpResponse response )
{
final int statusCode = response.getCode();
final String reasonPhrase = response.getReasonPhrase();

Try<String> maybeBody = Try.of(() -> HttpEntityUtil.getResponseBody(response));
if( maybeBody.isFailure() ) {
final var ex =
new DestinationAccessException("Failed to read body from HTTP response", maybeBody.getCause());
maybeBody = Try.failure(ex);
}
log.debug("Destination service returned HTTP status {} ({})", statusCode, reasonPhrase);

if( statusCode == HttpStatus.SC_OK ) {
final var ex = new DestinationAccessException("Failed to get destinations: no body returned in response.");
maybeBody = maybeBody.filter(it -> !Strings.isNullOrEmpty(it), () -> ex);
return maybeBody.get();
}
Try<String> maybeBody = Try.of(() -> handleEntity(response.getEntity()));
if( maybeBody.isFailure() ) {
final var ex =
new DestinationAccessException("Failed to read body from HTTP response", maybeBody.getCause());
maybeBody = Try.failure(ex);
}

final String requestUri = request.getURI().getPath();
if( statusCode == HttpStatus.SC_NOT_FOUND ) {
throw new DestinationNotFoundException(null, "Destination could not be found for path " + requestUri + ".");
if( statusCode == HttpStatus.SC_OK ) {
final var ex =
new DestinationAccessException("Failed to get destinations: no body returned in response.");
maybeBody = maybeBody.filter(it -> !Strings.isNullOrEmpty(it), () -> ex);
return maybeBody.get();
}

final String requestUri = request.getPath();
if( statusCode == HttpStatus.SC_NOT_FOUND ) {
throw new DestinationNotFoundException(
null,
"Destination could not be found for path " + requestUri + ".");
}
final String message =
"Failed to get destinations: destination service responded with HTTP status %s (%S) at '%s'."
.formatted(statusCode, reasonPhrase, requestUri);
final String messageWithBody =
message + " Body: %s".formatted(maybeBody.getOrElseGet(Throwable::getMessage));
log.error(messageWithBody);
throw new DestinationAccessException(message);
}
final String message =
"Failed to get destinations: destination service responded with HTTP status %s (%S) at '%s'."
.formatted(statusCode, reasonPhrase, requestUri);
final String messageWithBody = message + " Body: %s".formatted(maybeBody.getOrElseGet(Throwable::getMessage));
log.error(messageWithBody);
throw new DestinationAccessException(message);
}

private HttpUriRequest prepareRequest( final String servicePath, final DestinationRetrievalStrategy strategy )
private ClassicHttpRequest prepareRequest( final String servicePath, final DestinationRetrievalStrategy strategy )
{
final URI requestUri;
try {
Expand All @@ -183,7 +193,7 @@ private HttpUriRequest prepareRequest( final String servicePath, final Destinati
}

log.debug("Querying Destination Service via URI {}.", requestUri);
final HttpUriRequest request = new HttpGet(requestUri);
final ClassicHttpRequest request = new HttpGet(requestUri);

if( !servicePath.startsWith(DestinationService.PATH_DEFAULT)
&& !servicePath.startsWith(DestinationService.PATH_V2) ) {
Expand Down
34 changes: 20 additions & 14 deletions ...-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@
import javax.annotation.Nonnull;

import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.http.HttpMessage;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpHead;
import org.apache.hc.client5.http.classic.HttpClient;
import org.apache.hc.client5.http.classic.methods.HttpHead;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpMessage;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.io.entity.EntityUtils;

import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException;
Expand Down Expand Up @@ -76,7 +79,7 @@ public class TransparentProxy implements DestinationLoader
private static final String SET_COOKIE_HEADER = "Set-Cookie";
private static final Integer DEFAULT_PORT = 80;
private static final String SCHEME_SEPARATOR = "://";
private static final String HTTP_SCHEME = org.apache.http.HttpHost.DEFAULT_SCHEME_NAME + SCHEME_SEPARATOR;
private static final String HTTP_SCHEME = HttpHost.DEFAULT_SCHEME.getId() + SCHEME_SEPARATOR;
private static final String PORT_SEPARATOR = ":";
private static final String HOST_CONTAINS_PATH_ERROR_MESSAGE_TEMPLATE =
"Host '%s' contains a path '%s'. Paths are not allowed in host registration.";
Expand Down Expand Up @@ -279,7 +282,7 @@ private static String normalizeHostWithScheme( @Nonnull final String host )
private static TransparentProxyDestination verifyDestination(
@Nonnull final TransparentProxyDestination destination )
{
final HttpClient httpClient = HttpClientAccessor.getHttpClient(destination);
final HttpClient httpClient = ApacheHttpClient5Accessor.getHttpClient(destination);
final URI destinationUri = URI.create(uri);
final HttpHead headRequest = new HttpHead(destinationUri);
final String destinationName = getDestinationName(destination, destinationUri);
Expand All @@ -288,8 +291,8 @@ private static TransparentProxyDestination verifyDestination(
.debug(
"Performing HEAD request to destination with name {} to verify the destination exists",
destinationName);
final Supplier<HttpResponse> tpDestinationVerifierSupplier = prepareSupplier(httpClient, headRequest);
HttpResponse response = null;
final Supplier<ClassicHttpResponse> tpDestinationVerifierSupplier = prepareSupplier(httpClient, headRequest);
ClassicHttpResponse response = null;
try {
response = ResilienceDecorator.executeSupplier(tpDestinationVerifierSupplier, resilienceConfiguration);
verifyTransparentProxyResponse(response, destinationName);
Expand All @@ -303,7 +306,7 @@ private static TransparentProxyDestination verifyDestination(
finally {
if( response != null ) {
try {
org.apache.http.util.EntityUtils.consume(response.getEntity());
EntityUtils.consume(response.getEntity());
}
catch( IOException e ) {
log.warn("Failed to close HTTP response", e);
Expand Down Expand Up @@ -340,15 +343,15 @@ private static void verifyTransparentProxyResponse( final HttpResponse response,
throw new DestinationAccessException(FAILED_TO_VERIFY_DESTINATION + "Response is null.");
}
if( response.containsHeader(SET_COOKIE_HEADER) ) {
final org.apache.http.Header[] header = response.getHeaders(SET_COOKIE_HEADER);
final org.apache.hc.core5.http.Header[] header = response.getHeaders(SET_COOKIE_HEADER);
final List<String> cookieNames = Arrays.stream(header).map(h -> h.getValue().split("=", 2)[0]).toList();
log
.warn(
"received set-cookie headers as part of destination health check. This is unexpected and may have side effects for your application. The following cookies were set: {}",
cookieNames);
}

final int statusCode = response.getStatusLine().getStatusCode();
final int statusCode = response.getCode();
final String errorInternalCode = getHeaderValue(response, X_ERROR_INTERNAL_CODE_HEADER);
final String errorMessage = getHeaderValue(response, X_ERROR_MESSAGE_HEADER);
final String errorOrigin = getHeaderValue(response, X_ERROR_ORIGIN_HEADER);
Expand Down Expand Up @@ -378,11 +381,14 @@ private static void verifyTransparentProxyResponse( final HttpResponse response,
}

@Nonnull
private static Supplier<HttpResponse> prepareSupplier( final HttpClient httpClient, final HttpHead headRequest )
private static
Supplier<ClassicHttpResponse>
prepareSupplier( final HttpClient httpClient, final HttpHead headRequest )
{
return () -> {
try {
return httpClient.execute(headRequest);
// migration from apache httpclient4 to httpclient5 by possibly adding a response handler to httpClient.execute(headRequest);
return httpClient.execute(headRequest, classicHttpResponse -> classicHttpResponse);
}
catch( final IOException e ) {
throw new DestinationAccessException(FAILED_TO_VERIFY_DESTINATION, e);
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/AuthTokenHeaderProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import java.util.Arrays;
import java.util.Collection;

import org.apache.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHeaders;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand Down
Loading
Loading