Fix findbugs issues

And configuration for xml output
jenkinsci · Jul 7, 2015 · 67ca117 · 67ca117
1 parent dd0420d
commit 67ca117
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 4 deletions.
diff --git a/findbugs-exclude.xml b/findbugs-exclude.xml
@@ -0,0 +1,5 @@
+<FindBugsFilter>
+     <Match>
+       <Class name="~.+\.Messages" />
+     </Match>
+</FindBugsFilter>
diff --git a/pom.xml b/pom.xml
@@ -64,8 +64,29 @@
       <version>${powermock.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>annotations</artifactId>
+      <version>3.0.0</version>
+    </dependency>
   </dependencies>
 
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.codehaus.mojo</groupId>
+        <artifactId>findbugs-maven-plugin</artifactId>
+        <version>3.0.1</version>
+        <configuration>
+          <excludeFilterFile>findbugs-exclude.xml</excludeFilterFile>
+          <xmlOutput>true</xmlOutput>
+          <findbugsXmlWithMessages>true</findbugsXmlWithMessages>
+          <failOnError>false</failOnError>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
   <repositories>
     <repository>
       <id>repo.jenkins-ci.org</id>

diff --git a/...n/java/hudson/plugins/active_directory/AbstractActiveDirectoryAuthenticationProvider.java b/...n/java/hudson/plugins/active_directory/AbstractActiveDirectoryAuthenticationProvider.java
@@ -23,6 +23,7 @@
  */
 package hudson.plugins.active_directory;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
 import org.acegisecurity.providers.dao.AbstractUserDetailsAuthenticationProvider;
@@ -61,5 +62,6 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
     /**
      * Setting this to true might help with diagnosing login problem.
      */
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "It should, but changing would break backwards compatibility.")
     public static boolean SHOW_USER_NOT_FOUND_EXCEPTION = Boolean.getBoolean(AbstractActiveDirectoryAuthenticationProvider.class.getName()+".showUserNotFoundException");
 }
diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java
@@ -25,6 +25,7 @@
 
 import com.sun.jndi.ldap.LdapCtxFactory;
 import com4j.typelibs.ado20.ClassFactory;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import groovy.lang.Binding;
 import hudson.Extension;
 import hudson.Functions;
@@ -48,6 +49,7 @@
 import org.acegisecurity.userdetails.UserDetails;
 import org.acegisecurity.userdetails.UserDetailsService;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
+import org.apache.commons.io.IOUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
@@ -70,6 +72,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayList;
@@ -183,10 +186,16 @@ public SecurityComponents createSecurityComponents() {
         BeanBuilder builder = new BeanBuilder(getClass().getClassLoader());
         Binding binding = new Binding();
         binding.setVariable("realm", this);
-        builder.parse(getClass().getResourceAsStream("ActiveDirectory.groovy"), binding);
+        InputStream i = getClass().getResourceAsStream("ActiveDirectory.groovy");
+        try {
+            builder.parse(i, binding);
+        } finally {
+            IOUtils.closeQuietly(i);
+        }
         WebApplicationContext context = builder.createApplicationContext();
 
-        final AbstractActiveDirectoryAuthenticationProvider adp = findBean(AbstractActiveDirectoryAuthenticationProvider.class, context);
+        //final AbstractActiveDirectoryAuthenticationProvider adp = findBean(AbstractActiveDirectoryAuthenticationProvider.class, context);
+        findBean(AbstractActiveDirectoryAuthenticationProvider.class, context); //Keeping the call because there might be side effects?
         final UserDetailsService uds = findBean(UserDetailsService.class, context);
 
         TokenBasedRememberMeServices2 rms = new TokenBasedRememberMeServices2() {
@@ -539,7 +548,7 @@ public List<SocketInfo> obtainLDAPServer(String domainName, String site, String
 
         // domain name prefixes
         // see http://technet.microsoft.com/en-us/library/cc759550(WS.10).aspx
-        private static final List<String> CANDIDATES = Arrays.asList("_gc._tcp.","_ldap._tcp.");
+        private static final List<String> CANDIDATES = Arrays.asList("_gc._tcp.", "_ldap._tcp.");
 
         /**
          * Use DNS and obtains the LDAP servers that we should try.
@@ -600,7 +609,25 @@ class PrioritizedSocketInfo implements Comparable<PrioritizedSocketInfo> {
                     }
 
                     public int compareTo(PrioritizedSocketInfo that) {
-                        return that.priority-this.priority; // sort them so that bigger priority comes first
+                        int prio = that.priority - this.priority; // sort them so that bigger priority comes first
+                        if (prio != 0) {
+                            return prio;
+                        }
+                        String s1 = socket != null ? socket.toString() : "";
+                        String s2 = that.socket != null ? that.socket.toString() : "";
+                        return s1.compareTo(s2);
+                    }
+
+                    @Override
+                    public boolean equals(Object obj) {
+                        return obj instanceof PrioritizedSocketInfo && compareTo((PrioritizedSocketInfo)obj) == 0;
+                    }
+
+                    @Override
+                    public int hashCode() {
+                        int result = socket != null ? socket.hashCode() : 0;
+                        result = 31 * result + priority;
+                        return result;
                     }
                 }
                 List<PrioritizedSocketInfo> plist = new ArrayList<PrioritizedSocketInfo>();
@@ -676,6 +703,7 @@ protected UserDetails authenticate(String username, String password) throws Auth
      * @deprecated as of 1.28
      *      Use the UI field.
      */
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "It should, but changing would break backwards compatibility.")
     public static String DOMAIN_CONTROLLERS = System.getProperty(ActiveDirectorySecurityRealm.class.getName()+".domainControllers");
 
     /**
@@ -686,5 +714,6 @@ protected UserDetails authenticate(String username, String password) throws Auth
      * One legitimate use case is when the domain controller is Windows 2000, which doesn't support TLS
      * (according to http://support.microsoft.com/kb/321051).
      */
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "It should, but changing would break backwards compatibility.")
     public static boolean FORCE_LDAPS = Boolean.getBoolean(ActiveDirectorySecurityRealm.class.getName()+".forceLdaps");
 }
diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java
@@ -23,6 +23,7 @@
  */
 package hudson.plugins.active_directory;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Util;
 import hudson.security.GroupDetails;
 import hudson.security.SecurityRealm;
@@ -245,6 +246,7 @@ private List<SocketInfo> obtainLDAPServers(String domainName) throws Authenticat
      *      The user didn't exist.
      * @return never null
      */
+    @SuppressFBWarnings(value = "ES_COMPARING_PARAMETER_STRING_WITH_EQ", justification = "Intentional instance check.")
     public UserDetails retrieveUser(String username, String password, String domainName, List<SocketInfo> ldapServers) {
         DirContext context;
         boolean anonymousBind;    // did we bind anonymously?

diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUserDetail.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUserDetail.java
@@ -84,6 +84,37 @@ public String toString() {
         return toStringValue;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof ActiveDirectoryUserDetail)) return false;
+        if (!super.equals(o)) return false;
+
+        ActiveDirectoryUserDetail that = (ActiveDirectoryUserDetail)o;
+
+        if (displayName != null ? !displayName.equals(that.displayName) : that.displayName != null) {
+            return false;
+        }
+        if (mail != null ? !mail.equals(that.mail) : that.mail != null) {
+            return false;
+        }
+        if (telephoneNumber != null ? !telephoneNumber.equals(that.telephoneNumber) : that.telephoneNumber != null) {
+            return false;
+        }
+        return !(toStringValue != null ? !toStringValue.equals(that.toStringValue) : that.toStringValue != null);
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = super.hashCode();
+        result = 31 * result + (displayName != null ? displayName.hashCode() : 0);
+        result = 31 * result + (mail != null ? mail.hashCode() : 0);
+        result = 31 * result + (telephoneNumber != null ? telephoneNumber.hashCode() : 0);
+        result = 31 * result + (toStringValue != null ? toStringValue.hashCode() : 0);
+        return result;
+    }
+
     @Override
     protected void setAuthorities(GrantedAuthority[] authorities) {
         SecurityRealm realm = Jenkins.getInstance().getSecurityRealm();

diff --git a/src/main/java/hudson/plugins/active_directory/MultiCauseBadCredentialsException.java b/src/main/java/hudson/plugins/active_directory/MultiCauseBadCredentialsException.java
@@ -24,6 +24,7 @@
 package hudson.plugins.active_directory;
 
 import com.google.common.collect.ImmutableList;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.util.FlushProofOutputStream;
 import org.acegisecurity.AuthenticationException;
 
@@ -46,6 +47,7 @@ public MultiCauseBadCredentialsException(String msg, Collection<? extends Throwa
     }
 
     @Override
+    @SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "It seems that encoding selection is not supported.")
     public void printStackTrace(PrintStream s) {
         PrintWriter w = new PrintWriter(new FlushProofOutputStream(s));
         printStackTrace(w);

diff --git a/src/main/java/hudson/plugins/active_directory/MultiCauseUserMayOrMayNotExistException.java b/src/main/java/hudson/plugins/active_directory/MultiCauseUserMayOrMayNotExistException.java
@@ -24,6 +24,7 @@
 package hudson.plugins.active_directory;
 
 import com.google.common.collect.ImmutableList;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.security.UserMayOrMayNotExistException;
 import hudson.util.FlushProofOutputStream;
 
@@ -44,6 +45,7 @@ public MultiCauseUserMayOrMayNotExistException(String msg, Collection<? extends
     }
 
     @Override
+    @SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "It seems that encoding selection is not supported.")
     public void printStackTrace(PrintStream s) {
         PrintWriter w = new PrintWriter(new FlushProofOutputStream(s));
         printStackTrace(w);

diff --git a/src/main/java/hudson/plugins/active_directory/MultiCauseUserNotFoundException.java b/src/main/java/hudson/plugins/active_directory/MultiCauseUserNotFoundException.java
@@ -24,6 +24,7 @@
 package hudson.plugins.active_directory;
 
 import com.google.common.collect.ImmutableList;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.util.FlushProofOutputStream;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
 
@@ -46,6 +47,7 @@ public MultiCauseUserNotFoundException(String msg, Collection<? extends Throwabl
     }
 
     @Override
+    @SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "It seems that encoding selection is not supported.")
     public void printStackTrace(PrintStream s) {
         PrintWriter w = new PrintWriter(new FlushProofOutputStream(s));
         printStackTrace(w);