From 8c8130d0288446f88deef393823cccffcaed474d Mon Sep 17 00:00:00 2001 From: Robert Godfrey Date: Sun, 12 Sep 2010 22:40:40 +0000 Subject: QPID-2857 : Address issues found by running FindBugs against the Java codebase git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@996393 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/configuration/PropertyNameResolver.java | 2 +- .../socket/nio/MultiThreadSocketConnector.java | 2 +- .../src/main/java/org/apache/qpid/QpidConfig.java | 4 +-- .../src/main/java/org/apache/qpid/ToyBroker.java | 28 +++++++++++++--- .../org/apache/qpid/configuration/Accessor.java | 10 +++++- .../src/main/java/org/apache/qpid/dtx/XidImpl.java | 10 ++++++ .../java/org/apache/qpid/framing/AMQTypeMap.java | 2 +- .../org/apache/qpid/framing/HeartbeatBody.java | 2 +- .../apache/qpid/framing/ProtocolInitiation.java | 14 +++++++- .../org/apache/qpid/messaging/util/Lexicon.java | 16 +++++----- .../org/apache/qpid/transport/ClientDelegate.java | 13 ++++++-- .../java/org/apache/qpid/transport/Connection.java | 2 +- .../java/org/apache/qpid/transport/Session.java | 2 +- .../apache/qpid/transport/TransportBuilder.java | 2 -- .../network/security/sasl/SASLReceiver.java | 8 ++--- .../transport/network/security/ssl/SSLSender.java | 6 ++-- .../java/org/apache/qpid/url/AMQBindingURL.java | 3 +- .../java/org/apache/qpid/url/BindingURLParser.java | 18 +++++++++-- .../main/java/org/apache/qpid/url/URLHelper.java | 6 ++-- .../org/apache/qpid/util/CommandLineParser.java | 28 +++++++++------- .../main/java/org/apache/qpid/util/FileUtils.java | 37 ++++++++++++++-------- .../org/apache/qpid/util/PrettyPrintingUtils.java | 10 +++--- 22 files changed, 155 insertions(+), 70 deletions(-) (limited to 'qpid/java/common/src') diff --git a/qpid/java/common/src/main/java/org/apache/configuration/PropertyNameResolver.java b/qpid/java/common/src/main/java/org/apache/configuration/PropertyNameResolver.java index 2c1fb0ed67..73ee747c07 100644 --- a/qpid/java/common/src/main/java/org/apache/configuration/PropertyNameResolver.java +++ b/qpid/java/common/src/main/java/org/apache/configuration/PropertyNameResolver.java @@ -99,7 +99,7 @@ public class PropertyNameResolver return properties.get(propName).get(klass); } - class QpidProperty + static class QpidProperty { private Object defValue; private String[] names; diff --git a/qpid/java/common/src/main/java/org/apache/mina/transport/socket/nio/MultiThreadSocketConnector.java b/qpid/java/common/src/main/java/org/apache/mina/transport/socket/nio/MultiThreadSocketConnector.java index b1612840db..7344f70078 100644 --- a/qpid/java/common/src/main/java/org/apache/mina/transport/socket/nio/MultiThreadSocketConnector.java +++ b/qpid/java/common/src/main/java/org/apache/mina/transport/socket/nio/MultiThreadSocketConnector.java @@ -56,7 +56,7 @@ public class MultiThreadSocketConnector extends SocketConnector private final Object lock = new Object(); private final int id = nextId++; private final String threadName = "SocketConnector-" + id; - private SocketConnectorConfig defaultConfig = new SocketConnectorConfig(); + private final Queue connectQueue = new Queue(); private final MultiThreadSocketIoProcessor[] ioProcessors; private final int processorCount; diff --git a/qpid/java/common/src/main/java/org/apache/qpid/QpidConfig.java b/qpid/java/common/src/main/java/org/apache/qpid/QpidConfig.java index 9c8019f109..b4cad44130 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/QpidConfig.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/QpidConfig.java @@ -65,7 +65,7 @@ public class QpidConfig return saslClientFactories; } - public class SecurityMechanism + public static class SecurityMechanism { String type; String handler; @@ -87,7 +87,7 @@ public class QpidConfig } } - public class SaslClientFactory + public static class SaslClientFactory { String type; String factoryClass; diff --git a/qpid/java/common/src/main/java/org/apache/qpid/ToyBroker.java b/qpid/java/common/src/main/java/org/apache/qpid/ToyBroker.java index db84b83adb..5423bbb68f 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/ToyBroker.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/ToyBroker.java @@ -107,6 +107,7 @@ class ToyBroker extends SessionDelegate { System.out.println("received headers routing_key " + props.getRoutingKey()); } + MessageProperties mp = header.get(MessageProperties.class); System.out.println("MP: " + mp); if (mp != null) @@ -114,7 +115,7 @@ class ToyBroker extends SessionDelegate System.out.println(mp.getApplicationHeaders()); } - if (exchange.route(dest,props.getRoutingKey(),xfr)) + if (exchange.route(dest,props == null ? null : props.getRoutingKey(),xfr)) { System.out.println("queued " + xfr); dispatchMessages(ssn); @@ -165,21 +166,40 @@ class ToyBroker extends SessionDelegate // ugly, but who cares :) // assumes unit is always no of messages, not bytes // assumes it's credit mode and not window - private class Consumer + private static class Consumer { long _credit; String _queueName; } + private static final class ToyBrokerSession extends Session + { + + public ToyBrokerSession(Connection connection, Binary name, long expiry, ToyExchange exchange) + { + super(connection, new ToyBroker(exchange), name, expiry); + } + } + public static final void main(String[] args) throws IOException { final ToyExchange exchange = new ToyExchange(); ConnectionDelegate delegate = new ServerDelegate() { - public SessionDelegate getSessionDelegate() + @Override + public void init(Connection conn, ProtocolHeader hdr) { - return new ToyBroker(exchange); + conn.setSessionFactory(new Connection.SessionFactory() + { + public Session newSession(Connection conn, Binary name, long expiry) + { + return new ToyBrokerSession(conn, name, expiry, exchange); + } + }); + + super.init(conn, hdr); //To change body of overridden methods use File | Settings | File Templates. } + }; MinaHandler.accept("0.0.0.0", 5672, delegate); diff --git a/qpid/java/common/src/main/java/org/apache/qpid/configuration/Accessor.java b/qpid/java/common/src/main/java/org/apache/qpid/configuration/Accessor.java index c9d386607d..dc5b69dc89 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/configuration/Accessor.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/configuration/Accessor.java @@ -152,7 +152,15 @@ public interface Accessor { super(null); Properties props = new Properties(); - props.load(new FileInputStream(fileName)); + FileInputStream inStream = new FileInputStream(fileName); + try + { + props.load(inStream); + } + finally + { + inStream.close(); + } source = props; } } diff --git a/qpid/java/common/src/main/java/org/apache/qpid/dtx/XidImpl.java b/qpid/java/common/src/main/java/org/apache/qpid/dtx/XidImpl.java index 69ccef7c29..69457ca4a9 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/dtx/XidImpl.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/dtx/XidImpl.java @@ -24,6 +24,7 @@ import org.apache.qpid.AMQInvalidArgumentException; import javax.transaction.xa.Xid; import java.io.*; +import java.util.Arrays; /** * Implements javax.transaction.dtx.Xid @@ -217,6 +218,15 @@ public class XidImpl implements Xid return false; } + @Override + public int hashCode() + { + int result = _branchQualifier != null ? Arrays.hashCode(_branchQualifier) : 0; + result = 31 * result + _formatID; + result = 31 * result + (_globalTransactionID != null ? Arrays.hashCode(_globalTransactionID) : 0); + return result; + } + //-- Static helper method /** * Convert an Xid into the AMQP String format. diff --git a/qpid/java/common/src/main/java/org/apache/qpid/framing/AMQTypeMap.java b/qpid/java/common/src/main/java/org/apache/qpid/framing/AMQTypeMap.java index a16e137466..a07fd78c8c 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/framing/AMQTypeMap.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/framing/AMQTypeMap.java @@ -25,7 +25,7 @@ import java.util.Map; public class AMQTypeMap { - public static Map _reverseTypeMap = new HashMap(); + public static final Map _reverseTypeMap = new HashMap(); static { diff --git a/qpid/java/common/src/main/java/org/apache/qpid/framing/HeartbeatBody.java b/qpid/java/common/src/main/java/org/apache/qpid/framing/HeartbeatBody.java index 15a43345b5..18ab05ffa1 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/framing/HeartbeatBody.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/framing/HeartbeatBody.java @@ -27,7 +27,7 @@ import org.apache.qpid.AMQException; public class HeartbeatBody implements AMQBody { public static final byte TYPE = 8; - public static AMQFrame FRAME = new HeartbeatBody().toFrame(); + public static final AMQFrame FRAME = new HeartbeatBody().toFrame(); public HeartbeatBody() { diff --git a/qpid/java/common/src/main/java/org/apache/qpid/framing/ProtocolInitiation.java b/qpid/java/common/src/main/java/org/apache/qpid/framing/ProtocolInitiation.java index ac21fe4243..fb3dd89717 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/framing/ProtocolInitiation.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/framing/ProtocolInitiation.java @@ -24,12 +24,13 @@ import org.apache.qpid.AMQException; import java.io.UnsupportedEncodingException; import java.nio.ByteBuffer; +import java.util.Arrays; public class ProtocolInitiation extends AMQDataBlock implements EncodableAMQDataBlock { // TODO: generate these constants automatically from the xml protocol spec file - public static final byte[] AMQP_HEADER = new byte[]{(byte)'A',(byte)'M',(byte)'Q',(byte)'P'}; + private static final byte[] AMQP_HEADER = new byte[]{(byte)'A',(byte)'M',(byte)'Q',(byte)'P'}; private static final byte CURRENT_PROTOCOL_CLASS = 1; private static final byte TCP_PROTOCOL_INSTANCE = 1; @@ -124,6 +125,17 @@ public class ProtocolInitiation extends AMQDataBlock implements EncodableAMQData _protocolMinor == pi._protocolMinor); } + @Override + public int hashCode() + { + int result = _protocolHeader != null ? Arrays.hashCode(_protocolHeader) : 0; + result = 31 * result + (int) _protocolClass; + result = 31 * result + (int) _protocolInstance; + result = 31 * result + (int) _protocolMajor; + result = 31 * result + (int) _protocolMinor; + return result; + } + public static class Decoder //implements MessageDecoder { /** diff --git a/qpid/java/common/src/main/java/org/apache/qpid/messaging/util/Lexicon.java b/qpid/java/common/src/main/java/org/apache/qpid/messaging/util/Lexicon.java index c09eb7ef84..9ab610f37a 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/messaging/util/Lexicon.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/messaging/util/Lexicon.java @@ -58,30 +58,30 @@ public class Lexicon public Lexer compile() { - String joined = ""; + StringBuilder joined = new StringBuilder(); for (Token.Type t : types) { if (joined.length() > 0) { - joined += "|"; + joined.append('|'); } - joined += "(" + t.getPattern() + ")"; + joined.append('(').append(t.getPattern()).append(')'); } - Pattern rexp = Pattern.compile(joined); + Pattern rexp = Pattern.compile(joined.toString()); return new Lexer(new ArrayList(types), eof, rexp); } public static final void main(String[] args) { - String input = ""; + StringBuilder input = new StringBuilder(); for (String a : args) { if (input.length() > 0) { - input += " "; + input.append(" "); } - input += a; + input.append(a); } Lexicon lxi = new Lexicon(); @@ -94,7 +94,7 @@ public class Lexicon lxi.eof("EOF"); Lexer lx = lxi.compile(); - for (Token t : lx.lex(input)) + for (Token t : lx.lex(input.toString())) { System.out.println(t); } diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java index d5f97f48a8..82fb57eb7d 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java @@ -52,14 +52,21 @@ public class ClientDelegate extends ConnectionDelegate private static final Logger log = Logger.get(ClientDelegate.class); private static final String KRB5_OID_STR = "1.2.840.113554.1.2.2"; - protected static Oid KRB5_OID; + protected static final Oid KRB5_OID; static { + Oid oid; try { - KRB5_OID = new Oid(KRB5_OID_STR); - } catch (GSSException ignore) {} + oid = new Oid(KRB5_OID_STR); + } + catch (GSSException ignore) + { + oid = null; + } + + KRB5_OID = oid; } private List clientMechs; diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java index 13b8e461d4..3c56aa22bd 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java @@ -61,7 +61,7 @@ public class Connection extends ConnectionInvoker public enum State { NEW, CLOSED, OPENING, OPEN, CLOSING, CLOSE_RCVD } - class DefaultConnectionListener implements ConnectionListener + static class DefaultConnectionListener implements ConnectionListener { public void opened(Connection conn) {} public void exception(Connection conn, ConnectionException exception) diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java index e989849477..52e0026f29 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java @@ -60,7 +60,7 @@ public class Session extends SessionInvoker enum State { NEW, DETACHED, RESUMING, OPEN, CLOSING, CLOSED } - class DefaultSessionListener implements SessionListener + static class DefaultSessionListener implements SessionListener { public void opened(Session ssn) {} diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/TransportBuilder.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/TransportBuilder.java index d1ae95a3bb..c08909c6e4 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/TransportBuilder.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/TransportBuilder.java @@ -61,8 +61,6 @@ public class TransportBuilder public void buildReceiverPipe(Receiver delegate) { - ConnectionSettings settings = con.getConnectionSettings(); - Receiver receiver = new InputHandler(new Assembler(delegate)); // Security layer diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/sasl/SASLReceiver.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/sasl/SASLReceiver.java index a9e8566d01..86106318ef 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/sasl/SASLReceiver.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/sasl/SASLReceiver.java @@ -41,20 +41,18 @@ public class SASLReceiver extends SASLEncryptor implements Receiver this.delegate = delegate; } - @Override public void closed() { delegate.closed(); } - @Override + public void exception(Throwable t) { - delegate.equals(t); + delegate.exception(t); } - @Override - public void received(ByteBuffer buf) + public void received(ByteBuffer buf) { if (isSecurityLayerEstablished()) { diff --git a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLSender.java b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLSender.java index 3c2ad061f3..cd47a11825 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLSender.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/ssl/SSLSender.java @@ -82,9 +82,10 @@ public class SSLSender implements Sender throw new SenderException("Error closing SSL connection",e); } - while (!engine.isOutboundDone()) + + synchronized(engineState) { - synchronized(engineState) + while (!engine.isOutboundDone()) { try { @@ -94,6 +95,7 @@ public class SSLSender implements Sender { // pass } + } } delegate.close(); diff --git a/qpid/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java b/qpid/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java index 998242925c..26cb56ea97 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/url/AMQBindingURL.java @@ -55,7 +55,8 @@ public class AMQBindingURL implements BindingURL private void parseBindingURL() throws URISyntaxException { - BindingURLParser parser = new BindingURLParser(_url,this); + BindingURLParser parser = new BindingURLParser(); + parser.parse(_url,this); processOptions(); _logger.debug("URL Parsed: " + this); } diff --git a/qpid/java/common/src/main/java/org/apache/qpid/url/BindingURLParser.java b/qpid/java/common/src/main/java/org/apache/qpid/url/BindingURLParser.java index 7fe7d2e1da..0ebfe0e869 100644 --- a/qpid/java/common/src/main/java/org/apache/qpid/url/BindingURLParser.java +++ b/qpid/java/common/src/main/java/org/apache/qpid/url/BindingURLParser.java @@ -52,15 +52,24 @@ public class BindingURLParser private String _error; private int _index = 0; private String _currentPropName; - private Map _options = new HashMap(); + private Map _options; + + + public BindingURLParser() + { + } //:///[]/[]?