Skip to content
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.

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

2296 upgrade to apache httpclient 5 #2471

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
18 changes: 3 additions & 15 deletions karate-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,10 @@
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<version>4.5.14</version>
<exclusions>
<exclusion>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
</exclusion>
</exclusions>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
<version>5.3</version>
</dependency>
<!-- can be removed once httpclient is upgraded -->
<dependency>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
<version>1.16.0</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
Expand Down
326 changes: 206 additions & 120 deletions karate-core/src/main/java/com/intuit/karate/http/ApacheHttpClient.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

import java.io.IOException;

import org.apache.http.NoHttpResponseException;
import org.apache.http.client.HttpRequestRetryHandler;
import org.apache.http.protocol.HttpContext;
import org.apache.hc.client5.http.HttpRequestRetryStrategy;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.NoHttpResponseException;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.util.TimeValue;

import com.intuit.karate.Logger;

Expand All @@ -13,7 +16,7 @@
* This is usually the case when there is steal connection. The retry cause that
* the connection is renewed and the second call will succeed.
*/
public class CustomHttpRequestRetryHandler implements HttpRequestRetryHandler
public class CustomHttpRequestRetryHandler implements HttpRequestRetryStrategy
{
private final Logger logger;

Expand All @@ -22,9 +25,7 @@ public CustomHttpRequestRetryHandler(Logger logger)
this.logger = logger;
}

@Override
public boolean retryRequest(IOException exception, int executionCount, HttpContext context)
{
private boolean shouldRetry(IOException exception, int executionCount) {
if (exception instanceof NoHttpResponseException && executionCount < 1)
{
logger.error("Thrown an NoHttpResponseException retry...");
Expand All @@ -36,4 +37,19 @@ public boolean retryRequest(IOException exception, int executionCount, HttpConte
return false;
}
}

@Override
public boolean retryRequest(HttpRequest request, IOException exception, int executionCount, HttpContext context) {
return shouldRetry(exception, executionCount);
}

@Override
public boolean retryRequest(HttpResponse response, int execCount, HttpContext context) {
return false;
}

@Override
public TimeValue getRetryInterval(HttpResponse response, int execCount, HttpContext context) {
return TimeValue.ofSeconds(1); // NOt sure what the interval was in httpclient4 ... Sticking with the default value of the default http5 implementation.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.apache.http.client.utils.URIBuilder;
import org.apache.hc.core5.net.URIBuilder;
import org.graalvm.polyglot.Value;
import org.graalvm.polyglot.proxy.ProxyObject;
import org.slf4j.Logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.apache.http.conn.ssl;
package org.apache.hc.client5.http.ssl;

import java.io.IOException;
import java.net.Socket;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import org.apache.http.protocol.HttpContext;

import org.apache.hc.core5.http.protocol.HttpContext;

/**
* in a separate package just for log level config consistency
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,10 @@ void testTypeConv() {
run("type-conv.feature");
}

@Test
void testConfigureNtlmAuthentication() {
run("ntlm-authentication.feature");
}
// NTLM not supported in apache client 5.3
// @Test
// void testConfigureNtlmAuthentication() {
// run("ntlm-authentication.feature");
// }

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,20 @@
import java.nio.charset.StandardCharsets;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;

import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
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.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactoryBuilder;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -81,15 +84,18 @@ int http(HttpUriRequest request) throws Exception {
SSLContext sc = SSLContext.getInstance("SSL");
sc.init(null, new TrustManager[]{LenientTrustManager.INSTANCE}, null);
CloseableHttpClient client = HttpClients.custom()
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.setSSLContext(sc)
.setConnectionManager(PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(SSLConnectionSocketFactoryBuilder.create()
.setSslContext(sc)
.setHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.build())
.build())
.setProxy(new HttpHost("localhost", proxy.getPort()))
.build();
HttpResponse response = client.execute(request);
InputStream is = response.getEntity().getContent();
String responseString = FileUtils.toString(is);
String responseString = response.getReasonPhrase();
logger.debug("response: {}", responseString);
return response.getStatusLine().getStatusCode();
return response.getCode();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@
import com.intuit.karate.core.MockServer;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;

import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -81,10 +82,9 @@ static int http(HttpUriRequest request) throws Exception {
.setProxy(new HttpHost("localhost", proxy.getPort()))
.build();
HttpResponse response = client.execute(request);
InputStream is = response.getEntity().getContent();
String responseString = FileUtils.toString(is);
String responseString = response.getReasonPhrase();
logger.debug("response: {}", responseString);
return response.getStatusLine().getStatusCode();
return response.getCode();
}

}
6 changes: 6 additions & 0 deletions karate-demo/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
<!-- for a skeleton project, use the archetype: https://github.com/karatelabs/karate#quickstart !-->
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
<!-- Consistent with version from karate-core -->
<version>5.3</version>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@f-delahaye just curious, why was this needed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spring-boot comes with httpclient5 5.1.3 and overrides the version defined in karate-core (5.3)

$ mvn help:effective-pom -Dverbose | grep -A2 -B2 \<artifactId\>httpclient5\</artifactId\>
      <dependency>
        <groupId>org.apache.httpcomponents.client5</groupId>  <!-- org.springframework.boot:spring-boot-dependencies:2.6.6, line 953 -->
        <artifactId>httpclient5</artifactId>  <!-- org.springframework.boot:spring-boot-dependencies:2.6.6, line 954 -->
        <version>5.1.3</version>  <!-- org.springframework.boot:spring-boot-dependencies:2.6.6, line 955 -->
      </dependency>

Unfortunately, it looks like karate-demo does not start with my changes against httpclient 5.1.3 - it hangs.
Not sure why, I didn't investigate further -

I could not get it to work with the latest spring-boot either. But forcing to version 5.3 in karate-demo solves the issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok thanks

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
Expand Down