From e1f9674ab3f5daec3b500fc2614be1d9a2a9bfe2 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 10:31:23 +0100 Subject: [PATCH 1/3] false positives for null dereferences --- src/main/org/apache/tools/ant/UnknownElement.java | 3 ++- .../org/apache/tools/ant/taskdefs/MacroInstance.java | 12 +++++++++--- src/main/org/apache/tools/zip/ZipFile.java | 6 +++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/org/apache/tools/ant/UnknownElement.java b/src/main/org/apache/tools/ant/UnknownElement.java index 88cd4989d..15770c2f8 100644 --- a/src/main/org/apache/tools/ant/UnknownElement.java +++ b/src/main/org/apache/tools/ant/UnknownElement.java @@ -646,7 +646,8 @@ public class UnknownElement extends Task { return false; } for (int i = 0; i < childrenSize; ++i) { - UnknownElement child = (UnknownElement) children.get(i); + // children cannot be null childrenSize would have been 0 + UnknownElement child = (UnknownElement) children.get(i); //NOSONAR if (!child.similar(other.children.get(i))) { return false; } diff --git a/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java b/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java index 682ca1245..9ffa9b2e1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java +++ b/src/main/org/apache/tools/ant/taskdefs/MacroInstance.java @@ -197,9 +197,12 @@ public class MacroInstance extends Task implements DynamicAttribute, TaskContain } break; case STATE_EXPECT_NAME: + // macroName cannot be null as this state is only + // ever reached from STATE_EXPECT_BRACKET after it + // has been set if (ch == '}') { state = STATE_NORMAL; - String name = macroName.toString().toLowerCase(Locale.ENGLISH); + String name = macroName.toString().toLowerCase(Locale.ENGLISH); //NOSONAR String value = (String) macroMapping.get(name); if (value == null) { ret.append("@{"); @@ -210,7 +213,7 @@ public class MacroInstance extends Task implements DynamicAttribute, TaskContain } macroName = null; } else { - macroName.append(ch); + macroName.append(ch); //NOSONAR } break; default: @@ -224,8 +227,11 @@ public class MacroInstance extends Task implements DynamicAttribute, TaskContain ret.append('@'); break; case STATE_EXPECT_NAME: + // macroName cannot be null as this state is only + // ever reached from STATE_EXPECT_BRACKET after it + // has been set ret.append("@{"); - ret.append(macroName.toString()); + ret.append(macroName.toString()); //NOSONAR break; default: break; diff --git a/src/main/org/apache/tools/zip/ZipFile.java b/src/main/org/apache/tools/zip/ZipFile.java index 1be53cdcc..f963ceb85 100644 --- a/src/main/org/apache/tools/zip/ZipFile.java +++ b/src/main/org/apache/tools/zip/ZipFile.java @@ -1038,12 +1038,12 @@ public class ZipFile implements Closeable { @Override public boolean equals(final Object other) { if (super.equals(other)) { - // super.equals would return false if other were not an Entry + // super.equals would return false if other were null or not an Entry final Entry otherEntry = (Entry) other; return offsetEntry.headerOffset - == otherEntry.offsetEntry.headerOffset + == otherEntry.offsetEntry.headerOffset //NOSONAR && offsetEntry.dataOffset - == otherEntry.offsetEntry.dataOffset; + == otherEntry.offsetEntry.dataOffset; //NOSONAR } return false; } From 77c681c5bee6dc6bd50168356f794f3d5d42f90f Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 10:31:44 +0100 Subject: [PATCH 2/3] fix possible null dereferences --- src/main/org/apache/tools/ant/IntrospectionHelper.java | 2 ++ src/main/org/apache/tools/ant/taskdefs/Javadoc.java | 6 ++++-- src/main/org/apache/tools/ant/taskdefs/XmlProperty.java | 6 ++++-- .../apache/tools/ant/taskdefs/optional/depend/Depend.java | 2 +- .../tools/ant/taskdefs/optional/jdepend/JDependTask.java | 2 +- .../tools/ant/taskdefs/optional/sound/AntSoundPlayer.java | 4 +++- src/main/org/apache/tools/tar/TarEntry.java | 2 +- 7 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/org/apache/tools/ant/IntrospectionHelper.java b/src/main/org/apache/tools/ant/IntrospectionHelper.java index 30086563b..719f22d9d 100644 --- a/src/main/org/apache/tools/ant/IntrospectionHelper.java +++ b/src/main/org/apache/tools/ant/IntrospectionHelper.java @@ -407,6 +407,7 @@ public final class IntrospectionHelper { + " doesn't support the \"" + attributeName + "\" attribute."; throw new UnsupportedAttributeException(msg, attributeName); } + if (as != null) { // possible if value == null try { as.setObject(p, element, value); } catch (final IllegalAccessException ie) { @@ -415,6 +416,7 @@ public final class IntrospectionHelper { } catch (final InvocationTargetException ite) { throw extractBuildException(ite); } + } } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/Javadoc.java b/src/main/org/apache/tools/ant/taskdefs/Javadoc.java index f3c16ced0..b7fb41772 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Javadoc.java +++ b/src/main/org/apache/tools/ant/taskdefs/Javadoc.java @@ -1772,10 +1772,12 @@ public class Javadoc extends Task { useExternalFile, tmpList, srcListWriter); if (useExternalFile) { - srcListWriter.flush(); + srcListWriter.flush(); //NOSONAR } } catch (final IOException e) { - tmpList.delete(); + if (tmpList != null) { + tmpList.delete(); + } throw new BuildException("Error creating temporary file", e, getLocation()); } finally { diff --git a/src/main/org/apache/tools/ant/taskdefs/XmlProperty.java b/src/main/org/apache/tools/ant/taskdefs/XmlProperty.java index 2830bdf9c..ffd89d1c1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/XmlProperty.java +++ b/src/main/org/apache/tools/ant/taskdefs/XmlProperty.java @@ -384,10 +384,12 @@ public class XmlProperty extends org.apache.tools.ant.Task { if (containingPath != null && nodeName.equals(PATH)) { // A "path" attribute for a node within a Path object. containingPath.setPath(attributeValue); - } else if (container instanceof Path && nodeName.equals(REF_ID)) { + } else if (containingPath != null + && container instanceof Path && nodeName.equals(REF_ID)) { // A "refid" attribute for a node within a Path object. containingPath.setPath(attributeValue); - } else if (container instanceof Path && nodeName.equals(LOCATION)) { + } else if (containingPath != null && container instanceof Path + && nodeName.equals(LOCATION)) { // A "location" attribute for a node within a // Path object. containingPath.setLocation(resolveFile(attributeValue)); diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java b/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java index cc65129f9..2cf335cbe 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java @@ -197,7 +197,7 @@ public class Depend extends MatchingTask { dependencyList = new Vector(); className = line.substring(prependLength); dependencyMap.put(className, dependencyList); - } else { + } else if (dependencyList != null) { dependencyList.addElement(line); } } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/jdepend/JDependTask.java b/src/main/org/apache/tools/ant/taskdefs/optional/jdepend/JDependTask.java index 9d2e9ae68..c608583b1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/jdepend/JDependTask.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/jdepend/JDependTask.java @@ -553,7 +553,7 @@ public class JDependTask extends Task { } jdepend.analyze(); - if (pw.checkError()) { + if (pw != null && pw.checkError()) { throw new IOException("Encountered an error writing JDepend" + " output"); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/sound/AntSoundPlayer.java b/src/main/org/apache/tools/ant/taskdefs/optional/sound/AntSoundPlayer.java index f2ad68618..aef214fe0 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/sound/AntSoundPlayer.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/sound/AntSoundPlayer.java @@ -133,7 +133,9 @@ public class AntSoundPlayer implements LineListener, BuildListener { } else { playClip(audioClip, loops); } - audioClip.drain(); + if (audioClip != null) { + audioClip.drain(); + } } finally { if (audioClip != null) { audioClip.close(); diff --git a/src/main/org/apache/tools/tar/TarEntry.java b/src/main/org/apache/tools/tar/TarEntry.java index de15a1c71..67990797a 100644 --- a/src/main/org/apache/tools/tar/TarEntry.java +++ b/src/main/org/apache/tools/tar/TarEntry.java @@ -333,7 +333,7 @@ public class TarEntry implements TarConstants { * @return True if the entries are equal. */ public boolean equals(TarEntry it) { - return getName().equals(it.getName()); + return it != null && getName().equals(it.getName()); } /** From 85ff7bf6949f62f760e8f487a2ec6fd0152eef78 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 10:32:14 +0100 Subject: [PATCH 3/3] whitespace --- .../apache/tools/ant/IntrospectionHelper.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/org/apache/tools/ant/IntrospectionHelper.java b/src/main/org/apache/tools/ant/IntrospectionHelper.java index 719f22d9d..2b31e432f 100644 --- a/src/main/org/apache/tools/ant/IntrospectionHelper.java +++ b/src/main/org/apache/tools/ant/IntrospectionHelper.java @@ -408,14 +408,14 @@ public final class IntrospectionHelper { throw new UnsupportedAttributeException(msg, attributeName); } if (as != null) { // possible if value == null - try { - as.setObject(p, element, value); - } catch (final IllegalAccessException ie) { - // impossible as getMethods should only return public methods - throw new BuildException(ie); - } catch (final InvocationTargetException ite) { - throw extractBuildException(ite); - } + try { + as.setObject(p, element, value); + } catch (final IllegalAccessException ie) { + // impossible as getMethods should only return public methods + throw new BuildException(ie); + } catch (final InvocationTargetException ite) { + throw extractBuildException(ite); + } } }