Migrate CLI to Apache HttpClient 5

Closes gh-33534

spring-boot-project/spring-boot-tools/spring-boot-cli/build.gradle

@@ -25,9 +25,7 @@ dependencies {
 	implementation("com.vaadin.external.google:android-json")
 	implementation("jline:jline")
 	implementation("net.sf.jopt-simple:jopt-simple")
-	implementation("org.apache.httpcomponents:httpclient") {
-		exclude group: "commons-logging", module: "commons-logging"
-	}
+	implementation("org.apache.httpcomponents.client5:httpclient5")
 	implementation("org.slf4j:slf4j-simple")
 	implementation("org.springframework:spring-core")
 	implementation("org.springframework.security:spring-security-crypto")

spring-boot-project/spring-boot-tools/spring-boot-cli/src/main/java/org/springframework/boot/cli/command/init/InitializrService.java

@@ -21,16 +21,18 @@ import java.net.URI;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 
-import org.apache.http.Header;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpHeaders;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.entity.ContentType;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.message.BasicHeader;
+import org.apache.hc.client5.http.classic.HttpClient;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
+import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.hc.core5.http.message.StatusLine;
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -62,16 +64,16 @@ class InitializrService {
 	/**
 	 * Late binding HTTP client.
 	 */
-	private CloseableHttpClient http;
+	private HttpClient http;
 
 	InitializrService() {
 	}
 
-	InitializrService(CloseableHttpClient http) {
+	InitializrService(HttpClient http) {
 		this.http = http;
 	}
 
-	protected CloseableHttpClient getHttp() {
+	protected HttpClient getHttp() {
 		if (this.http == null) {
 			this.http = HttpClientBuilder.create().useSystemProperties().build();
 		}
@@ -88,7 +90,7 @@ class InitializrService {
 		Log.info("Using service at " + request.getServiceUrl());
 		InitializrServiceMetadata metadata = loadMetadata(request.getServiceUrl());
 		URI url = request.generateUrl(metadata);
-		CloseableHttpResponse httpResponse = executeProjectGenerationRequest(url);
+		ClassicHttpResponse httpResponse = executeProjectGenerationRequest(url);
 		HttpEntity httpEntity = httpResponse.getEntity();
 		validateResponse(httpResponse, request.getServiceUrl());
 		return createResponse(httpResponse, httpEntity);
@@ -101,7 +103,7 @@ class InitializrService {
 	 * @throws IOException if the service's metadata cannot be loaded
 	 */
 	InitializrServiceMetadata loadMetadata(String serviceUrl) throws IOException {
-		CloseableHttpResponse httpResponse = executeInitializrMetadataRetrieval(serviceUrl);
+		ClassicHttpResponse httpResponse = executeInitializrMetadataRetrieval(serviceUrl);
 		validateResponse(httpResponse, serviceUrl);
 		return parseJsonMetadata(httpResponse.getEntity());
 	}
@@ -118,10 +120,10 @@ class InitializrService {
 	Object loadServiceCapabilities(String serviceUrl) throws IOException {
 		HttpGet request = new HttpGet(serviceUrl);
 		request.setHeader(new BasicHeader(HttpHeaders.ACCEPT, ACCEPT_SERVICE_CAPABILITIES));
-		CloseableHttpResponse httpResponse = execute(request, serviceUrl, "retrieve help");
+		ClassicHttpResponse httpResponse = execute(request, URI.create(serviceUrl), "retrieve help");
 		validateResponse(httpResponse, serviceUrl);
 		HttpEntity httpEntity = httpResponse.getEntity();
-		ContentType contentType = ContentType.getOrDefault(httpEntity);
+		ContentType contentType = ContentType.create(httpEntity.getContentType());
 		if (contentType.getMimeType().equals("text/plain")) {
 			return getContent(httpEntity);
 		}
@@ -137,18 +139,19 @@ class InitializrService {
 		}
 	}
 
-	private void validateResponse(CloseableHttpResponse httpResponse, String serviceUrl) {
+	private void validateResponse(ClassicHttpResponse httpResponse, String serviceUrl) {
 		if (httpResponse.getEntity() == null) {
 			throw new ReportableException("No content received from server '" + serviceUrl + "'");
 		}
-		if (httpResponse.getStatusLine().getStatusCode() != 200) {
+		if (httpResponse.getCode() != 200) {
 			throw createException(serviceUrl, httpResponse);
 		}
 	}
 
-	private ProjectGenerationResponse createResponse(CloseableHttpResponse httpResponse, HttpEntity httpEntity)
+	private ProjectGenerationResponse createResponse(ClassicHttpResponse httpResponse, HttpEntity httpEntity)
 			throws IOException {
-		ProjectGenerationResponse response = new ProjectGenerationResponse(ContentType.getOrDefault(httpEntity));
+		ProjectGenerationResponse response = new ProjectGenerationResponse(
+				ContentType.create(httpEntity.getContentType()));
 		response.setContent(FileCopyUtils.copyToByteArray(httpEntity.getContent()));
 		String fileName = extractFileName(httpResponse.getFirstHeader("Content-Disposition"));
 		if (fileName != null) {
@@ -162,7 +165,7 @@ class InitializrService {
 	 * @param url the URL
 	 * @return the response
 	 */
-	private CloseableHttpResponse executeProjectGenerationRequest(URI url) {
+	private ClassicHttpResponse executeProjectGenerationRequest(URI url) {
 		return execute(new HttpGet(url), url, "generate project");
 	}
 
@@ -171,16 +174,17 @@ class InitializrService {
 	 * @param url the URL
 	 * @return the response
 	 */
-	private CloseableHttpResponse executeInitializrMetadataRetrieval(String url) {
+	private ClassicHttpResponse executeInitializrMetadataRetrieval(String url) {
 		HttpGet request = new HttpGet(url);
 		request.setHeader(new BasicHeader(HttpHeaders.ACCEPT, ACCEPT_META_DATA));
-		return execute(request, url, "retrieve metadata");
+		return execute(request, URI.create(url), "retrieve metadata");
 	}
 
-	private CloseableHttpResponse execute(HttpUriRequest request, Object url, String description) {
+	private ClassicHttpResponse execute(HttpUriRequest request, URI url, String description) {
 		try {
+			HttpHost host = HttpHost.create(url);
 			request.addHeader("User-Agent", "SpringBootCli/" + getClass().getPackage().getImplementationVersion());
-			return getHttp().execute(request);
+			return getHttp().execute(host, request);
 		}
 		catch (IOException ex) {
 			throw new ReportableException(
@@ -188,15 +192,16 @@ class InitializrService {
 		}
 	}
 
-	private ReportableException createException(String url, CloseableHttpResponse httpResponse) {
+	private ReportableException createException(String url, ClassicHttpResponse httpResponse) {
+		StatusLine statusLine = new StatusLine(httpResponse);
 		String message = "Initializr service call failed using '" + url + "' - service returned "
-				+ httpResponse.getStatusLine().getReasonPhrase();
+				+ statusLine.getReasonPhrase();
 		String error = extractMessage(httpResponse.getEntity());
 		if (StringUtils.hasText(error)) {
 			message += ": '" + error + "'";
 		}
 		else {
-			int statusCode = httpResponse.getStatusLine().getStatusCode();
+			int statusCode = statusLine.getStatusCode();
 			message += " (unexpected " + statusCode + " error)";
 		}
 		throw new ReportableException(message);
@@ -222,7 +227,7 @@ class InitializrService {
 	}
 
 	private String getContent(HttpEntity entity) throws IOException {
-		ContentType contentType = ContentType.getOrDefault(entity);
+		ContentType contentType = ContentType.create(entity.getContentType());
 		Charset charset = contentType.getCharset();
 		charset = (charset != null) ? charset : StandardCharsets.UTF_8;
 		byte[] content = FileCopyUtils.copyToByteArray(entity.getContent());

spring-boot-project/spring-boot-tools/spring-boot-cli/src/main/java/org/springframework/boot/cli/command/init/ProjectGenerationRequest.java

@@ -23,7 +23,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.apache.http.client.utils.URIBuilder;
+import org.apache.hc.core5.net.URIBuilder;
 
 import org.springframework.util.StringUtils;
 

spring-boot-project/spring-boot-tools/spring-boot-cli/src/main/java/org/springframework/boot/cli/command/init/ProjectGenerationResponse.java

@@ -16,7 +16,7 @@
 
 package org.springframework.boot.cli.command.init;
 
-import org.apache.http.entity.ContentType;
+import org.apache.hc.core5.http.ContentType;
 
 /**
  * Represent the response of a {@link ProjectGenerationRequest}.

spring-boot-project/spring-boot-tools/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/init/AbstractHttpClientMockTests.java

@@ -19,14 +19,14 @@ package org.springframework.boot.cli.command.init;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 
-import org.apache.http.Header;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpHeaders;
-import org.apache.http.StatusLine;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.message.BasicHeader;
+import org.apache.hc.client5.http.classic.HttpClient;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+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.HttpHeaders;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.message.BasicHeader;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mockito.ArgumentMatcher;
@@ -35,19 +35,20 @@ import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.Resource;
 import org.springframework.util.StreamUtils;
 
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
 /**
- * Abstract base class for tests that use a mock {@link CloseableHttpClient}.
+ * Abstract base class for tests that use a mock {@link HttpClient}.
  *
  * @author Stephane Nicoll
  */
 public abstract class AbstractHttpClientMockTests {
 
-	protected final CloseableHttpClient http = mock(CloseableHttpClient.class);
+	protected final HttpClient http = mock(HttpClient.class);
 
 	protected void mockSuccessfulMetadataTextGet() throws IOException {
 		mockSuccessfulMetadataGet("metadata/service-metadata-2.1.0.txt", "text/plain", true);
@@ -65,11 +66,12 @@ public abstract class AbstractHttpClientMockTests {
 
 	protected void mockSuccessfulMetadataGet(String contentPath, String contentType, boolean serviceCapabilities)
 			throws IOException {
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		byte[] content = readClasspathResource(contentPath);
 		mockHttpEntity(response, content, contentType);
 		mockStatus(response, 200);
-		given(this.http.execute(argThat(getForMetadata(serviceCapabilities)))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), argThat(getForMetadata(serviceCapabilities))))
+				.willReturn(response);
 	}
 
 	protected byte[] readClasspathResource(String contentPath) throws IOException {
@@ -80,36 +82,37 @@ public abstract class AbstractHttpClientMockTests {
 	protected void mockSuccessfulProjectGeneration(MockHttpProjectGenerationRequest request) throws IOException {
 		// Required for project generation as the metadata is read first
 		mockSuccessfulMetadataGet(false);
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		mockHttpEntity(response, request.content, request.contentType);
 		mockStatus(response, 200);
 		String header = (request.fileName != null) ? contentDispositionValue(request.fileName) : null;
 		mockHttpHeader(response, "Content-Disposition", header);
-		given(this.http.execute(argThat(getForNonMetadata()))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), argThat(getForNonMetadata()))).willReturn(response);
 	}
 
 	protected void mockProjectGenerationError(int status, String message) throws IOException, JSONException {
 		// Required for project generation as the metadata is read first
 		mockSuccessfulMetadataGet(false);
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		mockHttpEntity(response, createJsonError(status, message).getBytes(), "application/json");
 		mockStatus(response, status);
-		given(this.http.execute(isA(HttpGet.class))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), isA(HttpGet.class))).willReturn(response);
 	}
 
 	protected void mockMetadataGetError(int status, String message) throws IOException, JSONException {
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		mockHttpEntity(response, createJsonError(status, message).getBytes(), "application/json");
 		mockStatus(response, status);
-		given(this.http.execute(isA(HttpGet.class))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), isA(HttpGet.class))).willReturn(response);
 	}
 
-	protected HttpEntity mockHttpEntity(CloseableHttpResponse response, byte[] content, String contentType) {
+	protected HttpEntity mockHttpEntity(ClassicHttpResponse response, byte[] content, String contentType) {
 		try {
 			HttpEntity entity = mock(HttpEntity.class);
 			given(entity.getContent()).willReturn(new ByteArrayInputStream(content));
 			Header contentTypeHeader = (contentType != null) ? new BasicHeader("Content-Type", contentType) : null;
-			given(entity.getContentType()).willReturn(contentTypeHeader);
+			given(entity.getContentType())
+					.willReturn((contentTypeHeader != null) ? contentTypeHeader.getValue() : null);
 			given(response.getEntity()).willReturn(entity);
 			return entity;
 		}
@@ -118,13 +121,11 @@ public abstract class AbstractHttpClientMockTests {
 		}
 	}
 
-	protected void mockStatus(CloseableHttpResponse response, int status) {
-		StatusLine statusLine = mock(StatusLine.class);
-		given(statusLine.getStatusCode()).willReturn(status);
-		given(response.getStatusLine()).willReturn(statusLine);
+	protected void mockStatus(ClassicHttpResponse response, int status) {
+		given(response.getCode()).willReturn(status);
 	}
 
-	protected void mockHttpHeader(CloseableHttpResponse response, String headerName, String value) {
+	protected void mockHttpHeader(ClassicHttpResponse response, String headerName, String value) {
 		Header header = (value != null) ? new BasicHeader(headerName, value) : null;
 		given(response.getFirstHeader(headerName)).willReturn(header);
 	}
@@ -166,7 +167,7 @@ public abstract class AbstractHttpClientMockTests {
 		}
 
 		MockHttpProjectGenerationRequest(String contentType, String fileName, byte[] content) {
-			this.contentType = contentType;
+			this.contentType = (contentType != null) ? contentType : "application/text";
 			this.fileName = fileName;
 			this.content = content;
 		}

spring-boot-project/spring-boot-tools/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/init/InitCommandTests.java

@@ -25,8 +25,9 @@ import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
 import joptsimple.OptionSet;
-import org.apache.http.Header;
-import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpHost;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.io.TempDir;
@@ -37,6 +38,8 @@ import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.boot.cli.command.status.ExitStatus;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.then;
 
 /**
@@ -205,6 +208,9 @@ class InitCommandTests extends AbstractHttpClientMockTests {
 			assertThat(this.command.run("--extract", tempDir.getAbsolutePath())).isEqualTo(ExitStatus.OK);
 			assertThat(file).as("file should have been saved instead").exists();
 		}
+		catch (Exception ex) {
+			fail(ex);
+		}
 		finally {
 			assertThat(file.delete()).as("failed to delete test file").isTrue();
 		}
@@ -393,7 +399,7 @@ class InitCommandTests extends AbstractHttpClientMockTests {
 	@Test
 	void userAgent() throws Exception {
 		this.command.run("--list", "--target=https://fake-service");
-		then(this.http).should().execute(this.requestCaptor.capture());
+		then(this.http).should().execute(any(HttpHost.class), this.requestCaptor.capture());
 		Header agent = this.requestCaptor.getValue().getHeaders("User-Agent")[0];
 		assertThat(agent.getValue()).startsWith("SpringBootCli/");
 	}

spring-boot-project/spring-boot-tools/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/init/InitializrServiceTests.java

@@ -16,12 +16,14 @@
 
 package org.springframework.boot.cli.command.init;
 
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpHost;
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
@@ -91,9 +93,9 @@ class InitializrServiceTests extends AbstractHttpClientMockTests {
 	@Test
 	void generateProjectNoContent() throws Exception {
 		mockSuccessfulMetadataGet(false);
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		mockStatus(response, 500);
-		given(this.http.execute(isA(HttpGet.class))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), isA(HttpGet.class))).willReturn(response);
 		ProjectGenerationRequest request = new ProjectGenerationRequest();
 		assertThatExceptionOfType(ReportableException.class).isThrownBy(() -> this.invoker.generate(request))
 				.withMessageContaining("No content received from server");
@@ -110,10 +112,10 @@ class InitializrServiceTests extends AbstractHttpClientMockTests {
 
 	@Test
 	void loadMetadataInvalidJson() throws Exception {
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		mockHttpEntity(response, "Foo-Bar-Not-JSON".getBytes(), "application/json");
 		mockStatus(response, 200);
-		given(this.http.execute(isA(HttpGet.class))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), isA(HttpGet.class))).willReturn(response);
 		ProjectGenerationRequest request = new ProjectGenerationRequest();
 		assertThatExceptionOfType(ReportableException.class).isThrownBy(() -> this.invoker.generate(request))
 				.withMessageContaining("Invalid content received from server");
@@ -121,9 +123,9 @@ class InitializrServiceTests extends AbstractHttpClientMockTests {
 
 	@Test
 	void loadMetadataNoContent() throws Exception {
-		CloseableHttpResponse response = mock(CloseableHttpResponse.class);
+		ClassicHttpResponse response = mock(ClassicHttpResponse.class);
 		mockStatus(response, 500);
-		given(this.http.execute(isA(HttpGet.class))).willReturn(response);
+		given(this.http.execute(any(HttpHost.class), isA(HttpGet.class))).willReturn(response);
 		ProjectGenerationRequest request = new ProjectGenerationRequest();
 		assertThatExceptionOfType(ReportableException.class).isThrownBy(() -> this.invoker.generate(request))
 				.withMessageContaining("No content received from server");

spring-boot-project/spring-boot-tools/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/init/ProjectGenerationRequestTests.java

@@ -128,7 +128,7 @@ class ProjectGenerationRequestTests {
 		this.request.setDescription("Spring Boot Test");
 		assertThat(this.request.generateUrl(createDefaultMetadata()))
 				.isEqualTo(createDefaultUrl("?groupId=org.acme&artifactId=sample&version=1.0.1-SNAPSHOT"
-						+ "&description=Spring+Boot+Test&type=test-type"));
+						+ "&description=Spring%20Boot%20Test&type=test-type"));
 	}
 
 	@Test