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 <[email protected]> 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); } }
