Fix: TLS Upgrade config propagates to RestTemplate (#662)

Author: CharlesDuboisSAP

cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5Wrapper.java

@@ -8,6 +8,8 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 
+import org.apache.hc.client5.http.config.Configurable;
+import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
 import org.apache.hc.core5.http.ClassicHttpRequest;
@@ -30,11 +32,12 @@
  * and it will append the url configured in the destination.
  */
 @Slf4j
-class ApacheHttpClient5Wrapper extends CloseableHttpClient
+class ApacheHttpClient5Wrapper extends CloseableHttpClient implements Configurable
 {
     private final CloseableHttpClient httpClient;
     @Getter( AccessLevel.PACKAGE )
     private final HttpDestinationProperties destination;
+    private final RequestConfig requestConfig;
 
     @Override
     public void close()
@@ -49,7 +52,10 @@ public void close( final CloseMode closeMode )
         httpClient.close(closeMode);
     }
 
-    ApacheHttpClient5Wrapper( final CloseableHttpClient httpClient, final HttpDestinationProperties destination )
+    ApacheHttpClient5Wrapper(
+        final CloseableHttpClient httpClient,
+        final HttpDestinationProperties destination,
+        final RequestConfig requestConfig )
     {
         this.httpClient = httpClient;
 
@@ -62,6 +68,7 @@ public void close( final CloseMode closeMode )
                 """);
         }
         this.destination = destination;
+        this.requestConfig = requestConfig;
     }
 
     @Override
@@ -86,7 +93,7 @@ ApacheHttpClient5Wrapper withDestination( final HttpDestinationProperties destin
         if( destination == this.destination ) {
             return this;
         }
-        return new ApacheHttpClient5Wrapper(httpClient, destination);
+        return new ApacheHttpClient5Wrapper(httpClient, destination, requestConfig);
     }
 
     ClassicHttpRequest wrapRequest( final ClassicHttpRequest request )
@@ -119,4 +126,10 @@ ClassicHttpRequest wrapRequest( final ClassicHttpRequest request )
 
         return requestBuilder.build();
     }
+
+    @Override
+    public RequestConfig getConfig()
+    {
+        return requestConfig;
+    }
 }

cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java

@@ -77,22 +77,25 @@ public HttpClient createHttpClient( @Nullable final HttpDestinationProperties de
         throws DestinationAccessException,
             HttpClientInstantiationException
     {
-        final CloseableHttpClient httpClient = buildHttpClient(destination);
+        final var requestConfig = getRequestConfig(destination);
+        final CloseableHttpClient httpClient = buildHttpClient(destination, requestConfig);
         if( destination == null ) {
             return httpClient;
         }
 
-        return new ApacheHttpClient5Wrapper(httpClient, destination);
+        return new ApacheHttpClient5Wrapper(httpClient, destination, requestConfig);
     }
 
     @Nonnull
-    private CloseableHttpClient buildHttpClient( @Nullable final HttpDestinationProperties destination )
+    private CloseableHttpClient buildHttpClient(
+        @Nullable final HttpDestinationProperties destination,
+        @Nonnull final RequestConfig requestConfig )
     {
         final HttpClientBuilder builder =
             HttpClients
                 .custom()
                 .setConnectionManager(getConnectionManager(destination))
-                .setDefaultRequestConfig(getRequestConfig(destination))
+                .setDefaultRequestConfig(requestConfig)
                 .setProxy(getProxy(destination));
 
         if( requestInterceptor != null ) {

cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClientWrapperTest.java

@@ -6,6 +6,7 @@
 
 import java.util.List;
 
+import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.junit.jupiter.api.Test;
 
@@ -21,7 +22,7 @@ void testDestinationWrapping()
             DefaultHttpDestination.builder("http://foo.com").headerProviders(c -> List.of()).build();
         final DefaultHttpDestination thirdDestination = DefaultHttpDestination.builder("http://bar.com").build();
         final ApacheHttpClient5Wrapper sut =
-            new ApacheHttpClient5Wrapper(mock(CloseableHttpClient.class), firstDestination);
+            new ApacheHttpClient5Wrapper(mock(CloseableHttpClient.class), firstDestination, mock(RequestConfig.class));
 
         assertThat(sut.withDestination(firstDestination)).isSameAs(sut);
         assertThat(sut.withDestination(firstDestination)).isNotSameAs(sut.withDestination(secondDestination));

datamodel/openapi/openapi-core/src/test/java/com/sap/cloud/sdk/services/openapi/apiclient/ApiClientViaConstructorTest.java

@@ -4,6 +4,13 @@
 
 package com.sap.cloud.sdk.services.openapi.apiclient;
 
+import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.ok;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.springframework.test.web.client.match.MockRestRequestMatchers.*;
 
@@ -18,6 +25,7 @@
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
+import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
 import org.springframework.test.web.client.ExpectedCount;
 import org.springframework.test.web.client.MockRestServiceServer;
 import org.springframework.test.web.client.response.MockRestResponseCreators;
@@ -27,8 +35,14 @@
 import org.springframework.web.util.UriComponentsBuilder;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5FactoryBuilder;
+import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5FactoryBuilder.TlsUpgrade;
+import com.sap.cloud.sdk.cloudplatform.connectivity.DefaultHttpDestination;
 import com.sap.cloud.sdk.services.openapi.core.AbstractOpenApiService;
 
+@WireMockTest
 class ApiClientViaConstructorTest
 {
     private static final String RELATIVE_PATH = "/apiEndpoint";
@@ -121,6 +135,45 @@ void testApiClientWithQueryParams()
         server.verify();
     }
 
+    @Test
+    void testHttpRequestConfigIsTransmitted( WireMockRuntimeInfo wm )
+    {
+        httpRequest(TlsUpgrade.DISABLED, wm.getHttpBaseUrl());
+        verify(getRequestedFor(anyUrl()).withoutHeader("Upgrade"));
+
+        httpRequest(TlsUpgrade.ENABLED, wm.getHttpBaseUrl());
+        verify(getRequestedFor(anyUrl()).withHeader("Upgrade", equalTo("TLS/1.2")));
+    }
+
+    private static void httpRequest( TlsUpgrade toggle, String url )
+    {
+        var sut = new ApacheHttpClient5FactoryBuilder().tlsUpgrade(toggle).build();
+        var httpClient = sut.createHttpClient(DefaultHttpDestination.builder(url).build());
+        var clientHttpRequestFactory = new HttpComponentsClientHttpRequestFactory(httpClient);
+        var restTemplate = new RestTemplate(clientHttpRequestFactory);
+        var apiClient = new ApiClient(restTemplate);
+        apiClient.setBasePath(url);
+
+        stubFor(get(anyUrl()).willReturn(ok("success")));
+
+        assertThat(
+            apiClient
+                .invokeAPI(
+                    "/apiEndpoint",
+                    HttpMethod.GET,
+                    null,
+                    null,
+                    new HttpHeaders(),
+                    null,
+                    null,
+                    null,
+                    null,
+                    new ParameterizedTypeReference<String>()
+                    {
+                    }))
+            .isEqualTo("success");
+    }
+
     private static class MyDto
     {
         @JsonProperty( "Return" )

release_notes.md

@@ -24,4 +24,4 @@
 
 ### 🐛 Fixed Issues
 
-- 
+- Fix ApacheHttpClient5Wrapper to propagate the configuration to Spring RestTemplate.