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();
}