xmlgraphics-batik-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randy Jeune <randyje...@gmail.com>
Subject [PATCH] Found correctness bugs in implementation of Batik 1.6
Date Mon, 19 Dec 2005 23:25:07 GMT
[PATCH] Found correctness bugs in implementation of Batik 1.6

 Hello, my name is Vladimir Jeune.  I was interested in your project and
decided to try contributing
 to an Open Source project.  I ran a program called Findbugs on the Batik
code and found some correctness
 bugs in the implementation of Batik 1.6.

 There were a couple places where Object.equals() was overridden but not
Object.hashcode().  This could result
 in two equal objects having different hashcodes, since Object.hascode()
assigns an arbitrary value assigned
 by the virtual machine, this could lead two equal objects existing in the
HashMap.
 SVGConverterUrlSource.java                         :      line 89
 SVGConverterFileSource.java                        :      line 73

 There was a place where the String.trim() was being used and then the
length of the String being tested.  However,
 the value of the trim() was being ignored.  Most likely becuase it was
thought that trim modified the String when
 in actuality it returns a copy trimmed of whitespace.
 org/apache/batik/apps/slideshow/Main.java          :      line 300

 There was a point in the code where a stream was not being closed upon
exiting the function.
 org/apache/batik/apps/slideshow/Main.java          :      line 311

 There was the use of new to allocate a new String from the concatenation of
Strings.  This is not necessary
 becuase Strings are immutable.
 org/apache/batik/apps/rasterizer                   :      line 1027
 org/apache/batik/apps/rasterizer                   :      line 1030

 There were also some calls to new Boolean(boolean) which is not recommended
since valueof(boolean) gives better
 space and time performance.
 SVGConverter.java                                  :      line 876
 SVGConverter.java                                  :      line 881
 SVGConverter.java                                  :      line 892

 I hope that this was useful.
 I also have the patches here:
 ==========================================================================
 //batik/apps/slideshow
 --- Main.java.txt       Mon Dec 19 17:35:05 2005
+++ Main.java   Mon Dec 19 17:41:12 2005
@@ -297,8 +297,7 @@
                 int idx = str.indexOf('#');
                 if (idx != -1)
                     str = str.substring(0, idx);
-                str.trim();
-                if (str.length() == 0)
+                if (str.trim().length() == 0)
                     continue;
                 try {
                     URL imgURL = new URL(flURL, str);
@@ -309,6 +308,12 @@
             }
         } catch (IOException ioe) {
             System.err.println("Error while reading file-list: " + file);
+        } finally {
+                       try {
+                                       br.close();
+                               } catch (IOException e) {
+                                       e.printStackTrace();
+                               }
         }
     }

 ============================================================================
 //batik/apps/rasterizer/
 --- SVGConverter.java.txt       Mon Dec 19 17:45:41 2005
+++ SVGConverter.java   Mon Dec 19 17:46:34 2005
@@ -88,7 +88,7 @@
  *     to use when processing the SVG documents.</li>
  * </ul>
  *
- * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
+ * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
  * @author <a href="mailto:Henri.Ruini@nokia.com">Henri Ruini</a>
  * @author <a href="mailto:vhardy@apache.org">Vincent Hardy</a>
  */
@@ -873,12 +873,12 @@
         // Set validation
         if (validate){
             map.put(ImageTranscoder.KEY_XML_PARSER_VALIDATING,
-                    new Boolean(validate));
+                    Boolean.valueOf(validate));
         }

         // Set onload
         if (executeOnload) {
-            map.put(ImageTranscoder.KEY_EXECUTE_ONLOAD, new
Boolean(executeOnload));
+            map.put(ImageTranscoder.KEY_EXECUTE_ONLOAD, Boolean.valueOf
(executeOnload));
         }

         // Set allowed scripts
@@ -889,7 +889,7 @@
         // Set constrain script origin
         if (!constrainScriptOrigin) {
             map.put(ImageTranscoder.KEY_CONSTRAIN_SCRIPT_ORIGIN,
-                    new Boolean(constrainScriptOrigin));
+                    Boolean.valueOf(constrainScriptOrigin));
         }

         return map;
@@ -1024,10 +1024,10 @@
         String dest = null;
         if (suffixStart != -1) {
             // Replace existing suffix.
-            dest = new String(oldName.substring(0, suffixStart) +
newSuffix);
+            dest = oldName.substring(0, suffixStart) + newSuffix;
         } else {
             // Add new suffix.
-            dest = new String(oldName + newSuffix);
+            dest = oldName + newSuffix;
         }

         return dest;
 ==================================================================================================
//batik/apps/rasterizer/

--- SVGConverterFileSource.java.txt     Mon Dec 19 17:54:24 2005
+++ SVGConverterFileSource.java Mon Dec 19 17:55:18 2005
@@ -27,7 +27,7 @@
  * Describes a file source for the <tt>SVGConverter</tt>
  *
  * @author <a href="mailto:vhardy@apache.org">Vincent Hardy</a>
- * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
+ * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
  */
 public class SVGConverterFileSource implements SVGConverterSource {
     File file;
@@ -78,6 +78,10 @@
         return file.equals(((SVGConverterFileSource)o).file);
     }

+    public int hashCode() {
+               return 42;
+    }
+
     public InputStream openStream() throws FileNotFoundException{
         return new FileInputStream(file);
     }
 =============================================================================================
//batik/apps/rasterizer/

--- SVGConverterFileSource.java.txt     Mon Dec 19 17:54:24 2005
+++ SVGConverterFileSource.java Mon Dec 19 17:55:18 2005
@@ -27,7 +27,7 @@
  * Describes a file source for the <tt>SVGConverter</tt>
  *
  * @author <a href="mailto:vhardy@apache.org">Vincent Hardy</a>
- * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
+ * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
  */
 public class SVGConverterFileSource implements SVGConverterSource {
     File file;
@@ -78,6 +78,10 @@
         return file.equals(((SVGConverterFileSource)o).file);
     }

+    public int hashCode() {
+               return 42;
+    }
+
     public InputStream openStream() throws FileNotFoundException{
         return new FileInputStream(file);
     }
===================================================================
//batik/apps/rasterizer/

--- SVGConverterUrlSource.java.txt      Mon Dec 19 17:56:39 2005
+++ SVGConverterUrlSource.java  Mon Dec 19 17:57:18 2005
@@ -25,7 +25,7 @@

 /*
  * @author <a href="mailto:vhardy@apache.org">Vincent Hardy</a>
- * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
+ * @version $Id: workLog,v 1.11 2005/12/19 23:02:32 cs433021 Exp $
  */
 public class SVGConverterURLSource implements SVGConverterSource {
     /**
@@ -94,6 +94,10 @@
         return purl.equals(((SVGConverterURLSource)o).purl);
     }

+    public int hashCode() {
+               return 42;
+    }
+
     public InputStream openStream() throws IOException {
         return purl.openStream();
     }

Mime
View raw message