Address FindBugs errors (THRIFT-2239)

lib/java/src/org/apache/thrift/TBaseHelper.java

@@ -17,6 +17,7 @@
  */
 package org.apache.thrift;
 
+import java.io.Serializable;
 import java.nio.ByteBuffer;
 import java.util.Comparator;
 import java.util.Iterator;
@@ -198,7 +199,7 @@ public static int compareTo(Map a, Map b) {
   /**
    * Comparator to compare items inside a structure (e.g. a list, set, or map).
    */
-  private static class NestedStructureComparator implements Comparator {
+  private static class NestedStructureComparator implements Comparator, Serializable {
     public int compare(Object oA, Object oB) {
       if (oA == null && oB == null) {
         return 0;

lib/java/src/org/apache/thrift/TMultiplexedProcessor.java

@@ -128,7 +128,7 @@ Use the actual underlying protocol (e.g. TBinaryProtocol) to read the
      *  to allow them to call readMessageBegin() and get a TMessage in exactly
      *  the standard format, without the service name prepended to TMessage.name.
      */
-    private class StoredMessageProtocol extends TProtocolDecorator {
+    private static class StoredMessageProtocol extends TProtocolDecorator {
         TMessage messageBegin;
         public StoredMessageProtocol(TProtocol protocol, TMessage messageBegin) {
             super(protocol);

lib/java/src/org/apache/thrift/async/TAsyncClientManager.java

@@ -19,6 +19,7 @@
 package org.apache.thrift.async;
 
 import java.io.IOException;
+import java.io.Serializable;
 import java.nio.channels.ClosedSelectorException;
 import java.nio.channels.SelectionKey;
 import java.nio.channels.Selector;
@@ -182,7 +183,7 @@ private void startPendingMethods() {
   }
 
   /** Comparator used in TreeSet */
-  private static class TAsyncMethodCallTimeoutComparator implements Comparator<TAsyncMethodCall> {
+  private static class TAsyncMethodCallTimeoutComparator implements Comparator<TAsyncMethodCall>, Serializable {
     public int compare(TAsyncMethodCall left, TAsyncMethodCall right) {
       if (left.getTimeoutTimestamp() == right.getTimeoutTimestamp()) {
         return (int)(left.getSequenceId() - right.getSequenceId());

lib/java/src/org/apache/thrift/protocol/TField.java

@@ -19,6 +19,8 @@
 
 package org.apache.thrift.protocol;
 
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+
 /**
  * Helper class that encapsulates field metadata.
  *
@@ -42,7 +44,23 @@ public String toString() {
     return "<TField name:'" + name + "' type:" + type + " field-id:" + id + ">";
   }
 
-  public boolean equals(TField otherField) {
+  @Override
+  public int hashCode() {
+    return new HashCodeBuilder()
+      .append(name)
+      .append(type)
+      .append(id)
+      .toHashCode();
+  }
+
+  @Override
+  public boolean equals(Object object) {
+    if (this == object) {
+      return true;
+    } else if (!(object instanceof TField)) {
+      return false;
+    }
+    TField otherField = (TField) object;
     return type == otherField.type && id == otherField.id;
   }
 }

lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java

@@ -421,6 +421,8 @@ private void writeJSONDouble(double num) throws TException {
         special = true;
       }
       break;
+    default:
+      break;
     }
 
     boolean escapeNum = special || context_.escapeNum();

lib/java/src/org/apache/thrift/protocol/TMessage.java

@@ -19,6 +19,8 @@
 
 package org.apache.thrift.protocol;
 
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+
 /**
  * Helper class that encapsulates struct metadata.
  *
@@ -44,14 +46,22 @@ public String toString() {
   }
 
   @Override
-  public boolean equals(Object other) {
-    if (other instanceof TMessage) {
-      return equals((TMessage) other);
-    }
-    return false;
+  public int hashCode() {
+    return new HashCodeBuilder()
+      .append(name)
+      .append(type)
+      .append(seqid)
+      .toHashCode();
   }
 
-  public boolean equals(TMessage other) {
+  @Override
+  public boolean equals(Object object) {
+    if (this == object) {
+      return true;
+    } else if (!(object instanceof TMessage)) {
+      return false;
+    }
+    TMessage other = (TMessage) object;
     return name.equals(other.name) && type == other.type && seqid == other.seqid;
   }
 }

lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java

@@ -45,13 +45,13 @@ public TProtocol getProtocol(TTransport trans) {
     }
   }
 
-  public static final byte[] COMMA = new byte[] {','};
-  public static final byte[] COLON = new byte[] {':'};
-  public static final byte[] LBRACE = new byte[] {'{'};
-  public static final byte[] RBRACE = new byte[] {'}'};
-  public static final byte[] LBRACKET = new byte[] {'['};
-  public static final byte[] RBRACKET = new byte[] {']'};
-  public static final char QUOTE = '"';
+  private static final byte[] COMMA = new byte[] {','};
+  private static final byte[] COLON = new byte[] {':'};
+  private static final byte[] LBRACE = new byte[] {'{'};
+  private static final byte[] RBRACE = new byte[] {'}'};
+  private static final byte[] LBRACKET = new byte[] {'['};
+  private static final byte[] RBRACKET = new byte[] {']'};
+  private static final char QUOTE = '"';
 
   private static final TStruct ANONYMOUS_STRUCT = new TStruct();
   private static final TField ANONYMOUS_FIELD = new TField();

lib/java/src/org/apache/thrift/server/TThreadedSelectorServer.java

@@ -633,7 +633,7 @@ protected SelectorThreadLoadBalancer createSelectorThreadLoadBalancer(Collection
    * A round robin load balancer for choosing selector threads for new
    * connections.
    */
-  protected class SelectorThreadLoadBalancer {
+  protected static class SelectorThreadLoadBalancer {
     private final Collection<? extends SelectorThread> threads;
     private Iterator<? extends SelectorThread> nextThreadIterator;
 

lib/java/src/org/apache/thrift/transport/TFileTransport.java

@@ -38,14 +38,14 @@
  */
 public class TFileTransport extends TTransport {
 
-  public static class truncableBufferedInputStream extends BufferedInputStream {
+  public static class TruncableBufferedInputStream extends BufferedInputStream {
     public void trunc() {
       pos = count = 0;
     }        
-    public truncableBufferedInputStream(InputStream in) {
+    public TruncableBufferedInputStream(InputStream in) {
       super(in);
     }
-    public truncableBufferedInputStream(InputStream in, int size) {
+    public TruncableBufferedInputStream(InputStream in, int size) {
       super(in, size);
     }
   }
@@ -87,7 +87,7 @@ public int emit(byte[] buf, int offset, int ndesired) {
     }
   };
 
-  public static class chunkState {
+  public static class ChunkState {
     /**
      * Chunk Size. Must be same across all implementations
      */
@@ -96,8 +96,8 @@ public static class chunkState {
     private int chunk_size_ = DEFAULT_CHUNK_SIZE;
     private long offset_ = 0;
 
-    public chunkState() {}
-    public chunkState(int chunk_size) { chunk_size_ = chunk_size; }
+    public ChunkState() {}
+    public ChunkState(int chunk_size) { chunk_size_ = chunk_size; }
 
     public void skip(int size) {offset_ += size; }
     public void seek(long offset) {offset_ = offset;}
@@ -108,7 +108,7 @@ public chunkState() {}
     public long getOffset() { return (offset_);}
   }
 
-  public static enum tailPolicy {
+  public static enum TailPolicy {
 
     NOWAIT(0, 0),
       WAIT_FOREVER(500, -1);
@@ -133,7 +133,7 @@ public static enum tailPolicy {
      * @param retries number of retries
      */
 
-    tailPolicy(int timeout, int retries) {
+    TailPolicy(int timeout, int retries) {
       timeout_ = timeout;
       retries_ = retries;
     }
@@ -142,7 +142,7 @@ public static enum tailPolicy {
   /**
    * Current tailing policy
    */
-  tailPolicy currentPolicy_ = tailPolicy.NOWAIT;
+  TailPolicy currentPolicy_ = TailPolicy.NOWAIT;
 
 
   /** 
@@ -169,12 +169,7 @@ public static enum tailPolicy {
   /**
    * current Chunk state
    */
-  chunkState cs = null;
-
-  /**
-   * Read timeout
-   */
-  private int readTimeout_ = 0;
+  ChunkState cs = null;
 
   /**
    * is read only?
@@ -186,7 +181,7 @@ public static enum tailPolicy {
    * 
    * @return current read policy
    */
-  public tailPolicy getTailPolicy() {
+  public TailPolicy getTailPolicy() {
     return (currentPolicy_);
   }
 
@@ -196,8 +191,8 @@ public tailPolicy getTailPolicy() {
    * @param policy New policy to set
    * @return Old policy
    */
-  public tailPolicy setTailPolicy(tailPolicy policy) {
-    tailPolicy old = currentPolicy_;
+  public TailPolicy setTailPolicy(TailPolicy policy) {
+    TailPolicy old = currentPolicy_;
     currentPolicy_ = policy;
     return (old);
   }
@@ -212,10 +207,10 @@ private InputStream createInputStream() throws TTransportException {
     InputStream is;
     try {
       if(inputStream_ != null) {
-        ((truncableBufferedInputStream)inputStream_).trunc();
+        ((TruncableBufferedInputStream)inputStream_).trunc();
         is = inputStream_;
       } else {
-        is = new truncableBufferedInputStream(inputFile_.getInputStream());
+        is = new TruncableBufferedInputStream(inputFile_.getInputStream());
       }
     } catch (IOException iox) {
       System.err.println("createInputStream: "+iox.getMessage());
@@ -236,7 +231,7 @@ private InputStream createInputStream() throws TTransportException {
    * @return number of bytes read
    */
   private int tailRead(InputStream is, byte[] buf, 
-                       int off, int len, tailPolicy tp) throws TTransportException {
+                       int off, int len, TailPolicy tp) throws TTransportException {
     int orig_len = len;
     try {
       int retries = 0;
@@ -369,7 +364,7 @@ public void open() throws TTransportException {
 
     try {
       inputStream_ = createInputStream();
-      cs = new chunkState();
+      cs = new ChunkState();
       currentEvent_ = new Event(new byte [256]);
 
       if(!readOnly_)
@@ -545,7 +540,7 @@ public void seekToChunk(int chunk) throws TTransportException {
     if(seekToEnd) {
       // waiting forever here - otherwise we can hit EOF and end up
       // having consumed partial data from the data stream.
-      tailPolicy old = setTailPolicy(tailPolicy.WAIT_FOREVER);
+      TailPolicy old = setTailPolicy(TailPolicy.WAIT_FOREVER);
       while(cs.getOffset() < eofOffset) { readEvent(); }
       currentEvent_.setAvailable(0);
       setTailPolicy(old);

lib/java/src/org/apache/thrift/transport/TMemoryBuffer.java

@@ -77,12 +77,14 @@ public String toString(String enc) throws UnsupportedEncodingException {
   }
 
   public String inspect() {
-    String buf = "";
+    StringBuilder buf = new StringBuilder();
     byte[] bytes = arr_.toByteArray();
     for (int i = 0; i < bytes.length; i++) {
-      buf += (pos_ == i ? "==>" : "" ) + Integer.toHexString(bytes[i] & 0xff) + " ";
+      buf.append(pos_ == i ? "==>" : "")
+        .append(Integer.toHexString(bytes[i] & 0xff))
+        .append(" ");
     }
-    return buf;
+    return buf.toString();
   }
 
   // The contents of the buffer

lib/java/src/org/apache/thrift/transport/TSSLTransportFactory.java

@@ -22,6 +22,7 @@
 import java.io.FileInputStream;
 import java.net.InetAddress;
 import java.security.KeyStore;
+import java.util.Arrays;
 
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
@@ -176,14 +177,24 @@ private static SSLContext createSSLContext(TSSLTransportParameters params) throw
       if (params.isTrustStoreSet) {
         tmf = TrustManagerFactory.getInstance(params.trustManagerType);
         KeyStore ts = KeyStore.getInstance(params.trustStoreType);
-        ts.load(new FileInputStream(params.trustStore), params.trustPass.toCharArray());
+        FileInputStream fis = new FileInputStream(params.trustStore);
+        try {
+          ts.load(fis, params.trustPass.toCharArray());
+        } finally {
+          fis.close();
+        }
         tmf.init(ts);
       }
 
       if (params.isKeyStoreSet) {
         kmf = KeyManagerFactory.getInstance(params.keyManagerType);
         KeyStore ks = KeyStore.getInstance(params.keyStoreType);
-        ks.load(new FileInputStream(params.keyStore), params.keyPass.toCharArray());
+        FileInputStream fis = new FileInputStream(params.keyStore);
+        try {
+          ks.load(fis, params.keyPass.toCharArray());
+        } finally {
+          fis.close();
+        }
         kmf.init(ks, params.keyPass.toCharArray());
       }
 
@@ -256,7 +267,7 @@ public TSSLTransportParameters(String protocol, String[] cipherSuites, boolean c
       if (protocol != null) {
         this.protocol = protocol;
       }
-      this.cipherSuites = cipherSuites;
+      this.cipherSuites = Arrays.copyOf(cipherSuites, cipherSuites.length);
       this.clientAuth = clientAuth;
     }
 

lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java

@@ -126,7 +126,7 @@ protected void handleSaslStartMessage() throws TTransportException, SaslExceptio
 
     LOGGER.debug("Received start message with status {}", message.status);
     if (message.status != NegotiationStatus.START) {
-      sendAndThrowMessage(NegotiationStatus.ERROR, "Expecting START status, received " + message.status);
+      throw sendAndThrowMessage(NegotiationStatus.ERROR, "Expecting START status, received " + message.status);
     }
 
     // Get the mechanism name.
@@ -135,7 +135,7 @@ protected void handleSaslStartMessage() throws TTransportException, SaslExceptio
     LOGGER.debug("Received mechanism name '{}'", mechanismName);
 
     if (serverDefinition == null) {
-      sendAndThrowMessage(NegotiationStatus.BAD, "Unsupported mechanism type " + mechanismName);
+      throw sendAndThrowMessage(NegotiationStatus.BAD, "Unsupported mechanism type " + mechanismName);
     }
     SaslServer saslServer = Sasl.createSaslServer(serverDefinition.mechanism,
         serverDefinition.protocol, serverDefinition.serverName, serverDefinition.props,