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/06 15:10:13 UTC
[sling-whiteboard] 15/16: Implement default timeouts for httpclient
3.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 cffc06e75c769113cd44dd7d823b12a02b1f2577
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Thu Jun 6 16:51:45 2019 +0200
Implement default timeouts for httpclient 3.x
---
url-connection-agent/pom.xml | 6 ++
.../main/java/org/apache/sling/uca/impl/Agent.java | 12 +++-
.../uca/impl/HttpClient3TimeoutTransformer.java | 67 ++++++++++++++++++++++
...sformer.java => JavaNetTimeoutTransformer.java} | 4 +-
.../java/org/apache/sling/uca/impl/AgentIT.java | 17 ++++--
...est.java => JaveNetTimeoutTransformerTest.java} | 6 +-
.../test/java/org/apache/sling/uca/impl/Main.java | 22 +++++--
7 files changed, 117 insertions(+), 17 deletions(-)
diff --git a/url-connection-agent/pom.xml b/url-connection-agent/pom.xml
index f6b36e8..b020df3 100644
--- a/url-connection-agent/pom.xml
+++ b/url-connection-agent/pom.xml
@@ -121,5 +121,11 @@
<version>3.1</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>jcl-over-slf4j</artifactId>
+ <version>1.7.25</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 2b27373..4ed0067 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
@@ -16,13 +16,14 @@
*/
package org.apache.sling.uca.impl;
+import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.Instrumentation;
import java.util.concurrent.TimeUnit;
public class Agent {
public static void premain(String args, Instrumentation inst) {
-
+
System.out.println("[AGENT] Loading agent...");
String[] parsedArgs = args.split(",");
long connectTimeout = TimeUnit.MINUTES.toMillis(1);
@@ -34,9 +35,14 @@ public class Agent {
System.out.format("[AGENT] Set connectTimeout : %d, readTimeout: %d%n", connectTimeout, readTimeout);
- URLTimeoutTransformer transformer = new URLTimeoutTransformer(connectTimeout, readTimeout);
+ ClassFileTransformer[] transformers = new ClassFileTransformer[] {
+ new JavaNetTimeoutTransformer(connectTimeout, readTimeout),
+ new HttpClient3TimeoutTransformer(connectTimeout, readTimeout)
+ };
- inst.addTransformer(transformer, true);
+ for ( ClassFileTransformer transformer : transformers )
+ inst.addTransformer(transformer, true);
+
System.out.println("[AGENT] Loaded agent!");
}
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
new file mode 100644
index 0000000..b287447
--- /dev/null
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
@@ -0,0 +1,67 @@
+/*
+ * 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.CtMethod;
+import javassist.bytecode.Descriptor;
+
+public class HttpClient3TimeoutTransformer implements ClassFileTransformer {
+
+ private static final String DEFAULT_HTTP_PARAMS_FACTORY_CLASS_NAME = Descriptor.toJvmName("org.apache.commons.httpclient.params.DefaultHttpParamsFactory");
+
+ private final long connectTimeoutMillis;
+ private final long readTimeoutMillis;
+
+ public HttpClient3TimeoutTransformer(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 ( DEFAULT_HTTP_PARAMS_FACTORY_CLASS_NAME.equals(className) ) {
+ System.out.println("[AGENT] Asked to transform " + className);
+
+ ClassPool defaultPool = ClassPool.getDefault();
+ CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
+
+ CtMethod getSoTimeout = cc.getDeclaredMethod("createParams");
+ // javassist seems unable to resolve the constant values, so just inline them
+ // also, unable to resolve calls to setParameter with int values (no boxing?)
+ // HttpConnectionParams.CONNECTION_TIMEOUT
+ getSoTimeout.insertAfter("$_.setParameter(\"http.connection.timeout\", Integer.valueOf(" + connectTimeoutMillis + "));");
+ // HttpMethodParams.SO_TIMEOUT
+ getSoTimeout.insertAfter("$_.setParameter(\"http.socket.timeout\", Integer.valueOf(" + 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/main/java/org/apache/sling/uca/impl/URLTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
similarity index 95%
rename from url-connection-agent/src/main/java/org/apache/sling/uca/impl/URLTimeoutTransformer.java
rename to url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
index 1143004..d320875 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/URLTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
@@ -38,7 +38,7 @@ import javassist.bytecode.Descriptor;
* @see URLConnection#getReadTimeout()
*
*/
-class URLTimeoutTransformer implements ClassFileTransformer {
+class JavaNetTimeoutTransformer implements ClassFileTransformer {
private static final Set<String> CLASSES_TO_TRANSFORM = new HashSet<>();
@@ -50,7 +50,7 @@ class URLTimeoutTransformer implements ClassFileTransformer {
private final long readTimeoutMillis;
private final long connectTimeoutMillis;
- public URLTimeoutTransformer(long connectTimeout, long readTimeout) {
+ public JavaNetTimeoutTransformer(long connectTimeout, long readTimeout) {
this.connectTimeoutMillis = connectTimeout;
this.readTimeoutMillis = readTimeout;
}
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 3c2b9e7..65bd7af 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
@@ -32,6 +32,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
+import org.apache.commons.httpclient.ConnectTimeoutException;
import org.apache.sling.uca.impl.Main.ClientType;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
@@ -73,8 +74,12 @@ public class AgentIT {
public void connectTimeout(ClientType clientType) throws IOException {
RecordedThrowable error = assertTimeout(ofSeconds(5), () -> runTest("http://repo1.maven.org:81", clientType));
- assertEquals(SocketTimeoutException.class.getName(), error.className);
- assertEquals("connect timed out", error.message);
+
+ 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";
+
+ assertEquals(expectedClass.getName(), error.className);
+ assertEquals(expectedMessage, error.message);
}
/**
@@ -95,7 +100,7 @@ public class AgentIT {
private RecordedThrowable runTest(String urlSpec, ClientType clientType) throws IOException, InterruptedException {
Process process = runForkedCommandWithAgent(new URL(urlSpec), 3, 3, clientType);
- boolean done = process.waitFor(10, TimeUnit.SECONDS);
+ boolean done = process.waitFor(30, TimeUnit.SECONDS);
LOG.info("Dump of stdout: ");
Files
@@ -160,7 +165,11 @@ public class AgentIT {
elements.add(Paths.get("target", "test-classes").toString());
Files.list(Paths.get("target", "it-dependencies"))
- .filter( p -> p.getFileName().toString().startsWith("commons-"))
+ .filter( p -> p.getFileName().toString().equals("commons-httpclient.jar")
+ || 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"))
.forEach( p -> elements.add(p.toString()));
return String.join(File.pathSeparator, elements);
diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/UrlTimeoutTransformerTest.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/JaveNetTimeoutTransformerTest.java
similarity index 91%
rename from url-connection-agent/src/test/java/org/apache/sling/uca/impl/UrlTimeoutTransformerTest.java
rename to url-connection-agent/src/test/java/org/apache/sling/uca/impl/JaveNetTimeoutTransformerTest.java
index 3847ef2..c0abffc 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/UrlTimeoutTransformerTest.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/JaveNetTimeoutTransformerTest.java
@@ -25,13 +25,13 @@ import org.junit.jupiter.api.Test;
import javassist.NotFoundException;
-public class UrlTimeoutTransformerTest {
+public class JaveNetTimeoutTransformerTest {
- private URLTimeoutTransformer transformer;
+ private JavaNetTimeoutTransformer transformer;
@BeforeEach
public void initFields() {
- transformer = new URLTimeoutTransformer(1, 1);
+ transformer = new JavaNetTimeoutTransformer(1, 1);
}
@Test
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 2b19902..de07a23 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
@@ -23,24 +23,30 @@ import java.io.InputStreamReader;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;
+import java.util.Date;
+import org.apache.commons.httpclient.DefaultHttpMethodRetryHandler;
import org.apache.commons.httpclient.Header;
import org.apache.commons.httpclient.HttpClient;
import org.apache.commons.httpclient.HttpException;
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;
public class Main {
// TODO - write help messages with the values from this enum
public enum ClientType {
- JavaNet /* , HC3 */
+ JavaNet, HC3
}
public static void main(String[] args) throws MalformedURLException, IOException {
if ( args.length != 2 )
throw new IllegalArgumentException("Usage: java -cp ... " + Main.class.getName() + " <URL> JavaNet|HC3|HC4");
+
+ System.out.println(new Date() + " [WEB] Executing request via " + args[1]);
switch ( args[1] ) {
case "JavaNet":
@@ -70,14 +76,20 @@ public class Main {
private static void runUsingHttpClient3(String targetUrl) throws HttpException, IOException {
HttpClient client = new HttpClient();
+ // disable retries, to make sure that we get equivalent behaviour with other implementations
+ client.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, new DefaultHttpMethodRetryHandler(0, false));
HttpMethod get = new GetMethod(targetUrl);
-
+ System.out.format("Connection timeouts: connect: %d, so: %s%n",
+ client.getHttpConnectionManager().getParams().getConnectionTimeout(),
+ client.getHttpConnectionManager().getParams().getSoTimeout());
+ System.out.format("Client so timeout: %d (raw: %s) %n", client.getParams().getSoTimeout(),
+ client.getParams().getParameter(HttpClientParams.SO_TIMEOUT));
client.executeMethod(get);
- System.out.println("[WEB] " + get.getStatusLine());
+ System.out.println(new Date() + " [WEB] " + get.getStatusLine());
for ( Header header : get.getResponseHeaders() )
- System.out.print("[WEB] " + header.toExternalForm());
+ System.out.print(new Date() + " [WEB] " + header.toExternalForm());
try (InputStream in = get.getResponseBodyAsStream()) {
@@ -86,7 +98,7 @@ public class Main {
BufferedReader br = new BufferedReader(isr)) {
String line;
while ((line = br.readLine()) != null)
- System.out.println("[WEB] " + line);
+ System.out.println(new Date() + " [WEB] " + line);
}
}