THRIFT-2239 Address FindBugs errors

Client: Java
Patch: Liang Xie
diff --git a/lib/java/src/org/apache/thrift/TBaseHelper.java b/lib/java/src/org/apache/thrift/TBaseHelper.java
index eec648b..4f02b06 100644
--- a/lib/java/src/org/apache/thrift/TBaseHelper.java
+++ b/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 @@
   /**
    * 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;
diff --git a/lib/java/src/org/apache/thrift/TMultiplexedProcessor.java b/lib/java/src/org/apache/thrift/TMultiplexedProcessor.java
index dad6155..0bf4919 100644
--- a/lib/java/src/org/apache/thrift/TMultiplexedProcessor.java
+++ b/lib/java/src/org/apache/thrift/TMultiplexedProcessor.java
@@ -128,7 +128,7 @@
      *  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);
diff --git a/lib/java/src/org/apache/thrift/async/TAsyncClientManager.java b/lib/java/src/org/apache/thrift/async/TAsyncClientManager.java
index 98f7194..6fcf4b4 100644
--- a/lib/java/src/org/apache/thrift/async/TAsyncClientManager.java
+++ b/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 @@
   }
 
   /** 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());
diff --git a/lib/java/src/org/apache/thrift/protocol/TField.java b/lib/java/src/org/apache/thrift/protocol/TField.java
index 03affda..31331bb 100644
--- a/lib/java/src/org/apache/thrift/protocol/TField.java
+++ b/lib/java/src/org/apache/thrift/protocol/TField.java
@@ -42,7 +42,25 @@
     return "<TField name:'" + name + "' type:" + type + " field-id:" + id + ">";
   }
 
-  public boolean equals(TField otherField) {
+  @Override
+  public int hashCode() {
+    final int prime = 31;
+    int result = 1;
+    result = prime * result + id;
+    result = prime * result + ((name == null) ? 0 : name.hashCode());
+    result = prime * result + type;
+    return result;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj)
+      return true;
+    if (obj == null)
+      return false;
+    if (getClass() != obj.getClass())
+      return false;
+    TField otherField = (TField) obj;
     return type == otherField.type && id == otherField.id;
   }
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
index 6ce702e..f51322c 100644
--- a/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
@@ -436,7 +436,9 @@
         special = true;
       }
       break;
-    }
+    default:
+      break;
+  }
 
     boolean escapeNum = special || context_.escapeNum();
     if (escapeNum) {
diff --git a/lib/java/src/org/apache/thrift/protocol/TMessage.java b/lib/java/src/org/apache/thrift/protocol/TMessage.java
index 1438b11..f13b8ca 100644
--- a/lib/java/src/org/apache/thrift/protocol/TMessage.java
+++ b/lib/java/src/org/apache/thrift/protocol/TMessage.java
@@ -44,14 +44,33 @@
   }
 
   @Override
-  public boolean equals(Object other) {
-    if (other instanceof TMessage) {
-      return equals((TMessage) other);
-    }
-    return false;
+  public int hashCode() {
+    final int prime = 31;
+    int result = 1;
+    result = prime * result + ((name == null) ? 0 : name.hashCode());
+    result = prime * result + seqid;
+    result = prime * result + type;
+    return result;
   }
 
-  public boolean equals(TMessage other) {
-    return name.equals(other.name) && type == other.type && seqid == other.seqid;
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj)
+      return true;
+    if (obj == null)
+      return false;
+    if (getClass() != obj.getClass())
+      return false;
+    TMessage other = (TMessage) obj;
+    if (name == null) {
+      if (other.name != null)
+        return false;
+    } else if (!name.equals(other.name))
+      return false;
+    if (seqid != other.seqid)
+      return false;
+    if (type != other.type)
+      return false;
+    return true;
   }
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
index e005a4f..ffa139a 100644
--- a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
@@ -45,13 +45,13 @@
     }
   }
 
-  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();
diff --git a/lib/java/src/org/apache/thrift/server/TThreadedSelectorServer.java b/lib/java/src/org/apache/thrift/server/TThreadedSelectorServer.java
index 7c5ec4f..16445e5 100644
--- a/lib/java/src/org/apache/thrift/server/TThreadedSelectorServer.java
+++ b/lib/java/src/org/apache/thrift/server/TThreadedSelectorServer.java
@@ -643,7 +643,7 @@
    * 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;
 
diff --git a/lib/java/src/org/apache/thrift/transport/TFileTransport.java b/lib/java/src/org/apache/thrift/transport/TFileTransport.java
index f5abe53..0044d2a 100644
--- a/lib/java/src/org/apache/thrift/transport/TFileTransport.java
+++ b/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 static class chunkState {
+  public static class ChunkState {
     /**
      * Chunk Size. Must be same across all implementations
      */
@@ -96,8 +96,8 @@
     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 long getOffset() { return (offset_);}
   }
 
-  public static enum tailPolicy {
+  public static enum TailPolicy {
 
     NOWAIT(0, 0),
       WAIT_FOREVER(500, -1);
@@ -133,7 +133,7 @@
      * @param retries number of retries
      */
 
-    tailPolicy(int timeout, int retries) {
+    TailPolicy(int timeout, int retries) {
       timeout_ = timeout;
       retries_ = retries;
     }
@@ -142,7 +142,7 @@
   /**
    * Current tailing policy
    */
-  tailPolicy currentPolicy_ = tailPolicy.NOWAIT;
+  TailPolicy currentPolicy_ = TailPolicy.NOWAIT;
 
 
   /** 
@@ -169,12 +169,7 @@
   /**
    * current Chunk state
    */
-  chunkState cs = null;
-
-  /**
-   * Read timeout
-   */
-  private int readTimeout_ = 0;
+  ChunkState cs = null;
 
   /**
    * is read only?
@@ -186,7 +181,7 @@
    * 
    * @return current read policy
    */
-  public tailPolicy getTailPolicy() {
+  public TailPolicy getTailPolicy() {
     return (currentPolicy_);
   }
 
@@ -196,8 +191,8 @@
    * @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 @@
     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 @@
    * @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 @@
 
     try {
       inputStream_ = createInputStream();
-      cs = new chunkState();
+      cs = new ChunkState();
       currentEvent_ = new Event(new byte [256]);
 
       if(!readOnly_)
@@ -545,7 +540,7 @@
     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);
diff --git a/lib/java/src/org/apache/thrift/transport/TMemoryBuffer.java b/lib/java/src/org/apache/thrift/transport/TMemoryBuffer.java
index 53354af..ef5f5c2 100644
--- a/lib/java/src/org/apache/thrift/transport/TMemoryBuffer.java
+++ b/lib/java/src/org/apache/thrift/transport/TMemoryBuffer.java
@@ -77,12 +77,12 @@
   }
 
   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
diff --git a/lib/java/src/org/apache/thrift/transport/TSSLTransportFactory.java b/lib/java/src/org/apache/thrift/transport/TSSLTransportFactory.java
index ca27729..16d2fd1 100755
--- a/lib/java/src/org/apache/thrift/transport/TSSLTransportFactory.java
+++ b/lib/java/src/org/apache/thrift/transport/TSSLTransportFactory.java
@@ -23,6 +23,7 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.security.KeyStore;
+import java.util.Arrays;
 
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
@@ -171,6 +172,7 @@
   private static SSLContext createSSLContext(TSSLTransportParameters params) throws TTransportException {
     SSLContext ctx;
     FileInputStream fin = null;
+    FileInputStream fis = null;
 
     try {
       ctx = SSLContext.getInstance(params.protocol);
@@ -188,8 +190,8 @@
       if (params.isKeyStoreSet) {
         kmf = KeyManagerFactory.getInstance(params.keyManagerType);
         KeyStore ks = KeyStore.getInstance(params.keyStoreType);
-        fin = new FileInputStream(params.keyStore);
-        ks.load(fin, params.keyPass.toCharArray());
+        fis = new FileInputStream(params.keyStore);
+        ks.load(fis, params.keyPass.toCharArray());
         kmf.init(ks, params.keyPass.toCharArray());
       }
 
@@ -213,6 +215,13 @@
           e.printStackTrace();
         }
       }
+      if (fis != null) {
+        try {
+          fis.close();
+        } catch (IOException e) {
+          e.printStackTrace();
+        }
+      }
     }
 
     return ctx;
@@ -271,7 +280,7 @@
       if (protocol != null) {
         this.protocol = protocol;
       }
-      this.cipherSuites = cipherSuites;
+      this.cipherSuites = Arrays.copyOf(cipherSuites, cipherSuites.length);
       this.clientAuth = clientAuth;
     }
 
diff --git a/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java b/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
index 1a74d53..c883c8f 100644
--- a/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
+++ b/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
@@ -126,7 +126,7 @@
 
     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 @@
     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,
diff --git a/lib/java/src/org/apache/thrift/transport/TSaslTransport.java b/lib/java/src/org/apache/thrift/transport/TSaslTransport.java
index cb15e83..947ee13 100644
--- a/lib/java/src/org/apache/thrift/transport/TSaslTransport.java
+++ b/lib/java/src/org/apache/thrift/transport/TSaslTransport.java
@@ -183,7 +183,7 @@
 
     NegotiationStatus status = NegotiationStatus.byValue(statusByte);
     if (status == null) {
-      sendAndThrowMessage(NegotiationStatus.ERROR, "Invalid status " + statusByte);
+      throw sendAndThrowMessage(NegotiationStatus.ERROR, "Invalid status " + statusByte);
     } else if (status == NegotiationStatus.BAD || status == NegotiationStatus.ERROR) {
       try {
         String remoteMessage = new String(payload, "UTF-8");
@@ -210,8 +210,10 @@
    *          The optional message to send to the other side.
    * @throws TTransportException
    *           Always thrown with the message provided.
+   * @return always throws TTransportException but declares return type to allow
+   *          throw sendAndThrowMessage(...) to inform compiler control flow
    */
-  protected void sendAndThrowMessage(NegotiationStatus status, String message) throws TTransportException {
+  protected TTransportException sendAndThrowMessage(NegotiationStatus status, String message) throws TTransportException {
     try {
       sendSaslMessage(status, message.getBytes());
     } catch (Exception e) {
@@ -302,7 +304,7 @@
     } catch (SaslException e) {
       try {
         LOGGER.error("SASL negotiation failure", e);
-        sendAndThrowMessage(NegotiationStatus.BAD, e.getMessage());
+        throw sendAndThrowMessage(NegotiationStatus.BAD, e.getMessage());
       } finally {
         underlyingTransport.close();
       }