You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2016/12/28 22:12:58 UTC

svn commit: r1776341 - /jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java

Author: pmouawad
Date: Wed Dec 28 22:12:58 2016
New Revision: 1776341

URL: http://svn.apache.org/viewvc?rev=1776341&view=rev
Log:
sonar: fix errors 

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java

Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java?rev=1776341&r1=1776340&r2=1776341&view=diff
==============================================================================
--- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java (original)
+++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java Wed Dec 28 22:12:58 2016
@@ -44,7 +44,6 @@ import java.util.prefs.Preferences;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.time.DateUtils;
@@ -103,10 +102,10 @@ import org.apache.oro.text.MalformedCach
 import org.apache.oro.text.regex.Pattern;
 import org.apache.oro.text.regex.Perl5Compiler;
 
-//For unit tests, @see TestProxyControl
 
 /**
  * Class handles storing of generated samples, etc
+ * For unit tests, see TestProxyControl
  */
 public class ProxyControl extends GenericController {
 
@@ -174,7 +173,6 @@ public class ProxyControl extends Generi
 
     // Must agree with the order of entries in the drop-down
     // created in ProxyControlGui.createGroupingPanel()
-    //private static final int GROUPING_NO_GROUPS = 0;
     private static final int GROUPING_ADD_SEPARATORS = 1;
     private static final int GROUPING_IN_SIMPLE_CONTROLLERS = 2;
     private static final int GROUPING_STORE_FIRST_ONLY = 3;
@@ -208,13 +206,19 @@ public class ProxyControl extends Generi
 
     private static final String DEFAULT_PASSWORD = "password"; // $NON-NLS-1$ NOSONAR only default password, if user has not defined one
 
-    // Keys for user preferences
+    /**
+     * Keys for user preferences
+     */
     private static final String USER_PASSWORD_KEY = "proxy_cert_password"; // NOSONAR not a hardcoded password
 
+    /**
+     * Note: Windows user preferences are stored relative to: HKEY_CURRENT_USER\Software\JavaSoft\Prefs
+     */
     private static final Preferences PREFERENCES = Preferences.userNodeForPackage(ProxyControl.class);
-    // Note: Windows user preferences are stored relative to: HKEY_CURRENT_USER\Software\JavaSoft\Prefs
 
-    // Whether to use dymanic key generation (if supported)
+    /**
+     * Whether to use dymanic key generation (if supported)
+     */
     private static final boolean USE_DYNAMIC_KEYS = JMeterUtils.getPropDefault("proxy.cert.dynamic_keys", true); // $NON-NLS-1$;
 
     // The alias to be used if dynamic host names are not possible
@@ -251,18 +255,22 @@ public class ProxyControl extends Generi
         }
     }
 
-    // Whether to use the redirect disabling feature (can be switched off if it does not work)
+    /**
+     * Whether to use the redirect disabling feature (can be switched off if it does not work)
+     */
     private static final boolean ATTEMPT_REDIRECT_DISABLING =
             JMeterUtils.getPropDefault("proxy.redirect.disabling", true); // $NON-NLS-1$
 
-    // Although this field is mutable, it is only accessed within the synchronized method deliverSampler()
+    /**
+     * Although this field is mutable, it is only accessed within the synchronized method deliverSampler()
+     */
     private static String LAST_REDIRECT = null;
+    
     /*
      * TODO this assumes that the redirected response will always immediately follow the original response.
      * This may not always be true.
      * Is there a better way to do this?
      */
-
     private transient Daemon server;
 
     private long lastTime = 0;// When was the last sample seen?
@@ -353,6 +361,10 @@ public class ProxyControl extends Generi
         setProperty(new BooleanProperty(ADD_ASSERTIONS, b));
     }
 
+    /**
+     * @param samplerTypeName
+     * @deprecated replaced by {@link ProxyControl#setSamplerTypeName(String)}
+     */
     @Deprecated
     public void setSamplerTypeName(int samplerTypeName) {
         setProperty(new IntegerProperty(SAMPLER_TYPE_NAME, samplerTypeName));
@@ -503,7 +515,7 @@ public class ProxyControl extends Generi
 
     public void startProxy() throws IOException {
         try {
-            initKeyStore(); // TODO display warning dialog as this can take some time
+            initKeyStore();
         } catch (GeneralSecurityException e) {
             log.error("Could not initialise key store", e);
             throw new IOException("Could not create keystore", e);
@@ -579,24 +591,23 @@ public class ProxyControl extends Generi
     public synchronized void deliverSampler(final HTTPSamplerBase sampler, final TestElement[] testElements, final SampleResult result) {
         boolean notifySampleListeners = true;
         if (sampler != null) {
-            if (ATTEMPT_REDIRECT_DISABLING && (samplerRedirectAutomatically || samplerFollowRedirects)) {
-                if (result instanceof HTTPSampleResult) {
-                    final HTTPSampleResult httpSampleResult = (HTTPSampleResult) result;
-                    final String urlAsString = httpSampleResult.getUrlAsString();
-                    if (urlAsString.equals(LAST_REDIRECT)) { // the url matches the last redirect
-                        sampler.setEnabled(false);
-                        sampler.setComment("Detected a redirect from the previous sample");
-                    } else { // this is not the result of a redirect
-                        LAST_REDIRECT = null; // so break the chain
-                    }
-                    if (httpSampleResult.isRedirect()) { // Save Location so resulting sample can be disabled
-                        if (LAST_REDIRECT == null) {
-                            sampler.setComment("Detected the start of a redirect chain");
-                        }
-                        LAST_REDIRECT = httpSampleResult.getRedirectLocation();
-                    } else {
-                        LAST_REDIRECT = null;
+            if (ATTEMPT_REDIRECT_DISABLING && (samplerRedirectAutomatically || samplerFollowRedirects)
+                    && result instanceof HTTPSampleResult) {
+                final HTTPSampleResult httpSampleResult = (HTTPSampleResult) result;
+                final String urlAsString = httpSampleResult.getUrlAsString();
+                if (urlAsString.equals(LAST_REDIRECT)) { // the url matches the last redirect
+                    sampler.setEnabled(false);
+                    sampler.setComment("Detected a redirect from the previous sample");
+                } else { // this is not the result of a redirect
+                    LAST_REDIRECT = null; // so break the chain
+                }
+                if (httpSampleResult.isRedirect()) { // Save Location so resulting sample can be disabled
+                    if (LAST_REDIRECT == null) {
+                        sampler.setComment("Detected the start of a redirect chain");
                     }
+                    LAST_REDIRECT = httpSampleResult.getRedirectLocation();
+                } else {
+                    LAST_REDIRECT = null;
                 }
             }
             if (filterContentType(result) && filterUrl(sampler)) {
@@ -632,7 +643,7 @@ public class ProxyControl extends Generi
         }
         if(notifySampleListeners) {
             // SampleEvent is not passed JMeterVariables, because they don't make sense for Proxy Recording
-            notifySampleListeners(new SampleEvent(result, "WorkBench")); // TODO - is this the correct threadgroup name?
+            notifySampleListeners(new SampleEvent(result, "WorkBench"));
         } else {
             log.debug("Sample not delivered to Child Sampler Listener based on url or content-type: " + result.getUrlAsString() + " - " + result.getContentType());
         }
@@ -649,7 +660,7 @@ public class ProxyControl extends Generi
      * @return {@link Authorization}
      */
     private Authorization createAuthorization(final TestElement[] testElements, HTTPSamplerBase sampler) {
-        Header authHeader = null;
+        Header authHeader;
         Authorization authorization = null;
         // Iterate over subconfig elements searching for HeaderManager
         for (TestElement te : testElements) {
@@ -663,8 +674,8 @@ public class ProxyControl extends Generi
                         //Construct Authorization object from HEADER_AUTHORIZATION
                         authHeader = (Header) tep.getObjectValue();
                         String[] authHeaderContent = authHeader.getValue().split(" ");//$NON-NLS-1$
-                        String authType = null;
-                        String authCredentialsBase64 = null;
+                        String authType;
+                        String authCredentialsBase64;
                         if(authHeaderContent.length>=2) {
                             authType = authHeaderContent[0];
                             authCredentialsBase64 = authHeaderContent[1];
@@ -713,6 +724,7 @@ public class ProxyControl extends Generi
                 server.join(1000); // wait for server to stop
             } catch (InterruptedException e) {
                 //NOOP
+                Thread.currentThread().interrupt();
             }
             notifyTestListenersOfEnd();
             server = null;
@@ -737,7 +749,7 @@ public class ProxyControl extends Generi
                 return new String[]{"Problem with root certificate", e.getMessage()};
             }
         }
-        return null; // should not happen
+        return new String[0]; // should not happen
     }
     // Package protected to allow test case access
     boolean filterUrl(HTTPSamplerBase sampler) {
@@ -748,17 +760,13 @@ public class ProxyControl extends Generi
 
         String url = generateMatchUrl(sampler);
         CollectionProperty includePatterns = getIncludePatterns();
-        if (includePatterns.size() > 0) {
-            if (!matchesPatterns(url, includePatterns)) {
-                return false;
-            }
+        if (includePatterns.size() > 0 && !matchesPatterns(url, includePatterns)) {
+            return false;
         }
 
         CollectionProperty excludePatterns = getExcludePatterns();
-        if (excludePatterns.size() > 0) {
-            if (matchesPatterns(url, excludePatterns)) {
-                return false;
-            }
+        if (excludePatterns.size() > 0 && matchesPatterns(url, excludePatterns)) {
+            return false;
         }
 
         return true;
@@ -847,7 +855,7 @@ public class ProxyControl extends Generi
     private void setAuthorization(Authorization authorization, JMeterTreeNode target) {
         JMeterTreeModel jmeterTreeModel = getJmeterTreeModel();
         List<JMeterTreeNode> authManagerNodes = jmeterTreeModel.getNodesOfType(AuthManager.class);
-        if (authManagerNodes.size() == 0) {
+        if (authManagerNodes.isEmpty()) {
             try {
                 log.debug("Creating HTTP Authentication manager for authorization:"+authorization);
                 AuthManager authManager = newAuthorizationManager(authorization);
@@ -910,15 +918,12 @@ public class ProxyControl extends Generi
             final JMeterTreeModel model,
             final JMeterTreeNode node,
             final GenericController controller) {
-        JMeterUtils.runSafe(true, new Runnable() {
-            @Override
-            public void run() {
-                try {
-                    model.addComponent(controller, node);
-                } catch (IllegalUserActionException e) {
-                    log.error("Program error", e);
-                    throw new Error(e);
-                }
+        JMeterUtils.runSafe(true, () -> {
+            try {
+                model.addComponent(controller, node);
+            } catch (IllegalUserActionException e) {
+                log.error("Program error", e);
+                throw new Error(e);
             }
         });
     }
@@ -1186,32 +1191,30 @@ public class ProxyControl extends Generi
             final long deltaTFinal = deltaT;
             final boolean firstInBatchFinal = firstInBatch;
             final JMeterTreeNode myTargetFinal = myTarget;
-            JMeterUtils.runSafe(true, new Runnable() {
-                @Override
-                public void run() {
-                    try {
-                        final JMeterTreeNode newNode = treeModel.addComponent(sampler, myTargetFinal);
-                        if (firstInBatchFinal) {
-                            if (addAssertions) {
-                                addAssertion(treeModel, newNode);
-                            }
-                            addTimers(treeModel, newNode, deltaTFinal);
+            JMeterUtils.runSafe(true, () -> {
+                try {
+                    final JMeterTreeNode newNode = treeModel.addComponent(sampler, myTargetFinal);
+                    if (firstInBatchFinal) {
+                        if (addAssertions) {
+                            addAssertion(treeModel, newNode);
                         }
+                        addTimers(treeModel, newNode, deltaTFinal);
+                    }
 
-                        if (testElements != null) {
-                            for (TestElement testElement: testElements) {
-                                if (isAddableTestElement(testElement)) {
-                                    treeModel.addComponent(testElement, newNode);
-                                }
+                    if (testElements != null) {
+                        for (TestElement testElement: testElements) {
+                            if (isAddableTestElement(testElement)) {
+                                treeModel.addComponent(testElement, newNode);
                             }
                         }
-                    } catch (IllegalUserActionException e) {
-                        JMeterUtils.reportErrorToUser(e.getMessage());
                     }
+                } catch (IllegalUserActionException e) {
+                    log.error("Error placing sampler", e);
+                    JMeterUtils.reportErrorToUser(e.getMessage());
                 }
-
             });
         } catch (Exception e) {
+            log.error("Error placing sampler", e);
             JMeterUtils.reportErrorToUser(e.getMessage());
         }
     }
@@ -1429,8 +1432,8 @@ public class ProxyControl extends Generi
             initJMeterKeyStore();
             break;
         case USER_KEYSTORE:
-            storePassword = JMeterUtils.getPropDefault("proxy.cert.keystorepass", DEFAULT_PASSWORD); // $NON-NLS-1$;
-            keyPassword = JMeterUtils.getPropDefault("proxy.cert.keypassword", DEFAULT_PASSWORD); // $NON-NLS-1$;
+            storePassword = JMeterUtils.getPropDefault("proxy.cert.keystorepass", DEFAULT_PASSWORD); // $NON-NLS-1$
+            keyPassword = JMeterUtils.getPropDefault("proxy.cert.keypassword", DEFAULT_PASSWORD); // $NON-NLS-1$
             log.info("HTTP(S) Test Script Recorder will use the keystore '"+ CERT_PATH_ABS + "' with the alias: '" + CERT_ALIAS + "'");
             initUserKeyStore();
             break;
@@ -1456,7 +1459,7 @@ public class ProxyControl extends Generi
             }
         } catch (Exception e) {
             keyStore = null;
-            log.error("Could not open keystore or certificate is not valid " + CERT_PATH_ABS + " " + e.getMessage());
+            log.error("Could not open keystore or certificate is not valid " + CERT_PATH_ABS + " " + e.getMessage(), e);
         }
     }
 
@@ -1480,13 +1483,13 @@ public class ProxyControl extends Generi
             } catch (IOException e) { // store is faulty, we need to recreate it
                 keyStore = null; // if cert is not valid, flag up to recreate it
                 if (e.getCause() instanceof UnrecoverableKeyException) {
-                    log.warn("Could not read key store " + e.getMessage() + "; cause: " + e.getCause().getMessage());
+                    log.warn("Could not read key store " + e.getMessage() + "; cause: " + e.getCause().getMessage(), e);
                 } else {
-                    log.warn("Could not open/read key store " + e.getMessage()); // message includes the file name
+                    log.warn("Could not open/read key store " + e.getMessage(), e); // message includes the file name
                 }
             } catch (GeneralSecurityException e) {
                 keyStore = null; // if cert is not valid, flag up to recreate it
-                log.warn("Problem reading key store: " + e.getMessage());
+                log.warn("Problem reading key store: " + e.getMessage(), e);
             }
         }
         if (keyStore == null) { // no existing file or not valid
@@ -1547,7 +1550,7 @@ public class ProxyControl extends Generi
                 caCert.checkValidity(new Date(System.currentTimeMillis()+DateUtils.MILLIS_PER_DAY));
             } catch (Exception e) { // store is faulty, we need to recreate it
                 keyStore = null; // if cert is not valid, flag up to recreate it
-                log.warn("Could not open expected file or certificate is not valid " + CERT_PATH_ABS  + " " + e.getMessage());
+                log.warn("Could not open expected file or certificate is not valid " + CERT_PATH_ABS  + " " + e.getMessage(), e);
             }
         }
         if (keyStore == null) { // no existing file or not valid
@@ -1564,16 +1567,12 @@ public class ProxyControl extends Generi
     }
 
     private KeyStore getKeyStore(char[] password) throws GeneralSecurityException, IOException {
-        InputStream in = null;
-        try {
-            in = new BufferedInputStream(new FileInputStream(CERT_PATH));
+        try (InputStream in = new BufferedInputStream(new FileInputStream(CERT_PATH))) {
             log.debug("Opened Keystore file: " + CERT_PATH_ABS);
             KeyStore ks = KeyStore.getInstance(KEYSTORE_TYPE);
             ks.load(in, password);
             log.debug("Loaded Keystore file: " + CERT_PATH_ABS);
             return ks;
-        } finally {
-            IOUtils.closeQuietly(in);
         }
     }