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 15:19:21 UTC

[sling-whiteboard] branch url-connection-agent-cleanups created (now e368877)

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

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


      at e368877  Launcher cleanup

This branch includes the following new commits:

     new 7bde27c  Prevent NPE if agent is launched with no arguments
     new b5294ee  Send back user-agent to help identify which requests come through
     new ea60dfc  Whitespace fix
     new f459aad  Documentation updates
     new 06278ce  Create a simple log abstraction
     new bb21828  Rename 'Main' to 'HttpClientLauncher'
     new e368877  Launcher cleanup

The 7 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.



[sling-whiteboard] 05/07: Create a simple log abstraction

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

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

commit 06278cef3c7ada8e8a04647f1485d25108a96dcd
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 16:39:13 2019 +0200

    Create a simple log abstraction
---
 .../main/java/org/apache/sling/uca/impl/Agent.java |  17 ++--
 .../uca/impl/HttpClient3TimeoutTransformer.java    |   7 +-
 .../uca/impl/HttpClient4TimeoutTransformer.java    |   7 +-
 .../sling/uca/impl/JavaNetTimeoutTransformer.java  |  10 +-
 .../main/java/org/apache/sling/uca/impl/Log.java   | 113 +++++++++++++++++++++
 .../java/org/apache/sling/uca/impl/AgentIT.java    |   2 +-
 6 files changed, 135 insertions(+), 21 deletions(-)

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 a9320d6..e523155 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
@@ -24,16 +24,20 @@ public class Agent {
 
     public static void premain(String args, Instrumentation inst) {
         
-        System.out.println("[AGENT] Loading agent...");
         String[] parsedArgs = args != null ? args.split(",") : new String[0];
         long connectTimeout =  TimeUnit.MINUTES.toMillis(1);
         long readTimeout = TimeUnit.MINUTES.toMillis(1);
+        String logSpec = "";
         if ( parsedArgs.length > 0 )
             connectTimeout = Long.parseLong(parsedArgs[0]);
         if ( parsedArgs.length > 1 )
             readTimeout = Long.parseLong(parsedArgs[1]);
+        if ( parsedArgs.length > 2)
+            logSpec = parsedArgs[2];
         
-        System.out.format("[AGENT] Set connectTimeout : %d, readTimeout: %d%n", connectTimeout, readTimeout);
+        Log.configure(logSpec);
+        
+        Log.get().log("Preparing to install URL transformers. Configured timeouts - connectTimeout : %d, readTimeout: %d", connectTimeout, readTimeout);
 
         ClassFileTransformer[] transformers = new ClassFileTransformer[] {
             new JavaNetTimeoutTransformer(connectTimeout, readTimeout),
@@ -44,11 +48,6 @@ public class Agent {
         for ( ClassFileTransformer transformer : transformers )
             inst.addTransformer(transformer, true);
 
-        System.out.println("[AGENT] Loaded agent!");
-    }
-
-    public static void agentmain(String args, Instrumentation inst) {
-        premain(args, inst);
-    }
-    
+        Log.get().log("All transformers installed");
+    }    
 }
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
index 6e57245..9bf5d92 100644
--- 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
@@ -48,7 +48,7 @@ public class HttpClient3TimeoutTransformer implements ClassFileTransformer {
             ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
         try {
             if ( DEFAULT_HTTP_PARAMS_FACTORY_CLASS_NAME.equals(className) ) {
-                System.out.println("[AGENT] Asked to transform " + className);
+                Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
                 
                 ClassPool defaultPool = ClassPool.getDefault();
                 CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
@@ -63,11 +63,12 @@ public class HttpClient3TimeoutTransformer implements ClassFileTransformer {
                 
                 classfileBuffer = cc.toBytecode();
                 cc.detach();
+                Log.get().log("Transformation complete.");
             }
             return classfileBuffer;
         } catch (Exception e) {
-            e.printStackTrace(); // ensure _something_ is printed
-            throw new RuntimeException("[AGENT] Transformation failed", e);
+            Log.get().fatal("Transformation failed", e);
+            return null;
         }
     }
 }
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
index e74dabf..230c1db 100644
--- 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
@@ -51,7 +51,7 @@ public class HttpClient4TimeoutTransformer implements ClassFileTransformer {
             ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
         try {
             if ( REQUEST_CONFIG_BUILDER_CLASS_NAME.equals(className) ) {
-                System.out.println("[AGENT] Asked to transform " + className);
+                Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
                 
                 ClassPool defaultPool = ClassPool.getDefault();
                 CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
@@ -65,11 +65,12 @@ public class HttpClient4TimeoutTransformer implements ClassFileTransformer {
                 
                 classfileBuffer = cc.toBytecode();
                 cc.detach();
+                Log.get().log("Transformation complete.");
             }
             return classfileBuffer;
         } catch (Exception e) {
-            e.printStackTrace(); // ensure _something_ is printed
-            throw new RuntimeException("[AGENT] Transformation failed", e);
+            Log.get().fatal("Transformation failed", e);
+            return null;
         }
     }
 
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
index 9978c33..121417c 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
@@ -60,18 +60,18 @@ class JavaNetTimeoutTransformer implements ClassFileTransformer {
             ProtectionDomain protectionDomain, byte[] classfileBuffer) {
         try {
             if (CLASSES_TO_TRANSFORM.contains(className)) {
-                System.out.println("[AGENT] Asked to transform " + className);
+                Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
                 CtMethod connectMethod = findConnectMethod(className);
                 connectMethod.insertBefore("if ( getConnectTimeout() == 0 ) { setConnectTimeout(" + connectTimeoutMillis + "); }");
                 connectMethod.insertBefore("if ( getReadTimeout() == 0 ) { setReadTimeout(" + readTimeoutMillis + "); }");
                 classfileBuffer = connectMethod.getDeclaringClass().toBytecode();
                 connectMethod.getDeclaringClass().detach();
-                System.out.println("[AGENT] Transformation complete!");
+                Log.get().log("Transformation complete.");
             }
             return classfileBuffer;
         } catch (Exception e) {
-            e.printStackTrace(); // ensure _something_ is printed
-            throw new RuntimeException("[AGENT] Transformation failed", e);
+            Log.get().fatal("Transformation failed", e);
+            return null;
         }
     }
     
@@ -80,7 +80,7 @@ class JavaNetTimeoutTransformer implements ClassFileTransformer {
         ClassPool defaultPool = ClassPool.getDefault();
         CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
         if (cc == null) {
-            System.out.println("[AGENT] no class found with name " + className);
+            Log.get().log("No class found with name %s", className);
             return null;
         }
         return cc.getDeclaredMethod("connect");
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Log.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Log.java
new file mode 100644
index 0000000..900f426
--- /dev/null
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Log.java
@@ -0,0 +1,113 @@
+/*
+ * 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 static java.util.Objects.requireNonNull;
+
+import java.util.Formatter;
+
+/**
+ * Simple logger abstraction
+ * 
+ * <p>This is an intentionally simple and simplistic logger for agent-internal usage. Since the agent environment
+ * is limited and can make no assumption about how it will be used, it only uses the console and no
+ * external libraries.</p>
+ * 
+ * <p>It is required to call {@link #configure(String)} before accessing the log instance using {@link #get()}.</p>
+ *
+ */
+abstract class Log {
+    
+    private static Log INSTANCE;
+
+    /**
+     * Configures the global logger instance
+     * 
+     * @param spec the logger spec, <tt>v</tt> for a console log, anything else for a no-op log
+     */
+    public static void configure(String spec) {
+        INSTANCE = "v".equals(spec) ? new ConsoleLog() : new NoopLog();
+    }
+    
+    /**
+     * Gets the global logger instance, configured with {@link #configure(String)}
+     * 
+     * @return the global logger instance
+     * @throws NullPointerException in case the logger is not configured
+     */
+    public static Log get() {
+        return requireNonNull(INSTANCE, "Log is null, did you foget to call Log.configure() ?");
+    }
+
+    private Log() {
+
+    }
+
+    /**
+     * Logs a message
+     * 
+     * <p>The message and the arguments are interpolated using a {@link Formatter}, e.g.
+     * 
+     * <pre>Logger.get().log("Transforming %s", klazz.getName());</pre>
+     * 
+     *  </p>
+     *  
+     *  <p>The line separator <tt>%n</tt> is automatically appended to the message.</p>
+     * 
+     * @param msg the message
+     * @param args the arguments
+     */
+    public abstract void log(String msg, Object... args);
+    
+    /**
+     * Prints the throwable stack trace and throws a <tt>RuntimeException</tt>
+     * 
+     * @param message the message to include in the <tt>RuntimeException</tt>
+     * @param t the throwable to print the stack trace for 
+     */
+    public abstract void fatal(String message, Throwable t);
+
+    static class ConsoleLog extends Log {
+
+        private static final String LOG_ENTRY_PREFIX = "[AGENT] ";
+
+        @Override
+        public void log(String msg, Object... args) {
+            System.out.format(LOG_ENTRY_PREFIX + msg + " %n", args);
+        }
+        
+        @Override
+        public void fatal(String msg, Throwable t) {
+            t.printStackTrace(); // ensure _something_ is printed
+            throw new RuntimeException(LOG_ENTRY_PREFIX + msg, t);
+            
+        }
+    }
+    
+    static class NoopLog extends Log {
+
+        @Override
+        public void log(String msg, Object... args) {
+            // empty by design
+        }
+        
+        @Override
+        public void fatal(String message, Throwable t) {
+            // empty by design
+        }
+    }
+}
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 a14c700..3a343ba 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
@@ -162,7 +162,7 @@ public class AgentIT {
         ProcessBuilder pb = new ProcessBuilder(
             javaExe.toString(),
             "-showversion",
-            "-javaagent:" + jar +"=" + TimeUnit.SECONDS.toMillis(connectTimeoutSeconds) +"," + TimeUnit.SECONDS.toMillis(readTimeoutSeconds),
+            "-javaagent:" + jar +"=" + TimeUnit.SECONDS.toMillis(connectTimeoutSeconds) +"," + TimeUnit.SECONDS.toMillis(readTimeoutSeconds)+",v",
             "-cp",
             classPath,
             "org.apache.sling.uca.impl.Main",


[sling-whiteboard] 07/07: Launcher cleanup

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

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

commit e3688776c1f55b825d2a700adb752cc41a8aa574
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 17:18:55 2019 +0200

    Launcher cleanup
---
 .../apache/sling/uca/impl/HttpClientLauncher.java  | 164 +++++++++++++--------
 1 file changed, 99 insertions(+), 65 deletions(-)

diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java
index 9d8ca73..bce7dda 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java
@@ -24,11 +24,13 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.util.Date;
+import java.util.EnumSet;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
 
 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;
@@ -45,92 +47,124 @@ import org.apache.http.util.EntityUtils;
  */
 public class HttpClientLauncher {
     
-    // TODO - write help messages with the values from this enum
     public enum ClientType {
-        JavaNet, HC3, HC4
+        JavaNet(HttpClientLauncher::runUsingJavaNet), 
+        HC3(HttpClientLauncher::runUsingHttpClient3),
+        HC4(HttpClientLauncher::runUsingHttpClient4);
+        
+        private final Consumer<String> consumer;
+
+        ClientType(Consumer<String> consumer) {
+            this.consumer = consumer;
+        }
+        
+        public Consumer<String> getConsumer() {
+            return consumer;
+        }
+        
+        static String pipeSeparatedString() {
+            return EnumSet.allOf(ClientType.class).stream()
+                .map(ClientType::toString)
+                .collect(Collectors.joining("|"));
+        }
+        
+        static ClientType fromString(String value) {
+            return EnumSet.allOf(ClientType.class).stream()
+                .filter( e -> e.toString().equals(value) )
+                .findFirst()
+                .orElse(null);
+        }
     }
 
     public static void main(String[] args) throws MalformedURLException, IOException {
         
         if ( args.length != 2 )
-            throw new IllegalArgumentException("Usage: java -cp ... " + HttpClientLauncher.class.getName() + " <URL> JavaNet|HC3|HC4");
+            throw new IllegalArgumentException(usage());
+        
+        ClientType type = ClientType.fromString(args[1]);
+        if ( type == null )
+            throw new IllegalArgumentException(usage());
         
-        System.out.println(new Date() + " [WEB] Executing request via " + args[1]);
+        System.out.println("[WEB] Executing request via " + type);
+        
+        type.consumer.accept(args[0]);
+    }
 
-        switch ( args[1] ) {
-            case "JavaNet":
-                runUsingJavaNet(args[0]);
-                break;
-            case "HC3":
-                runUsingHttpClient3(args[0]);
-                break;
-            case "HC4":
-                runUsingHttpClient4(args[0]);
-                break;
-            default:
-                throw new IllegalArgumentException("Usage: java -cp ... " + HttpClientLauncher.class.getName() + " <URL> JavaNet|HC3|HC4");
-        }
+    private static String usage() {
+        return "Usage: java -cp ... " + HttpClientLauncher.class.getName() + " <URL> " + ClientType.pipeSeparatedString();
     }
 
-    private static void runUsingJavaNet(String targetUrl) throws MalformedURLException, IOException {
-        URLConnection con = new URL(targetUrl).openConnection();
-        System.out.println("Connection type is " + con);
-        
-        try (InputStream in = con.getInputStream();
-                InputStreamReader isr = new InputStreamReader(in);
-                BufferedReader br = new BufferedReader(isr)) {
-            String line;
-            while ( (line = br.readLine()) != null )
-                System.out.println("[WEB] " + line);
+    private static void runUsingJavaNet(String targetUrl)  {
+        try {
+            URLConnection con = new URL(targetUrl).openConnection();
+            System.out.println("Connection type is " + con);
+            
+            try (InputStream in = con.getInputStream();
+                    InputStreamReader isr = new InputStreamReader(in);
+                    BufferedReader br = new BufferedReader(isr)) {
+                String line;
+                while ( (line = br.readLine()) != null )
+                    System.out.println("[WEB] " + line);
+            }
+        } catch (IOException e) {
+            throw new RuntimeException(e);
         }
     }
 
 
-    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(new Date() + " [WEB] " + get.getStatusLine());
-        
-        for ( Header header : get.getResponseHeaders() )
-            System.out.print(new Date() + " [WEB] " + header.toExternalForm());
-        
-        
-        try (InputStream in = get.getResponseBodyAsStream()) {
-            if (in != null) {
-                try (InputStreamReader isr = new InputStreamReader(in); 
-                        BufferedReader br = new BufferedReader(isr)) {
-                    String line;
-                    while ((line = br.readLine()) != null)
-                        System.out.println(new Date() + " [WEB] " + line);
+    private static void runUsingHttpClient3(String targetUrl) {
+        try {
+            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(new Date() + " [WEB] " + get.getStatusLine());
+            
+            for ( Header header : get.getResponseHeaders() )
+                System.out.print(new Date() + " [WEB] " + header.toExternalForm());
+            
+            
+            try (InputStream in = get.getResponseBodyAsStream()) {
+                if (in != null) {
+                    try (InputStreamReader isr = new InputStreamReader(in); 
+                            BufferedReader br = new BufferedReader(isr)) {
+                        String line;
+                        while ((line = br.readLine()) != null)
+                            System.out.println(new Date() + " [WEB] " + line);
 
+                    }
                 }
             }
+        } catch (IOException e) {
+            throw new RuntimeException(e);
         }
     }
     
-    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);
+    private static void runUsingHttpClient4(String targetUrl) {
+        try {
+            // 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);
+                }
                 
-                HttpEntity entity = response.getEntity();
-                // TODO - print response body
-                EntityUtils.consume(entity);
             }
-            
+        } catch (IOException e) {
+            throw new RuntimeException(e);
         }
     }
 


[sling-whiteboard] 04/07: Documentation updates

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

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

commit f459aad3070bab6a9ab3b799136a819f9c314fb1
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 16:07:44 2019 +0200

    Documentation updates
---
 url-connection-agent/README.md                           | 16 +++++++++++++---
 .../sling/uca/impl/HttpClient3TimeoutTransformer.java    |  6 ++++++
 .../sling/uca/impl/HttpClient4TimeoutTransformer.java    |  6 ++++++
 .../apache/sling/uca/impl/JavaNetTimeoutTransformer.java |  6 +++---
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/url-connection-agent/README.md b/url-connection-agent/README.md
index fcc5d06..916c0ca 100644
--- a/url-connection-agent/README.md
+++ b/url-connection-agent/README.md
@@ -1,10 +1,18 @@
-# Apache Sling URL connection agent
+# Apache Sling HTTP timeout enforcer
 
 This module is part of the [Apache Sling](https://sling.apache.org) project.
 
-This module provides a java agent that uses the instrumentation API to add timeouts to `connect` calls made via HTTP or HTTPs without setting read and connect timeouts. It is intended as an additional layer of control to use when running unstrusted client code that may make calls without explicitly setting timeouts.
+This module provides a java agent that uses the [instrumentation API](https://docs.oracle.com/javase/7/docs/api/java/lang/instrument/package-summary.html) to add connect and read timeouts to `connect` made via HTTP or HTTPs. It only applies these timeouts if none were set explicitly.
 
-## Launching
+The agent is intended as an additional layer of control to use when running untrusted client code that may make calls without explicitly setting timeouts. It is always recommended to set timeouts in client code, rather than relying on this agent.
+
+It currently supports setting timeouts for HTTP connections done using:
+
+* [java.net.URL](https://docs.oracle.com/javase/7/docs/api/java/net/URL.html) and/or [java.net.URLConnection](https://docs.oracle.com/javase/7/docs/api/java/net/URLConnection.html)
+* [Apache Commons HttpClient 3.x](https://hc.apache.org/httpclient-3.x/)
+* [Apache HttpComponents Client 4.x](https://hc.apache.org/httpcomponents-client-ga/)
+
+## Validation
 
 Build the project with `mvn clean package` and then run a simple connection test with 
 
@@ -31,6 +39,8 @@ In contrast, the execution below should succeed:
 java -javaagent:target/url-connection-agent-0.0.1-SNAPSHOT-jar-with-dependencies.jar=1000,1000 -cp target/test-classes:target/it-dependencies/* org.apache.sling.uca.impl.Main https://sling.apache.org JavaNet
 ```
 
+To use this in your own project you should 
+
 ## Tested platforms
 
 * openjdk version "1.8.0_212"
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
index b287447..6e57245 100644
--- 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
@@ -25,6 +25,12 @@ import javassist.CtClass;
 import javassist.CtMethod;
 import javassist.bytecode.Descriptor;
 
+/**
+ * Sets timeouts for HTTP calls done using <em>Apache Commons HttpClient 3.x</em>
+ * 
+ * <p>It inserts two calls in <tt>org.apache.commons.httpclient.params.DefaultHttpParamsFactory.createParams</tt> that set
+ * default values for <tt>http.connection.timeout</tt> and <tt>http.socket.timeout</tt>.</p>
+ */
 public class HttpClient3TimeoutTransformer implements ClassFileTransformer {
     
     private static final String DEFAULT_HTTP_PARAMS_FACTORY_CLASS_NAME = Descriptor.toJvmName("org.apache.commons.httpclient.params.DefaultHttpParamsFactory");
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
index ee62458..e74dabf 100644
--- 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
@@ -26,6 +26,12 @@ import javassist.CtConstructor;
 import javassist.CtField;
 import javassist.bytecode.Descriptor;
 
+/**
+ * Sets timeouts for HTTP calls done using <em>Apache HttpComponents Client 4.x</em>
+ * 
+ * <p>It inserts two calls to <tt>org.apache.http.client.config.RequestConfig$Builder</tt> that set default
+ * values for <tt>connectTimeout</tt> and <tt>socketTimeout</tt>.</p>
+ */
 public class HttpClient4TimeoutTransformer implements ClassFileTransformer {
 
     // org.apache.http.client.config.RequestConfig.Builder
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
index d320875..9978c33 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
@@ -29,10 +29,10 @@ import javassist.NotFoundException;
 import javassist.bytecode.Descriptor;
 
 /**
- * Transforms well-known HTTP URL connection classes
+ * Sets timeouts for HTTP calls done using <tt>java.net.URL</tt>/<tt>java.net.URLConnection</tt>.
  * 
- * <p>This implementation adds connect and read timeouts to those connections
- * if none are defined.</p>
+ * <p>It transforms calls to <tt>connect</tt> methods of internal URL connection classes to set the
+ * connect and read timeout in case they have the default value of <tt>0</tt>.</p>
  * 
  * @see URLConnection#getConnectTimeout()
  * @see URLConnection#getReadTimeout()


[sling-whiteboard] 03/07: Whitespace fix

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

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

commit ea60dfc41bd156a94348897ea82151128c41cd5b
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 11:47:57 2019 +0200

    Whitespace fix
---
 .../src/test/java/org/apache/sling/uca/impl/AgentIT.java                 | 1 -
 1 file changed, 1 deletion(-)

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 97f0c4c..a14c700 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
@@ -114,7 +114,6 @@ public class AgentIT {
         assertEquals(SocketTimeoutException.class.getName(), error.className);
         assertEquals("Read timed out", error.message);
     }
-    
 
     private RecordedThrowable runTest(String urlSpec, ClientType clientType) throws IOException, InterruptedException {
 


[sling-whiteboard] 06/07: Rename 'Main' to 'HttpClientLauncher'

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

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

commit bb2182820844a27bfd90c96f3e5a88ccdd275aa8
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 16:47:31 2019 +0200

    Rename 'Main' to 'HttpClientLauncher'
---
 .../src/test/java/org/apache/sling/uca/impl/AgentIT.java         | 8 ++++----
 .../apache/sling/uca/impl/{Main.java => HttpClientLauncher.java} | 9 ++++++---
 2 files changed, 10 insertions(+), 7 deletions(-)

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 3a343ba..59240cd 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
@@ -34,7 +34,7 @@ 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.apache.sling.uca.impl.HttpClientLauncher.ClientType;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
@@ -71,7 +71,7 @@ public class AgentIT {
      * @throws IOException various I/O problems 
      */
     @ParameterizedTest
-    @EnumSource(Main.ClientType.class)
+    @EnumSource(HttpClientLauncher.ClientType.class)
     public void connectTimeout(ClientType clientType) throws IOException {
 
         RecordedThrowable error = assertTimeout(ofSeconds(5),  () -> runTest("http://repo1.maven.org:81", clientType));
@@ -107,7 +107,7 @@ public class AgentIT {
      * @throws IOException various I/O problems
      */
     @ParameterizedTest
-    @EnumSource(Main.ClientType.class)
+    @EnumSource(HttpClientLauncher.ClientType.class)
     public void readTimeout(ClientType clientType, MisbehavingServerControl server) throws IOException {
         
         RecordedThrowable error = assertTimeout(ofSeconds(5),  () -> runTest("http://localhost:" + server.getLocalPort(), clientType));
@@ -165,7 +165,7 @@ public class AgentIT {
             "-javaagent:" + jar +"=" + TimeUnit.SECONDS.toMillis(connectTimeoutSeconds) +"," + TimeUnit.SECONDS.toMillis(readTimeoutSeconds)+",v",
             "-cp",
             classPath,
-            "org.apache.sling.uca.impl.Main",
+            HttpClientLauncher.class.getName(),
             url.toString(),
             clientType.toString()
         );
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/HttpClientLauncher.java
similarity index 96%
rename from url-connection-agent/src/test/java/org/apache/sling/uca/impl/Main.java
rename to url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java
index aee0725..9d8ca73 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/HttpClientLauncher.java
@@ -40,7 +40,10 @@ import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.util.EntityUtils;
 
-public class Main {
+/**
+ * CLI interface to run HTTP clients
+ */
+public class HttpClientLauncher {
     
     // TODO - write help messages with the values from this enum
     public enum ClientType {
@@ -50,7 +53,7 @@ public class Main {
     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");
+            throw new IllegalArgumentException("Usage: java -cp ... " + HttpClientLauncher.class.getName() + " <URL> JavaNet|HC3|HC4");
         
         System.out.println(new Date() + " [WEB] Executing request via " + args[1]);
 
@@ -65,7 +68,7 @@ public class Main {
                 runUsingHttpClient4(args[0]);
                 break;
             default:
-                throw new IllegalArgumentException("Usage: java -cp ... " + Main.class.getName() + " <URL> JavaNet|HC3|HC4");
+                throw new IllegalArgumentException("Usage: java -cp ... " + HttpClientLauncher.class.getName() + " <URL> JavaNet|HC3|HC4");
         }
     }
 


[sling-whiteboard] 02/07: Send back user-agent to help identify which requests come through

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

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

commit b5294eeaf562137f718437fd8dd71259e04b1765
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 11:47:48 2019 +0200

    Send back user-agent to help identify which requests come through
---
 .../test/java/org/apache/sling/uca/impl/MisbehavingServerExtension.java | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/MisbehavingServerExtension.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/MisbehavingServerExtension.java
index 702b82f..601adfd 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/MisbehavingServerExtension.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/MisbehavingServerExtension.java
@@ -94,6 +94,8 @@ class MisbehavingServerExtension implements BeforeAllCallback, AfterAllCallback,
             public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
                     throws IOException, ServletException {
                 response.setStatus(HttpServletResponse.SC_NO_CONTENT);
+                if ( baseRequest.getHeader("User-Agent") != null )
+                    response.addHeader("Original-User-Agent", baseRequest.getHeader("User-Agent"));
                 baseRequest.setHandled(true);
             }
         });


[sling-whiteboard] 01/07: Prevent NPE if agent is launched with no arguments

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

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

commit 7bde27c85fc92ac94983463ccb511c870ce2461b
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 11:47:33 2019 +0200

    Prevent NPE if agent is launched with no arguments
---
 url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 c35565f..a9320d6 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
@@ -25,7 +25,7 @@ public class Agent {
     public static void premain(String args, Instrumentation inst) {
         
         System.out.println("[AGENT] Loading agent...");
-        String[] parsedArgs = args.split(",");
+        String[] parsedArgs = args != null ? args.split(",") : new String[0];
         long connectTimeout =  TimeUnit.MINUTES.toMillis(1);
         long readTimeout = TimeUnit.MINUTES.toMillis(1);
         if ( parsedArgs.length > 0 )