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.

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

Jump to bottom

2296 upgrade to apache httpclient 5 #2471

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

2296 upgrade to apache httpclient 5 #2471

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a0b30bf
#2296
f-delahaye Nov 5, 2023
b143b25
#2296 - disable keepalive strategy to fix SocketException
f-delahaye Dec 3, 2023
b470148
#2296 - code clean up
f-delahaye Dec 3, 2023
5f33d92
Merge branch 'develop' into 2296-upgrade-to-apache-httpclient-5
f-delahaye Dec 17, 2023
4544a15
Merge branch 'develop' into 2296-upgrade-to-apache-httpclient-5
f-delahaye Dec 17, 2023
abc38b4
remove references to http client 4
f-delahaye Dec 17, 2023
683ce85
#2296 Upgraded to latest apache httpclient (5.3) and dropped support …
f-delahaye Dec 19, 2023
d9d58e9
2296 Cosmetic change
f-delahaye Dec 23, 2023
715ed4a
Upgrade karate-demo to same version of httpclient5 as karate-core
f-delahaye Dec 23, 2023
7246fa5
Introduced close() method in HttpClient and use it to close the conne…
f-delahaye Jan 10, 2024
f9a530b
Redesigned RoutePlanner - Separate ProxySelector use cases from proxy…
f-delahaye Jan 23, 2024
b9507de
cosmetic changes
f-delahaye Jan 24, 2024
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
18 changes: 3 additions & 15 deletions karate-core/pom.xml
View file
Open in desktop
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
View file
Open in desktop

Large diffs are not rendered by default.

30 changes: 23 additions & 7 deletions karate-core/src/main/java/com/intuit/karate/http/CustomHttpRequestRetryHandler.java
View file
Open in desktop
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.
}
}
2 changes: 1 addition & 1 deletion karate-core/src/main/java/com/intuit/karate/http/HttpRequestBuilder.java
View file
Open in desktop
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
5 changes: 3 additions & 2 deletions ...sl/LenientSslConnectionSocketFactory.java → ...sl/LenientSslConnectionSocketFactory.java
View file
Open in desktop
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
9 changes: 5 additions & 4 deletions karate-core/src/test/java/com/intuit/karate/core/FeatureRuntimeTest.java
View file
Open in desktop
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");
// }

}
10 changes: 0 additions & 10 deletions karate-core/src/test/java/com/intuit/karate/core/ntlm-authentication.feature
View file
Open in desktop

This file was deleted.

38 changes: 22 additions & 16 deletions karate-core/src/test/java/com/intuit/karate/fatjar/ProxyServerSslTest.java
View file
Open in desktop
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();
}

}
26 changes: 13 additions & 13 deletions karate-core/src/test/java/com/intuit/karate/fatjar/ProxyServerTest.java
View file
Open in desktop
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
View file
Open in desktop
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
Contributor 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
Loading