You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2019/06/12 14:27:24 UTC
[sling-whiteboard] 01/11: Implement default timeouts for httpclient
4.x
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit 69e0fe76b73e5b1be772221fb1ff530bc326351d
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 11:33:04 2019 +0200
Implement default timeouts for httpclient 4.x
---
url-connection-agent/README.md | 5 +-
url-connection-agent/pom.xml | 6 ++
.../main/java/org/apache/sling/uca/impl/Agent.java | 3 +-
.../uca/impl/HttpClient4TimeoutTransformer.java | 70 ++++++++++++++++++++++
.../java/org/apache/sling/uca/impl/AgentIT.java | 37 +++++++++---
.../test/java/org/apache/sling/uca/impl/Main.java | 29 ++++++++-
6 files changed, 139 insertions(+), 11 deletions(-)
diff --git a/url-connection-agent/README.md b/url-connection-agent/README.md
index d39b235..fcc5d06 100644
--- a/url-connection-agent/README.md
+++ b/url-connection-agent/README.md
@@ -15,7 +15,7 @@ Build the project with `mvn clean package` and then run a simple connection test
- `<connect-timeout>` - connection timeout in milliseconds
- `<read-timeout>`- read timeout in milliseconds
- `<url>` - the URL to access
- - `<client-type>` - the client type, either `JavaNet` for java.net.URL-based connections or `HC3` for commons-httpclient 3.x
+ - `<client-type>` - the client type, either `JavaNet` for java.net.URL-based connections ,`HC3` for Apache Commons HttpClient 3.x or `HC4` for Apache Commons HttpClient 4.x
For a test that always fails, set one of the timeouts to 1. Both executions listed below will typically fail:
@@ -35,4 +35,5 @@ java -javaagent:target/url-connection-agent-0.0.1-SNAPSHOT-jar-with-dependencies
* openjdk version "1.8.0_212"
* openjdk version "11.0.2" 2019-01-15
-* commons-httpclient 3.1
\ No newline at end of file
+* commons-httpclient 3.1
+* httpclient 4.5.4
\ No newline at end of file
diff --git a/url-connection-agent/pom.xml b/url-connection-agent/pom.xml
index b020df3..c148a64 100644
--- a/url-connection-agent/pom.xml
+++ b/url-connection-agent/pom.xml
@@ -127,5 +127,11 @@
<version>1.7.25</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ <version>4.5.4</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
\ No newline at end of file
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java
index 4ed0067..c35565f 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java
@@ -37,7 +37,8 @@ public class Agent {
ClassFileTransformer[] transformers = new ClassFileTransformer[] {
new JavaNetTimeoutTransformer(connectTimeout, readTimeout),
- new HttpClient3TimeoutTransformer(connectTimeout, readTimeout)
+ new HttpClient3TimeoutTransformer(connectTimeout, readTimeout),
+ new HttpClient4TimeoutTransformer(connectTimeout, readTimeout)
};
for ( ClassFileTransformer transformer : transformers )
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
new file mode 100644
index 0000000..ee62458
--- /dev/null
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.uca.impl;
+
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.IllegalClassFormatException;
+import java.security.ProtectionDomain;
+
+import javassist.ClassPool;
+import javassist.CtClass;
+import javassist.CtConstructor;
+import javassist.CtField;
+import javassist.bytecode.Descriptor;
+
+public class HttpClient4TimeoutTransformer implements ClassFileTransformer {
+
+ // org.apache.http.client.config.RequestConfig.Builder
+
+ private static final String REQUEST_CONFIG_BUILDER_CLASS_NAME = Descriptor.toJvmName("org.apache.http.client.config.RequestConfig$Builder");
+
+ private final long connectTimeoutMillis;
+ private final long readTimeoutMillis;
+
+ public HttpClient4TimeoutTransformer(long connectTimeoutMillis, long readTimeoutMillis) {
+ this.connectTimeoutMillis = connectTimeoutMillis;
+ this.readTimeoutMillis = readTimeoutMillis;
+ }
+
+ @Override
+ public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined,
+ ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
+ try {
+ if ( REQUEST_CONFIG_BUILDER_CLASS_NAME.equals(className) ) {
+ System.out.println("[AGENT] Asked to transform " + className);
+
+ ClassPool defaultPool = ClassPool.getDefault();
+ CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
+
+ // TODO - access the default constructor explicitly in case it changes
+ CtConstructor noArgCtor = cc.getConstructors()[0];
+ CtField connectTimeout = cc.getDeclaredField("connectTimeout");
+ CtField socketTimeout = cc.getDeclaredField("socketTimeout");
+ noArgCtor.insertAfter("this." + connectTimeout.getName() + " = " + connectTimeoutMillis + ";");
+ noArgCtor.insertAfter("this." + socketTimeout.getName() + " = " + readTimeoutMillis + ";");
+
+ classfileBuffer = cc.toBytecode();
+ cc.detach();
+ }
+ return classfileBuffer;
+ } catch (Exception e) {
+ e.printStackTrace(); // ensure _something_ is printed
+ throw new RuntimeException("[AGENT] Transformation failed", e);
+ }
+ }
+
+}
diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
index 65bd7af..97f0c4c 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
@@ -19,6 +19,7 @@ package org.apache.sling.uca.impl;
import static java.time.Duration.ofSeconds;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTimeout;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.File;
import java.io.IOException;
@@ -75,11 +76,29 @@ public class AgentIT {
RecordedThrowable error = assertTimeout(ofSeconds(5), () -> runTest("http://repo1.maven.org:81", clientType));
- Class<?> expectedClass = clientType == ClientType.HC3 ? ConnectTimeoutException.class : SocketTimeoutException.class;
- String expectedMessage = clientType == ClientType.HC3 ? "The host did not accept the connection within timeout of 3000 ms" : "connect timed out";
+ Class<?> expectedClass;
+ String expectedMessageRegex;
+
+ switch ( clientType ) {
+ case JavaNet:
+ expectedClass= SocketTimeoutException.class;
+ expectedMessageRegex = "connect timed out";
+ break;
+ case HC3:
+ expectedClass = ConnectTimeoutException.class;
+ expectedMessageRegex = "The host did not accept the connection within timeout of 3000 ms";
+ break;
+ case HC4:
+ expectedClass = org.apache.http.conn.ConnectTimeoutException.class;
+ expectedMessageRegex = "Connect to repo1.maven.org:81 \\[.*\\] failed: connect timed out";
+ break;
+ default:
+ throw new AssertionError("Unhandled clientType " + clientType);
+ }
assertEquals(expectedClass.getName(), error.className);
- assertEquals(expectedMessage, error.message);
+ assertTrue(error.message.matches(expectedMessageRegex),
+ "Actual message " + error.message + " did not match regex " + expectedMessageRegex);
}
/**
@@ -169,7 +188,9 @@ public class AgentIT {
|| p.getFileName().toString().equals("commons-codec.jar")
|| p.getFileName().toString().equals("slf4j-simple.jar")
|| p.getFileName().toString().equals("slf4j-api.jar")
- || p.getFileName().toString().equals("jcl-over-slf4j.jar"))
+ || p.getFileName().toString().equals("jcl-over-slf4j.jar")
+ || p.getFileName().toString().contentEquals("httpclient.jar")
+ || p.getFileName().toString().contentEquals("httpcore.jar") )
.forEach( p -> elements.add(p.toString()));
return String.join(File.pathSeparator, elements);
@@ -177,10 +198,12 @@ public class AgentIT {
private RecordedThrowable newRecordedThrowable(String string) {
- string = string.replace("Exception in thread \"main\"", "");
- String[] parts = string.split(":");
+ string = string.replace("Exception in thread \"main\" ", "");
+
+ String className = string.substring(0, string.indexOf(':'));
+ String message = string.substring(string.indexOf(':') + 2); // ignore ':' and leading ' '
- return new RecordedThrowable(parts[0].trim(), parts[1].trim());
+ return new RecordedThrowable(className, message);
}
/**
diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/Main.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/Main.java
index de07a23..aee0725 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/Main.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/Main.java
@@ -33,12 +33,18 @@ import org.apache.commons.httpclient.HttpMethod;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.params.HttpClientParams;
import org.apache.commons.httpclient.params.HttpMethodParams;
+import org.apache.http.HttpEntity;
+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.impl.client.HttpClients;
+import org.apache.http.util.EntityUtils;
public class Main {
// TODO - write help messages with the values from this enum
public enum ClientType {
- JavaNet, HC3
+ JavaNet, HC3, HC4
}
public static void main(String[] args) throws MalformedURLException, IOException {
@@ -55,6 +61,9 @@ public class Main {
case "HC3":
runUsingHttpClient3(args[0]);
break;
+ case "HC4":
+ runUsingHttpClient4(args[0]);
+ break;
default:
throw new IllegalArgumentException("Usage: java -cp ... " + Main.class.getName() + " <URL> JavaNet|HC3|HC4");
}
@@ -104,4 +113,22 @@ public class Main {
}
}
}
+
+ private static void runUsingHttpClient4(String targetUrl) throws IOException {
+ // disable retries, to make sure that we get equivalent behaviour with other implementations
+ try ( CloseableHttpClient client = HttpClients.custom().disableAutomaticRetries().build() ) {
+ HttpGet get = new HttpGet(targetUrl);
+ try ( CloseableHttpResponse response = client.execute(get)) {
+ System.out.println("[WEB] " + response.getStatusLine());
+ for ( org.apache.http.Header header : response.getAllHeaders() )
+ System.out.println("[WEB] " + header);
+
+ HttpEntity entity = response.getEntity();
+ // TODO - print response body
+ EntityUtils.consume(entity);
+ }
+
+ }
+ }
+
}