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/07 09:34:14 UTC

[sling-whiteboard] branch feature/url-connection-agent-hc4 updated (e6d8e3b -> ed87808)

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a change to branch feature/url-connection-agent-hc4
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git.


 discard e6d8e3b  Implement default timeouts for httpclient 4.x
    omit 00a9ca8  Updated README to reflect latest state.
    omit f83667d  Implement default timeouts for httpclient 3.x
    omit 739415d  Prepare tests for running against multiple client types
    omit 2739f23  Prepared testing infrastructure for multiple client implementations
    omit fb1f825  Move launcher out of the jar
    omit 751cf60  Converted IntegrationTest to a proper IT
    omit a1c070e  Use Maven central for connect timeout tests. Accessing sling.apache.org failed with a 'connection refused' error on Jenkins, presumably since builds.apache.org has direct access to other apache.org servers.
    omit a4fa68b  Add missing license header
    omit 976cd1f  Stubbed out a wait to launch external processes
    omit 5589434  Removed unused dependencies
    omit bb37fd8  Improved test support classes naming and documentation
    omit cc4b11d  Allow accessing server as an instance field and stop hardcoding the local port.
    omit 6c53142  Cleanups in tests
    omit c5079c9  Provisional integration tests, currently setting the timeouts manually
    omit 45660b5  Migrate to Junit 5 for more fine-grained timeouts and exception checks.
    omit a1b1871  Some work on integration tests
     add b0fd4f8  Some work on integration tests
     add 4b2d40b  Migrate to Junit 5 for more fine-grained timeouts and exception checks.
     add f79ae3c  Provisional integration tests, currently setting the timeouts manually
     add 69bb6e4  Cleanups in tests
     add d4d8491  Allow accessing server as an instance field and stop hardcoding the local port.
     add 59c680c  Improved test support classes naming and documentation
     add 5360a60  Removed unused dependencies
     add 827efd0  Stubbed out a wait to launch external processes
     add 87cf8d7  Add missing license header
     add 89d2e52  Use Maven central for connect timeout tests. Accessing sling.apache.org failed with a 'connection refused' error on Jenkins, presumably since builds.apache.org has direct access to other apache.org servers.
     add 9a46cac  Converted IntegrationTest to a proper IT
     add e974da3  Move launcher out of the jar
     add f7693f1  Prepared testing infrastructure for multiple client implementations
     add 697c1b9  Prepare tests for running against multiple client types
     add cffc06e  Implement default timeouts for httpclient 3.x
     add 2dbd960  Updated README to reflect latest state.
     new ed87808  Implement default timeouts for httpclient 4.x

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (e6d8e3b)
            \
             N -- N -- N   refs/heads/feature/url-connection-agent-hc4 (ed87808)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:


[sling-whiteboard] 01/01: Implement default timeouts for httpclient 4.x

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch feature/url-connection-agent-hc4
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit ed87808b18571a7765d65efa0f6deb05ee9cac6b
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);
+            }
+            
+        }
+    }
+
 }