From 990a34074ac609e259149c993d6cc308ad9a7a88 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Wed, 14 Sep 2022 15:36:08 +0200 Subject: [PATCH] Upgrade RestTemplate to HttpClient 5 This commit upgrades the HttpComponentClientHttpRequestFactory and related types from HttpClient version 4.5 to 5. Closes gh-28925 --- framework-platform/framework-platform.gradle | 1 - spring-web/spring-web.gradle | 3 - .../HttpComponentsClientHttpRequest.java | 49 +++--- ...ttpComponentsClientHttpRequestFactory.java | 143 ++++++++---------- .../HttpComponentsClientHttpResponse.java | 33 ++-- ...pComponentsStreamingClientHttpRequest.java | 74 +++++---- ...mponentsClientHttpRequestFactoryTests.java | 76 ++++------ .../ServerHttpsRequestIntegrationTests.java | 21 ++- spring-webflux/spring-webflux.gradle | 3 - spring-webmvc/spring-webmvc.gradle | 4 +- 10 files changed, 189 insertions(+), 218 deletions(-) diff --git a/framework-platform/framework-platform.gradle b/framework-platform/framework-platform.gradle index e44b5e53f05c..4c2494469687 100644 --- a/framework-platform/framework-platform.gradle +++ b/framework-platform/framework-platform.gradle @@ -94,7 +94,6 @@ dependencies { api("org.apache.derby:derbyclient:10.14.2.0") api("org.apache.httpcomponents.client5:httpclient5:5.1.3") api("org.apache.httpcomponents.core5:httpcore5-reactive:5.1.3") - api("org.apache.httpcomponents:httpclient:4.5.13") api("org.apache.poi:poi-ooxml:5.2.2") api("org.apache.tomcat.embed:tomcat-embed-core:10.0.23") api("org.apache.tomcat.embed:tomcat-embed-websocket:10.0.23") diff --git a/spring-web/spring-web.gradle b/spring-web/spring-web.gradle index b52b6431d866..846080616d95 100644 --- a/spring-web/spring-web.gradle +++ b/spring-web/spring-web.gradle @@ -40,9 +40,6 @@ dependencies { optional("org.eclipse.jetty:jetty-reactive-httpclient") optional('org.apache.httpcomponents.client5:httpclient5') optional('org.apache.httpcomponents.core5:httpcore5-reactive') - optional("org.apache.httpcomponents:httpclient") { - exclude group: "commons-logging", module: "commons-logging" - } optional("com.squareup.okhttp3:okhttp") optional("com.fasterxml.woodstox:woodstox-core") optional("com.fasterxml:aalto-xml") diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java index f837391ed950..8ad4af721030 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,18 +18,20 @@ import java.io.IOException; import java.net.URI; +import java.net.URISyntaxException; -import org.apache.http.HttpEntity; -import org.apache.http.HttpEntityEnclosingRequest; -import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.protocol.HTTP; -import org.apache.http.protocol.HttpContext; +import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.io.entity.ByteArrayEntity; +import org.apache.hc.core5.http.protocol.HttpContext; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** @@ -48,12 +50,12 @@ final class HttpComponentsClientHttpRequest extends AbstractBufferingClientHttpR private final HttpClient httpClient; - private final HttpUriRequest httpRequest; + private final ClassicHttpRequest httpRequest; private final HttpContext httpContext; - HttpComponentsClientHttpRequest(HttpClient client, HttpUriRequest request, HttpContext context) { + HttpComponentsClientHttpRequest(HttpClient client, ClassicHttpRequest request, HttpContext context) { this.httpClient = client; this.httpRequest = request; this.httpContext = context; @@ -73,7 +75,12 @@ public String getMethodValue() { @Override public URI getURI() { - return this.httpRequest.getURI(); + try { + return this.httpRequest.getUri(); + } + catch (URISyntaxException ex) { + throw new IllegalStateException(ex.getMessage(), ex); + } } HttpContext getHttpContext() { @@ -85,28 +92,28 @@ HttpContext getHttpContext() { protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] bufferedOutput) throws IOException { addHeaders(this.httpRequest, headers); - if (this.httpRequest instanceof HttpEntityEnclosingRequest entityEnclosingRequest) { - HttpEntity requestEntity = new ByteArrayEntity(bufferedOutput); - entityEnclosingRequest.setEntity(requestEntity); - } + ContentType contentType = ContentType.parse(headers.getFirst(HttpHeaders.CONTENT_TYPE)); + HttpEntity requestEntity = new ByteArrayEntity(bufferedOutput, contentType); + this.httpRequest.setEntity(requestEntity); HttpResponse httpResponse = this.httpClient.execute(this.httpRequest, this.httpContext); - return new HttpComponentsClientHttpResponse(httpResponse); + Assert.isInstanceOf(ClassicHttpResponse.class, httpResponse, + "HttpResponse not an instance of ClassicHttpResponse"); + return new HttpComponentsClientHttpResponse((ClassicHttpResponse) httpResponse); } - /** * Add the given headers to the given HTTP request. * @param httpRequest the request to add the headers to * @param headers the headers to add */ - static void addHeaders(HttpUriRequest httpRequest, HttpHeaders headers) { + static void addHeaders(ClassicHttpRequest httpRequest, HttpHeaders headers) { headers.forEach((headerName, headerValues) -> { if (HttpHeaders.COOKIE.equalsIgnoreCase(headerName)) { // RFC 6265 String headerValue = StringUtils.collectionToDelimitedString(headerValues, "; "); httpRequest.addHeader(headerName, headerValue); } - else if (!HTTP.CONTENT_LEN.equalsIgnoreCase(headerName) && - !HTTP.TRANSFER_ENCODING.equalsIgnoreCase(headerName)) { + else if (!HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(headerName) && + !HttpHeaders.TRANSFER_ENCODING.equalsIgnoreCase(headerName)) { for (String headerValue : headerValues) { httpRequest.addHeader(headerName, headerValue); } diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java index 5e9773cffc7e..2045742b6cc9 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java @@ -19,23 +19,29 @@ import java.io.Closeable; import java.io.IOException; import java.net.URI; +import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; -import org.apache.http.client.HttpClient; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.Configurable; -import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpHead; -import org.apache.http.client.methods.HttpOptions; -import org.apache.http.client.methods.HttpPatch; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.methods.HttpPut; -import org.apache.http.client.methods.HttpTrace; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.protocol.HttpContext; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.client5.http.classic.methods.HttpDelete; +import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.classic.methods.HttpHead; +import org.apache.hc.client5.http.classic.methods.HttpOptions; +import org.apache.hc.client5.http.classic.methods.HttpPatch; +import org.apache.hc.client5.http.classic.methods.HttpPost; +import org.apache.hc.client5.http.classic.methods.HttpPut; +import org.apache.hc.client5.http.classic.methods.HttpTrace; +import org.apache.hc.client5.http.config.Configurable; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.io.SocketConfig; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Timeout; import org.springframework.beans.factory.DisposableBean; import org.springframework.http.HttpMethod; @@ -60,16 +66,19 @@ */ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequestFactory, DisposableBean { - private HttpClient httpClient; + private static final Log logger = LogFactory.getLog(HttpComponentsClientHttpRequestFactory.class); - @Nullable - private RequestConfig requestConfig; + + private HttpClient httpClient; private boolean bufferRequestBody = true; @Nullable private BiFunction httpContextFactory; + private int connectTimeout = -1; + + private int connectionRequestTimeout = -1; /** * Create a new instance of the {@code HttpComponentsClientHttpRequestFactory} @@ -113,15 +122,15 @@ public HttpClient getHttpClient() { * {@link RequestConfig} instance on a custom {@link HttpClient}. *

This options does not affect connection timeouts for SSL * handshakes or CONNECT requests; for that, it is required to - * use the {@link org.apache.http.config.SocketConfig} on the + * use the {@link SocketConfig} on the * {@link HttpClient} itself. - * @param timeout the timeout value in milliseconds + * @param connectTimeout the timeout value in milliseconds * @see RequestConfig#getConnectTimeout() - * @see org.apache.http.config.SocketConfig#getSoTimeout + * @see SocketConfig#getSoTimeout */ - public void setConnectTimeout(int timeout) { - Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value"); - this.requestConfig = requestConfigBuilder().setConnectTimeout(timeout).build(); + public void setConnectTimeout(int connectTimeout) { + Assert.isTrue(connectTimeout >= 0, "Timeout must be a non-negative value"); + this.connectTimeout = connectTimeout; } /** @@ -134,21 +143,24 @@ public void setConnectTimeout(int timeout) { * @see RequestConfig#getConnectionRequestTimeout() */ public void setConnectionRequestTimeout(int connectionRequestTimeout) { - this.requestConfig = requestConfigBuilder() - .setConnectionRequestTimeout(connectionRequestTimeout).build(); + Assert.isTrue(connectionRequestTimeout >= 0, "Timeout must be a non-negative value"); + this.connectionRequestTimeout = connectionRequestTimeout; } /** - * Set the socket read timeout for the underlying {@link RequestConfig}. - * A timeout value of 0 specifies an infinite timeout. - *

Additional properties can be configured by specifying a - * {@link RequestConfig} instance on a custom {@link HttpClient}. - * @param timeout the timeout value in milliseconds - * @see RequestConfig#getSocketTimeout() + * As of version 6.0, setting this property has no effect. + * + *

To change the socket read timeout, use {@link SocketConfig.Builder#setSoTimeout(Timeout)}, + * supply the resulting {@link SocketConfig} to + * {@link org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder#setDefaultSocketConfig(SocketConfig)}, + * use the resulting connection manager for + * {@link org.apache.hc.client5.http.impl.classic.HttpClientBuilder#setConnectionManager(HttpClientConnectionManager)}, + * and supply the built {@link HttpClient} to {@link #HttpComponentsClientHttpRequestFactory(HttpClient)}. + * @deprecated as of 6.0, in favor of {@link SocketConfig.Builder#setSoTimeout(Timeout)}, see above. */ + @Deprecated(since = "6.0", forRemoval = true) public void setReadTimeout(int timeout) { - Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value"); - this.requestConfig = requestConfigBuilder().setSocketTimeout(timeout).build(); + logger.warn("HttpComponentsClientHttpRequestFactory.setReadTimeout has no effect"); } /** @@ -179,7 +191,7 @@ public void setHttpContextFactory(BiFunction httpC public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException { HttpClient client = getHttpClient(); - HttpUriRequest httpRequest = createHttpUriRequest(httpMethod, uri); + ClassicHttpRequest httpRequest = createHttpUriRequest(httpMethod, uri); postProcessHttpRequest(httpRequest); HttpContext context = createHttpContext(httpMethod, uri); if (context == null) { @@ -210,14 +222,6 @@ public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IO } - /** - * Return a builder for modifying the factory-level {@link RequestConfig}. - * @since 4.2 - */ - private RequestConfig.Builder requestConfigBuilder() { - return (this.requestConfig != null ? RequestConfig.copy(this.requestConfig) : RequestConfig.custom()); - } - /** * Create a default {@link RequestConfig} to use with the given client. * Can return {@code null} to indicate that no custom request config should @@ -231,37 +235,31 @@ private RequestConfig.Builder requestConfigBuilder() { */ @Nullable protected RequestConfig createRequestConfig(Object client) { - if (client instanceof Configurable) { - RequestConfig clientRequestConfig = ((Configurable) client).getConfig(); + if (client instanceof Configurable configurableClient) { + RequestConfig clientRequestConfig = configurableClient.getConfig(); return mergeRequestConfig(clientRequestConfig); } - return this.requestConfig; + return mergeRequestConfig(RequestConfig.DEFAULT); } /** * Merge the given {@link HttpClient}-level {@link RequestConfig} with - * the factory-level {@link RequestConfig}, if necessary. + * the factory-level configuration, if necessary. * @param clientConfig the config held by the current * @return the merged request config * @since 4.2 */ protected RequestConfig mergeRequestConfig(RequestConfig clientConfig) { - if (this.requestConfig == null) { // nothing to merge + if (this.connectTimeout == -1 && this.connectionRequestTimeout == -1) { // nothing to merge return clientConfig; } RequestConfig.Builder builder = RequestConfig.copy(clientConfig); - int connectTimeout = this.requestConfig.getConnectTimeout(); - if (connectTimeout >= 0) { - builder.setConnectTimeout(connectTimeout); - } - int connectionRequestTimeout = this.requestConfig.getConnectionRequestTimeout(); - if (connectionRequestTimeout >= 0) { - builder.setConnectionRequestTimeout(connectionRequestTimeout); + if (this.connectTimeout >= 0) { + builder.setConnectTimeout(this.connectTimeout, TimeUnit.MILLISECONDS); } - int socketTimeout = this.requestConfig.getSocketTimeout(); - if (socketTimeout >= 0) { - builder.setSocketTimeout(socketTimeout); + if (this.connectionRequestTimeout >= 0) { + builder.setConnectionRequestTimeout(this.connectionRequestTimeout, TimeUnit.MILLISECONDS); } return builder.build(); } @@ -272,7 +270,7 @@ protected RequestConfig mergeRequestConfig(RequestConfig clientConfig) { * @param uri the URI * @return the Commons HttpMethodBase object */ - protected HttpUriRequest createHttpUriRequest(HttpMethod httpMethod, URI uri) { + protected ClassicHttpRequest createHttpUriRequest(HttpMethod httpMethod, URI uri) { if (HttpMethod.GET.equals(httpMethod)) { return new HttpGet(uri); } @@ -301,12 +299,12 @@ else if (HttpMethod.TRACE.equals(httpMethod)) { } /** - * Template method that allows for manipulating the {@link HttpUriRequest} before it is + * Template method that allows for manipulating the {@link ClassicHttpRequest} before it is * returned as part of a {@link HttpComponentsClientHttpRequest}. *

The default implementation is empty. * @param request the request to process */ - protected void postProcessHttpRequest(HttpUriRequest request) { + protected void postProcessHttpRequest(ClassicHttpRequest request) { } /** @@ -324,7 +322,7 @@ protected HttpContext createHttpContext(HttpMethod httpMethod, URI uri) { /** * Shutdown hook that closes the underlying - * {@link org.apache.http.conn.HttpClientConnectionManager ClientConnectionManager}'s + * {@link HttpClientConnectionManager ClientConnectionManager}'s * connection pool, if any. */ @Override @@ -340,25 +338,4 @@ public void close() throws IOException { } } - /** - * An alternative to {@link org.apache.http.client.methods.HttpDelete} that - * extends {@link org.apache.http.client.methods.HttpEntityEnclosingRequestBase} - * rather than {@link org.apache.http.client.methods.HttpRequestBase} and - * hence allows HTTP delete with a request body. For use with the RestTemplate - * exchange methods which allow the combination of HTTP DELETE with an entity. - * @since 4.1.2 - */ - private static class HttpDelete extends HttpEntityEnclosingRequestBase { - - public HttpDelete(URI uri) { - super(); - setURI(uri); - } - - @Override - public String getMethod() { - return "DELETE"; - } - } - } diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpResponse.java index 2d4108fa0975..9e7b59e5bae8 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,14 +16,13 @@ package org.springframework.http.client; -import java.io.Closeable; import java.io.IOException; import java.io.InputStream; -import org.apache.http.Header; -import org.apache.http.HttpEntity; -import org.apache.http.HttpResponse; -import org.apache.http.util.EntityUtils; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.io.entity.EntityUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatusCode; @@ -42,38 +41,38 @@ */ final class HttpComponentsClientHttpResponse implements ClientHttpResponse { - private final HttpResponse httpResponse; + private final ClassicHttpResponse httpResponse; @Nullable private HttpHeaders headers; - HttpComponentsClientHttpResponse(HttpResponse httpResponse) { + HttpComponentsClientHttpResponse(ClassicHttpResponse httpResponse) { this.httpResponse = httpResponse; } @Override - public HttpStatusCode getStatusCode() throws IOException { - return HttpStatusCode.valueOf(this.httpResponse.getStatusLine().getStatusCode()); + public HttpStatusCode getStatusCode() { + return HttpStatusCode.valueOf(this.httpResponse.getCode()); } @Override @Deprecated - public int getRawStatusCode() throws IOException { - return this.httpResponse.getStatusLine().getStatusCode(); + public int getRawStatusCode() { + return this.httpResponse.getCode(); } @Override - public String getStatusText() throws IOException { - return this.httpResponse.getStatusLine().getReasonPhrase(); + public String getStatusText() { + return this.httpResponse.getReasonPhrase(); } @Override public HttpHeaders getHeaders() { if (this.headers == null) { this.headers = new HttpHeaders(); - for (Header header : this.httpResponse.getAllHeaders()) { + for (Header header : this.httpResponse.getHeaders()) { this.headers.add(header.getName(), header.getValue()); } } @@ -95,9 +94,7 @@ public void close() { EntityUtils.consume(this.httpResponse.getEntity()); } finally { - if (this.httpResponse instanceof Closeable) { - ((Closeable) this.httpResponse).close(); - } + this.httpResponse.close(); } } catch (IOException ex) { diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java index 2dc701df6deb..02fb26af4487 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,21 +20,24 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.URI; - -import org.apache.http.Header; -import org.apache.http.HttpEntity; -import org.apache.http.HttpEntityEnclosingRequest; -import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.message.BasicHeader; -import org.apache.http.protocol.HttpContext; +import java.net.URISyntaxException; +import java.util.List; +import java.util.Set; + +import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.core5.function.Supplier; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.protocol.HttpContext; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; -import org.springframework.http.MediaType; import org.springframework.http.StreamingHttpOutputMessage; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * {@link ClientHttpRequest} implementation based on @@ -51,7 +54,7 @@ final class HttpComponentsStreamingClientHttpRequest extends AbstractClientHttpR private final HttpClient httpClient; - private final HttpUriRequest httpRequest; + private final ClassicHttpRequest httpRequest; private final HttpContext httpContext; @@ -59,7 +62,7 @@ final class HttpComponentsStreamingClientHttpRequest extends AbstractClientHttpR private Body body; - HttpComponentsStreamingClientHttpRequest(HttpClient client, HttpUriRequest request, HttpContext context) { + HttpComponentsStreamingClientHttpRequest(HttpClient client, ClassicHttpRequest request, HttpContext context) { this.httpClient = client; this.httpRequest = request; this.httpContext = context; @@ -78,7 +81,12 @@ public String getMethodValue() { @Override public URI getURI() { - return this.httpRequest.getURI(); + try { + return this.httpRequest.getUri(); + } + catch (URISyntaxException ex) { + throw new IllegalStateException(ex.getMessage(), ex); + } } @Override @@ -88,7 +96,7 @@ public void setBody(Body body) { } @Override - protected OutputStream getBodyInternal(HttpHeaders headers) throws IOException { + protected OutputStream getBodyInternal(HttpHeaders headers) { throw new UnsupportedOperationException("getBody not supported"); } @@ -96,13 +104,14 @@ protected OutputStream getBodyInternal(HttpHeaders headers) throws IOException { protected ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException { HttpComponentsClientHttpRequest.addHeaders(this.httpRequest, headers); - if (this.httpRequest instanceof HttpEntityEnclosingRequest entityEnclosingRequest && this.body != null) { + if (this.body != null) { HttpEntity requestEntity = new StreamingHttpEntity(getHeaders(), this.body); - entityEnclosingRequest.setEntity(requestEntity); + this.httpRequest.setEntity(requestEntity); } - HttpResponse httpResponse = this.httpClient.execute(this.httpRequest, this.httpContext); - return new HttpComponentsClientHttpResponse(httpResponse); + Assert.isInstanceOf(ClassicHttpResponse.class, httpResponse, + "HttpResponse not an instance of ClassicHttpResponse"); + return new HttpComponentsClientHttpResponse((ClassicHttpResponse) httpResponse); } @@ -134,17 +143,14 @@ public long getContentLength() { @Override @Nullable - public Header getContentType() { - MediaType contentType = this.headers.getContentType(); - return (contentType != null ? new BasicHeader("Content-Type", contentType.toString()) : null); + public String getContentType() { + return this.headers.getFirst(HttpHeaders.CONTENT_TYPE); } @Override @Nullable - public Header getContentEncoding() { - String contentEncoding = this.headers.getFirst("Content-Encoding"); - return (contentEncoding != null ? new BasicHeader("Content-Encoding", contentEncoding) : null); - + public String getContentEncoding() { + return this.headers.getFirst(HttpHeaders.CONTENT_ENCODING); } @Override @@ -163,9 +169,19 @@ public boolean isStreaming() { } @Override - @Deprecated - public void consumeContent() throws IOException { - throw new UnsupportedOperationException(); + @Nullable + public Supplier> getTrailers() { + return null; + } + + @Override + @Nullable + public Set getTrailerNames() { + return null; + } + + @Override + public void close() throws IOException { } } diff --git a/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java index d05969fab679..4d7b4ebe9efb 100644 --- a/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java @@ -18,18 +18,18 @@ import java.net.URI; -import org.apache.http.HttpEntityEnclosingRequest; -import org.apache.http.client.HttpClient; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.Configurable; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.hc.client5.http.classic.HttpClient; +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.HttpClientBuilder; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.core5.util.Timeout; import org.junit.jupiter.api.Test; import org.springframework.http.HttpMethod; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -59,7 +59,6 @@ public void assertCustomConfig() throws Exception { try (HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient)) { hrf.setConnectTimeout(1234); hrf.setConnectionRequestTimeout(4321); - hrf.setReadTimeout(4567); URI uri = new URI(baseUrl + "/status/ok"); request = (HttpComponentsClientHttpRequest) @@ -69,15 +68,14 @@ public void assertCustomConfig() throws Exception { assertThat(config).as("Request config should be set").isNotNull(); assertThat(config).as("Wrong request config type " + config.getClass().getName()).isInstanceOf(RequestConfig.class); RequestConfig requestConfig = (RequestConfig) config; - assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(1234); - assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(4321); - assertThat(requestConfig.getSocketTimeout()).as("Wrong custom socket timeout").isEqualTo(4567); + assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(Timeout.of(1234, MILLISECONDS)); + assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(Timeout.of(4321, MILLISECONDS)); } } @Test public void defaultSettingsOfHttpClientMergedOnExecutorCustomization() throws Exception { - RequestConfig defaultConfig = RequestConfig.custom().setConnectTimeout(1234).build(); + RequestConfig defaultConfig = RequestConfig.custom().setConnectTimeout(1234, MILLISECONDS).build(); CloseableHttpClient client = mock(CloseableHttpClient.class, withSettings().extraInterfaces(Configurable.class)); Configurable configurable = (Configurable) client; @@ -89,15 +87,17 @@ public void defaultSettingsOfHttpClientMergedOnExecutorCustomization() throws Ex hrf.setConnectionRequestTimeout(4567); RequestConfig requestConfig = retrieveRequestConfig(hrf); assertThat(requestConfig).isNotNull(); - assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(4567); + assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(Timeout.of(4567, MILLISECONDS)); // Default connection timeout merged - assertThat(requestConfig.getConnectTimeout()).isEqualTo(1234); + assertThat(requestConfig.getConnectTimeout()).isEqualTo(Timeout.of(1234, MILLISECONDS)); } @Test public void localSettingsOverrideClientDefaultSettings() throws Exception { RequestConfig defaultConfig = RequestConfig.custom() - .setConnectTimeout(1234).setConnectionRequestTimeout(6789).build(); + .setConnectTimeout(1234, MILLISECONDS) + .setConnectionRequestTimeout(6789, MILLISECONDS) + .build(); CloseableHttpClient client = mock(CloseableHttpClient.class, withSettings().extraInterfaces(Configurable.class)); Configurable configurable = (Configurable) client; @@ -107,15 +107,15 @@ public void localSettingsOverrideClientDefaultSettings() throws Exception { hrf.setConnectTimeout(5000); RequestConfig requestConfig = retrieveRequestConfig(hrf); - assertThat(requestConfig.getConnectTimeout()).isEqualTo(5000); - assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(6789); - assertThat(requestConfig.getSocketTimeout()).isEqualTo(-1); + assertThat(requestConfig.getConnectTimeout()).isEqualTo(Timeout.of(5000, MILLISECONDS)); + assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(Timeout.of(6789, MILLISECONDS)); } @Test public void mergeBasedOnCurrentHttpClient() throws Exception { RequestConfig defaultConfig = RequestConfig.custom() - .setSocketTimeout(1234).build(); + .setConnectionRequestTimeout(1234, MILLISECONDS) + .build(); final CloseableHttpClient client = mock(CloseableHttpClient.class, withSettings().extraInterfaces(Configurable.class)); Configurable configurable = (Configurable) client; @@ -127,22 +127,20 @@ public HttpClient getHttpClient() { return client; } }; - hrf.setReadTimeout(5000); + hrf.setConnectionRequestTimeout(5000); RequestConfig requestConfig = retrieveRequestConfig(hrf); - assertThat(requestConfig.getConnectTimeout()).isEqualTo(-1); - assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(-1); - assertThat(requestConfig.getSocketTimeout()).isEqualTo(5000); + assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(Timeout.of(5000, MILLISECONDS)); + assertThat(requestConfig.getConnectTimeout()).isEqualTo(RequestConfig.DEFAULT.getConnectTimeout()); // Update the Http client so that it returns an updated config RequestConfig updatedDefaultConfig = RequestConfig.custom() - .setConnectTimeout(1234).build(); + .setConnectTimeout(1234, MILLISECONDS).build(); given(configurable.getConfig()).willReturn(updatedDefaultConfig); - hrf.setReadTimeout(7000); + hrf.setConnectionRequestTimeout(7000); RequestConfig requestConfig2 = retrieveRequestConfig(hrf); - assertThat(requestConfig2.getConnectTimeout()).isEqualTo(1234); - assertThat(requestConfig2.getConnectionRequestTimeout()).isEqualTo(-1); - assertThat(requestConfig2.getSocketTimeout()).isEqualTo(7000); + assertThat(requestConfig2.getConnectTimeout()).isEqualTo(Timeout.of(1234, MILLISECONDS)); + assertThat(requestConfig2.getConnectionRequestTimeout()).isEqualTo(Timeout.of(7000, MILLISECONDS)); } private RequestConfig retrieveRequestConfig(HttpComponentsClientHttpRequestFactory factory) throws Exception { @@ -152,24 +150,4 @@ private RequestConfig retrieveRequestConfig(HttpComponentsClientHttpRequestFacto return (RequestConfig) request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG); } - @Test - public void createHttpUriRequest() throws Exception { - URI uri = new URI("https://example.com"); - testRequestBodyAllowed(uri, HttpMethod.GET, false); - testRequestBodyAllowed(uri, HttpMethod.HEAD, false); - testRequestBodyAllowed(uri, HttpMethod.OPTIONS, false); - testRequestBodyAllowed(uri, HttpMethod.TRACE, false); - testRequestBodyAllowed(uri, HttpMethod.PUT, true); - testRequestBodyAllowed(uri, HttpMethod.POST, true); - testRequestBodyAllowed(uri, HttpMethod.PATCH, true); - testRequestBodyAllowed(uri, HttpMethod.DELETE, true); - - } - - private void testRequestBodyAllowed(URI uri, HttpMethod method, boolean allowed) { - HttpUriRequest request = ((HttpComponentsClientHttpRequestFactory) this.factory).createHttpUriRequest(method, uri); - Object actual = request instanceof HttpEntityEnclosingRequest; - assertThat(actual).isEqualTo(allowed); - } - } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpsRequestIntegrationTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpsRequestIntegrationTests.java index b8deed7afa4c..3dba514e8533 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpsRequestIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpsRequestIntegrationTests.java @@ -18,12 +18,14 @@ import java.net.URI; -import org.apache.http.conn.ssl.NoopHostnameVerifier; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.conn.ssl.TrustSelfSignedStrategy; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.ssl.SSLContextBuilder; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; +import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.TrustSelfSignedStrategy; +import org.apache.hc.core5.ssl.SSLContextBuilder; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -67,8 +69,11 @@ void startServer() throws Exception { builder.loadTrustMaterial(new TrustSelfSignedStrategy()); SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory( builder.build(), NoopHostnameVerifier.INSTANCE); - CloseableHttpClient httpclient = HttpClients.custom().setSSLSocketFactory( - socketFactory).build(); + PoolingHttpClientConnectionManager connectionManager = PoolingHttpClientConnectionManagerBuilder.create() + .setSSLSocketFactory(socketFactory) + .build(); + CloseableHttpClient httpclient = HttpClients.custom(). + setConnectionManager(connectionManager).build(); HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory(httpclient); this.restTemplate = new RestTemplate(requestFactory); diff --git a/spring-webflux/spring-webflux.gradle b/spring-webflux/spring-webflux.gradle index f7df70a0820e..afd978fede04 100644 --- a/spring-webflux/spring-webflux.gradle +++ b/spring-webflux/spring-webflux.gradle @@ -26,9 +26,6 @@ dependencies { } optional("org.eclipse.jetty.websocket:websocket-jetty-client") optional("io.undertow:undertow-websockets-jsr-jakarta") - optional("org.apache.httpcomponents:httpclient") { - exclude group: "commons-logging", module: "commons-logging" - } optional("org.jetbrains.kotlin:kotlin-reflect") optional("org.jetbrains.kotlin:kotlin-stdlib") optional("com.google.protobuf:protobuf-java-util") diff --git a/spring-webmvc/spring-webmvc.gradle b/spring-webmvc/spring-webmvc.gradle index 585eba2915ac..39f712ab2a52 100644 --- a/spring-webmvc/spring-webmvc.gradle +++ b/spring-webmvc/spring-webmvc.gradle @@ -44,9 +44,6 @@ dependencies { testImplementation("org.eclipse.jetty:jetty-server") { exclude group: "jakarta.servlet", module: "jakarta.servlet-api" } - testImplementation("org.apache.httpcomponents:httpclient") { - exclude group: "commons-logging", module: "commons-logging" - } testImplementation("commons-io:commons-io") testImplementation("org.mozilla:rhino") testImplementation("org.dom4j:dom4j") { @@ -64,6 +61,7 @@ dependencies { testImplementation("io.projectreactor:reactor-core") testImplementation("io.reactivex.rxjava3:rxjava") testImplementation("org.jetbrains.kotlin:kotlin-script-runtime") + testRuntimeOnly("org.apache.httpcomponents.client5:httpclient5") testRuntimeOnly("org.jetbrains.kotlin:kotlin-scripting-jsr223") testRuntimeOnly("org.jruby:jruby") testRuntimeOnly("org.python:jython-standalone")