[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="" href="mailto:[EMAIL PROTECTED]" target="_blank" >[EMAIL PROTECTED]">Henri Ruini</a>
* @author <a href="" href="mailto:[EMAIL PROTECTED]" target="_blank" >[EMAIL PROTECTED]">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="" href="mailto:[EMAIL PROTECTED]" target="_blank" >[EMAIL PROTECTED]">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="" href="mailto:[EMAIL PROTECTED]" target="_blank" >[EMAIL PROTECTED]">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="" href="mailto:[EMAIL PROTECTED]" target="_blank" >[EMAIL PROTECTED]">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();
}
- [PATCH] Found correctness bugs in implementation of Batik... Randy Jeune
- Re: [PATCH] Found correctness bugs in implementation... thomas . deweese
