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 2015/02/19 22:30:55 UTC

svn commit: r1661007 - in /jmeter/trunk: src/components/org/apache/jmeter/control/ src/core/org/apache/jmeter/ src/core/org/apache/jmeter/gui/action/ src/core/org/apache/jmeter/save/ test/src/org/apache/jmeter/gui/action/ test/src/org/apache/jmeter/pro...

Author: pmouawad
Date: Thu Feb 19 21:30:55 2015
New Revision: 1661007

URL: http://svn.apache.org/r1661007
Log:
Bug 57605 - When there is an error loading Test Plan, SaveService.loadTree returns null leading to NPE in callers
Bugzilla Id: 57605

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/control/IncludeController.java
    jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
    jmeter/trunk/src/core/org/apache/jmeter/gui/action/Load.java
    jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java
    jmeter/trunk/test/src/org/apache/jmeter/gui/action/TestLoad.java
    jmeter/trunk/test/src/org/apache/jmeter/protocol/http/modifier/TestAnchorModifier.java
    jmeter/trunk/test/src/org/apache/jmeter/save/TestSaveService.java
    jmeter/trunk/xdocs/changes.xml

Modified: jmeter/trunk/src/components/org/apache/jmeter/control/IncludeController.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/control/IncludeController.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/src/components/org/apache/jmeter/control/IncludeController.java (original)
+++ jmeter/trunk/src/components/org/apache/jmeter/control/IncludeController.java Thu Feb 19 21:30:55 2015
@@ -19,10 +19,8 @@
 package org.apache.jmeter.control;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.InputStream;
 import java.util.Iterator;
 import java.util.LinkedList;
 
@@ -34,7 +32,6 @@ import org.apache.jmeter.testelement.Tes
 import org.apache.jmeter.util.JMeterUtils;
 import org.apache.jorphan.collections.HashTree;
 import org.apache.jorphan.logging.LoggingManager;
-import org.apache.jorphan.util.JOrphanUtils;
 import org.apache.log.Logger;
 
 public class IncludeController extends GenericController implements ReplaceableController {
@@ -124,7 +121,6 @@ public class IncludeController extends G
     protected HashTree loadIncludedElements() {
         // only try to load the JMX test plan if there is one
         final String includePath = getIncludePath();
-        InputStream reader = null;
         HashTree tree = null;
         if (includePath != null && includePath.length() > 0) {
             try {
@@ -142,8 +138,7 @@ public class IncludeController extends G
                     }
                 }
                 
-                reader = new FileInputStream(file);
-                tree = SaveService.loadTree(reader);
+                tree = SaveService.loadTree(file);
                 // filter the tree for a TestFragment.
                 tree = getProperBranch(tree);
                 removeDisabledItems(tree);
@@ -168,9 +163,6 @@ public class IncludeController extends G
                 JMeterUtils.reportErrorToUser(msg);
                 log.warn("Unexpected error", ex);
             }
-            finally{
-                JOrphanUtils.closeQuietly(reader);
-            }
         }
         return tree;
     }

Modified: jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/JMeter.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/JMeter.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/JMeter.java Thu Feb 19 21:30:55 2015
@@ -53,10 +53,10 @@ import org.apache.commons.cli.avalon.CLO
 import org.apache.commons.cli.avalon.CLUtil;
 import org.apache.jmeter.control.ReplaceableController;
 import org.apache.jmeter.engine.ClientJMeterEngine;
+import org.apache.jmeter.engine.DistributedRunner;
 import org.apache.jmeter.engine.JMeterEngine;
 import org.apache.jmeter.engine.RemoteJMeterEngineImpl;
 import org.apache.jmeter.engine.StandardJMeterEngine;
-import org.apache.jmeter.engine.DistributedRunner;
 import org.apache.jmeter.exceptions.IllegalUserActionException;
 import org.apache.jmeter.gui.GuiPackage;
 import org.apache.jmeter.gui.MainFrame;
@@ -244,14 +244,12 @@ public class JMeter implements JMeterPlu
         main.setVisible(true);
         ActionRouter.getInstance().actionPerformed(new ActionEvent(main, 1, ActionNames.ADD_ALL));
         if (testFile != null) {
-            FileInputStream reader = null;
             try {
                 File f = new File(testFile);
                 log.info("Loading file: " + f);
                 FileServer.getFileServer().setBaseForScript(f);
 
-                reader = new FileInputStream(f);
-                HashTree tree = SaveService.loadTree(reader);
+                HashTree tree = SaveService.loadTree(f);
 
                 GuiPackage.getInstance().setTestPlanFile(f.getAbsolutePath());
 
@@ -262,8 +260,6 @@ public class JMeter implements JMeterPlu
             } catch (Exception e) {
                 log.error("Failure loading test file", e);
                 JMeterUtils.reportErrorToUser(e.toString());
-            } finally {
-                JOrphanUtils.closeQuietly(reader);
             }
         } else {
             JTree jTree = GuiPackage.getInstance().getMainFrame().getTree();
@@ -743,7 +739,6 @@ public class JMeter implements JMeterPlu
 
     // run test in batch mode
     private void runNonGui(String testFile, String logFile, boolean remoteStart, String remote_hosts_string) {
-        FileInputStream reader = null;
         try {
             File f = new File(testFile);
             if (!f.exists() || !f.isFile()) {
@@ -752,10 +747,7 @@ public class JMeter implements JMeterPlu
             }
             FileServer.getFileServer().setBaseForScript(f);
 
-            reader = new FileInputStream(f);
-            log.info("Loading file: " + f);
-
-            HashTree tree = SaveService.loadTree(reader);
+            HashTree tree = SaveService.loadTree(f);
 
             @SuppressWarnings("deprecation") // Deliberate use of deprecated ctor
             JMeterTreeModel treeModel = new JMeterTreeModel(new Object());// Create non-GUI version to avoid headless problems
@@ -826,8 +818,6 @@ public class JMeter implements JMeterPlu
         } catch (Exception e) {
             System.out.println("Error in NonGUIDriver " + e.toString());
             log.error("Error in NonGUIDriver", e);
-        } finally {
-            JOrphanUtils.closeQuietly(reader);
         }
     }
 

Modified: jmeter/trunk/src/core/org/apache/jmeter/gui/action/Load.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/action/Load.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/gui/action/Load.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/gui/action/Load.java Thu Feb 19 21:30:55 2015
@@ -20,9 +20,7 @@ package org.apache.jmeter.gui.action;
 
 import java.awt.event.ActionEvent;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -44,7 +42,6 @@ import org.apache.jmeter.testelement.Wor
 import org.apache.jmeter.util.JMeterUtils;
 import org.apache.jorphan.collections.HashTree;
 import org.apache.jorphan.logging.LoggingManager;
-import org.apache.jorphan.util.JOrphanUtils;
 import org.apache.log.Logger;
 
 import com.thoughtworks.xstream.converters.ConversionException;
@@ -119,7 +116,6 @@ public class Load implements Command {
 
         final GuiPackage guiPackage = GuiPackage.getInstance();
         if (f != null) {
-            InputStream reader = null;
             try {
                 if (merging) {
                     log.info("Merging file: " + f);
@@ -131,8 +127,7 @@ public class Load implements Command {
                         FileServer.getFileServer().setBaseForScript(f);
                     }
                 }
-                reader = new FileInputStream(f);
-                final HashTree tree = SaveService.loadTree(reader);
+                final HashTree tree = SaveService.loadTree(f);
                 final boolean isTestPlan = insertLoadedTree(e.getID(), tree, merging);
 
                 // don't change name if merging
@@ -150,8 +145,6 @@ public class Load implements Command {
                 reportError("Error reading file: ", ex, false);
             } catch (Exception ex) {
                 reportError("Unexpected error", ex, true);
-            } finally {
-                JOrphanUtils.closeQuietly(reader);
             }
             FileDialoger.setLastJFCDirectory(f.getParentFile().getAbsolutePath());
             guiPackage.updateCurrentGui();

Modified: jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java Thu Feb 19 21:30:55 2015
@@ -19,6 +19,7 @@
 package org.apache.jmeter.save;
 
 import java.io.BufferedInputStream;
+import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -506,40 +507,44 @@ public class SaveService {
 
     /**
      * Load a Test tree (JMX file)
-     * @param reader on the JMX file
+     * @param file the JMX file
      * @return the loaded tree
      * @throws IOException if there is a problem reading the file or processing it
      */
-    public static HashTree loadTree(InputStream reader) throws IOException {
-        if (!reader.markSupported()) {
-            reader = new BufferedInputStream(reader);
-        }
-        reader.mark(Integer.MAX_VALUE);
-        ScriptWrapper wrapper = null;
+    public static HashTree loadTree(File file) throws IOException {
+        log.info("Loading file: " + file);
+        InputStream reader = null;
         try {
-            // Get the InputReader to use
-            InputStreamReader inputStreamReader = getInputStreamReader(reader);
-            wrapper = (ScriptWrapper) JMXSAVER.fromXML(inputStreamReader);
-            inputStreamReader.close();
-            if (wrapper == null){
-                log.error("Problem loading XML: see above.");
-                return null;
+            reader = new FileInputStream(file);
+            if (!reader.markSupported()) {
+                reader = new BufferedInputStream(reader);
             }
-            return wrapper.testPlan;
-        } catch (CannotResolveClassException e) {
-            if (e.getMessage().startsWith("node")) {
-                log.info("Problem loading XML, trying Avalon format");
-                reader.reset();
-                return OldSaveService.loadSubTree(reader);                
+            reader.mark(Integer.MAX_VALUE);
+            ScriptWrapper wrapper = null;
+            try {
+                // Get the InputReader to use
+                InputStreamReader inputStreamReader = getInputStreamReader(reader);
+                wrapper = (ScriptWrapper) JMXSAVER.fromXML(inputStreamReader);
+                inputStreamReader.close();
+                if (wrapper == null){
+                    log.error("Problem loading XML: see above.");
+                    return null;
+                }
+                return wrapper.testPlan;
+            } catch (CannotResolveClassException e) {
+                if (e.getMessage().startsWith("node")) {
+                    log.info("Problem loading XML, trying Avalon format");
+                    reader.reset();
+                    return OldSaveService.loadSubTree(reader);                
+                }
+                throw new IllegalArgumentException("Problem loading XML from:'"+file.getAbsolutePath()+"', cannot determine class for element: " + e, e);
+            } catch (NoClassDefFoundError e) {
+                throw new IllegalArgumentException("Problem loading XML from:'"+file.getAbsolutePath()+"', missing class "+e , e);
+            } catch (ConversionException e) {
+                throw new IllegalArgumentException("Problem loading XML from:'"+file.getAbsolutePath()+"', conversion error "+e , e);
             }
-            log.warn("Problem loading XML, cannot determine class for element: " + e.getLocalizedMessage());
-            return null;
-        } catch (NoClassDefFoundError e) {
-            log.error("Missing class "+e);
-            return null;
-        } catch (ConversionException e) {
-            log.error("Conversion error "+e);
-            return null;
+        } finally {
+            JOrphanUtils.closeQuietly(reader);
         }
     }
 

Modified: jmeter/trunk/test/src/org/apache/jmeter/gui/action/TestLoad.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/gui/action/TestLoad.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/gui/action/TestLoad.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/gui/action/TestLoad.java Thu Feb 19 21:30:55 2015
@@ -19,7 +19,6 @@
 package org.apache.jmeter.gui.action;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FilenameFilter;
 import java.util.HashSet;
 import java.util.Set;
@@ -107,13 +106,7 @@ public class TestLoad extends JMeterTest
     }
 
     private HashTree getTree(File f) throws Exception {
-        FileInputStream fis = new FileInputStream(f);
-        HashTree tree = null;
-        try {
-            tree = SaveService.loadTree(fis);
-        } finally {
-            fis.close();
-        }
+        HashTree tree = SaveService.loadTree(f);
         return tree;
     }
 }

Modified: jmeter/trunk/test/src/org/apache/jmeter/protocol/http/modifier/TestAnchorModifier.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/modifier/TestAnchorModifier.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/protocol/http/modifier/TestAnchorModifier.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/protocol/http/modifier/TestAnchorModifier.java Thu Feb 19 21:30:55 2015
@@ -18,7 +18,7 @@
 
 package org.apache.jmeter.protocol.http.modifier;
 
-import java.io.FileInputStream;
+import java.io.File;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLEncoder;
@@ -48,12 +48,12 @@ public class TestAnchorModifier extends
         }
 
         public void testProcessingHTMLFile(String HTMLFileName) throws Exception {
-            HTTPSamplerBase config = (HTTPSamplerBase) SaveService.loadTree(
-                    new FileInputStream(System.getProperty("user.dir") + "/testfiles/load_bug_list.jmx")).getArray()[0];
+            File file = new File(System.getProperty("user.dir") + "/testfiles/load_bug_list.jmx");
+            HTTPSamplerBase config = (HTTPSamplerBase) SaveService.loadTree(file).getArray()[0];
             config.setRunningVersion(true);
             HTTPSampleResult result = new HTTPSampleResult();
-            HTTPSamplerBase context = (HTTPSamplerBase) SaveService.loadTree(
-                    new FileInputStream(System.getProperty("user.dir") + "/testfiles/Load_JMeter_Page.jmx")).getArray()[0];
+            file = new File(System.getProperty("user.dir") + "/testfiles/Load_JMeter_Page.jmx");
+            HTTPSamplerBase context = (HTTPSamplerBase) SaveService.loadTree(file).getArray()[0];
             jmctx.setCurrentSampler(context);
             jmctx.setCurrentSampler(config);
             result.setResponseData(new TextFile(System.getProperty("user.dir") + HTMLFileName).getText(), null);

Modified: jmeter/trunk/test/src/org/apache/jmeter/save/TestSaveService.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/save/TestSaveService.java?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/save/TestSaveService.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/save/TestSaveService.java Thu Feb 19 21:30:55 2015
@@ -22,10 +22,8 @@ import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FileReader;
-import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.List;
 
@@ -110,16 +108,7 @@ public class TestSaveService extends JMe
 
         int [] orig = readFile(new BufferedReader(new FileReader(testFile)));
 
-        InputStream in = null;
-        HashTree tree = null;
-        try {
-            in = new FileInputStream(testFile);
-            tree = SaveService.loadTree(in);
-        } finally {
-            if(in != null) {
-                in.close();
-            }
-        }
+        HashTree tree = SaveService.loadTree(testFile);
 
         ByteArrayOutputStream out = new ByteArrayOutputStream(1000000);
         try {
@@ -191,16 +180,14 @@ public class TestSaveService extends JMe
 
     public void testLoad() throws Exception {
         for (int i = 0; i < FILES_LOAD_ONLY.length; i++) {
-            InputStream in = null;
+            File file = findTestFile("testfiles/" + FILES_LOAD_ONLY[i]);
             try {
-                in = new FileInputStream(findTestFile("testfiles/" + FILES_LOAD_ONLY[i]));
-                HashTree tree =SaveService.loadTree(in);
+                HashTree tree =SaveService.loadTree(file);
                 assertNotNull(tree);
-            } finally {
-                if(in != null) {
-                    in.close();
-                }
+            } catch(IllegalArgumentException ex) {
+                fail("Exception loading "+file.getAbsolutePath());
             }
+            
         }
 
     }

Modified: jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1661007&r1=1661006&r2=1661007&view=diff
==============================================================================
--- jmeter/trunk/xdocs/changes.xml (original)
+++ jmeter/trunk/xdocs/changes.xml Thu Feb 19 21:30:55 2015
@@ -235,6 +235,7 @@ See  <bugzilla>56357</bugzilla> for deta
 <h3>General</h3>
 <ul>
 <li><bug>57518</bug>Icons for toolbar with several sizes</li>
+<li><bug>57605</bug>When there is an error loading Test Plan, SaveService.loadTree returns null leading to NPE in callers</li>
 </ul>
 <ch_section>Non-functional changes</ch_section>
 <ul>



Re: svn commit: r1661007 - in /jmeter/trunk: src/components/org/apache/jmeter/control/ src/core/org/apache/jmeter/ src/core/org/apache/jmeter/gui/action/ src/core/org/apache/jmeter/save/ test/src/org/apache/jmeter/gui/action/ test/src/org/apache/jmeter/pro...

Posted by sebb <se...@gmail.com>.
On 19 February 2015 at 21:30,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Thu Feb 19 21:30:55 2015
> New Revision: 1661007
>
> URL: http://svn.apache.org/r1661007
> Log:
> Bug 57605 - When there is an error loading Test Plan, SaveService.loadTree returns null leading to NPE in callers
> Bugzilla Id: 57605

-1, see below.

> Modified:
>     jmeter/trunk/src/components/org/apache/jmeter/control/IncludeController.java
>     jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
>     jmeter/trunk/src/core/org/apache/jmeter/gui/action/Load.java
>     jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java
>     jmeter/trunk/test/src/org/apache/jmeter/gui/action/TestLoad.java
>     jmeter/trunk/test/src/org/apache/jmeter/protocol/http/modifier/TestAnchorModifier.java
>     jmeter/trunk/test/src/org/apache/jmeter/save/TestSaveService.java
>     jmeter/trunk/xdocs/changes.xml
>
...
> Modified: jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java?rev=1661007&r1=1661006&r2=1661007&view=diff
> ==============================================================================
> --- jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java (original)
> +++ jmeter/trunk/src/core/org/apache/jmeter/save/SaveService.java Thu Feb 19 21:30:55 2015
> @@ -19,6 +19,7 @@
>  package org.apache.jmeter.save;
>
>  import java.io.BufferedInputStream;
> +import java.io.File;
>  import java.io.FileInputStream;
>  import java.io.IOException;
>  import java.io.InputStream;
> @@ -506,40 +507,44 @@ public class SaveService {
>
>      /**
>       * Load a Test tree (JMX file)
> -     * @param reader on the JMX file
> +     * @param file the JMX file
>       * @return the loaded tree
>       * @throws IOException if there is a problem reading the file or processing it
>       */
> -    public static HashTree loadTree(InputStream reader) throws IOException {
> -        if (!reader.markSupported()) {
> -            reader = new BufferedInputStream(reader);
> -        }
> -        reader.mark(Integer.MAX_VALUE);
> -        ScriptWrapper wrapper = null;
> +    public static HashTree loadTree(File file) throws IOException {

-1

This breaks the API.

The code needs to be fixed so that it continues to support the method:

 public static HashTree loadTree(InputStream reader) throws IOException

It should be easy enough to do this without much code duplication.

> +        log.info("Loading file: " + file);
> +        InputStream reader = null;
>          try {
> -            // Get the InputReader to use
> -            InputStreamReader inputStreamReader = getInputStreamReader(reader);
> -            wrapper = (ScriptWrapper) JMXSAVER.fromXML(inputStreamReader);
> -            inputStreamReader.close();
> -            if (wrapper == null){
> -                log.error("Problem loading XML: see above.");
> -                return null;
> +            reader = new FileInputStream(file);