SpotBugs Report

Project Information

Project: ActiveMQ :: Web

SpotBugs version: 4.8.3

Code analyzed:



Metrics

2356 lines of code analyzed, in 48 classes, in 4 packages.

Metric Total Density*
High Priority Warnings 3 1.27
Medium Priority Warnings 43 18.25
Total Warnings 46 19.52

(* Defects per Thousand lines of non-commenting source statements)



Contents

Summary

Warning Type Number
Bad practice Warnings 6
Internationalization Warnings 2
Malicious code vulnerability Warnings 22
Multithreaded correctness Warnings 4
Performance Warnings 9
Dodgy code Warnings 3
Total 46

Warnings

Click on a warning row to see full context information.

Bad practice Warnings

Code Warning
CT Exception thrown in class org.apache.activemq.web.QueueBrowseQuery at new org.apache.activemq.web.QueueBrowseQuery(BrokerFacade, SessionPool) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks.
CT Exception thrown in class org.apache.activemq.web.WebClient at new org.apache.activemq.web.WebClient() will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks.
PA Primitive field org.apache.activemq.web.WebClient.selectorName is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility.
Se org.apache.activemq.web.AjaxWebClient is Externalizable but doesn't define a void constructor
Se Class org.apache.activemq.web.MessageListenerServlet defines non-transient non-serializable instance field clientCleanupTimer
Se Class org.apache.activemq.web.MessageServlet defines non-transient non-serializable instance field activeConsumers

Internationalization Warnings

Code Warning
Dm Found reliance on default encoding in org.apache.activemq.web.MessageQuery.getBody(): new String(byte[])
Dm Found reliance on default encoding in org.apache.activemq.web.WebClient.createWebClient(HttpServletRequest): new String(byte[])

Malicious code vulnerability Warnings

Code Warning
EI org.apache.activemq.web.AjaxListener.getUndeliveredMessages() may expose internal representation by returning AjaxListener.undeliveredMessages
EI org.apache.activemq.web.AjaxWebClient.getDestinationNameMap() may expose internal representation by returning AjaxWebClient.destinationNameMap
EI org.apache.activemq.web.AjaxWebClient.getIdMap() may expose internal representation by returning AjaxWebClient.idMap
EI org.apache.activemq.web.AjaxWebClient.getListener() may expose internal representation by returning AjaxWebClient.listener
EI org.apache.activemq.web.LocalBrokerFacade.getBrokerService() may expose internal representation by returning LocalBrokerFacade.brokerService
EI org.apache.activemq.web.MessageQuery.getMessage() may expose internal representation by returning MessageQuery.message
EI org.apache.activemq.web.QueueBrowseQuery.getSession() may expose internal representation by returning QueueBrowseQuery.session
EI org.apache.activemq.web.SessionPool.getConnection() may expose internal representation by returning SessionPool.connection
EI org.apache.activemq.web.WebClient.getConnection() may expose internal representation by returning WebClient.connection
EI org.apache.activemq.web.WebClient.getProducer() may expose internal representation by returning WebClient.producer
EI org.apache.activemq.web.WebClient.getSession() may expose internal representation by returning WebClient.session
EI org.apache.activemq.web.view.RssMessageRenderer.getFeed(QueueBrowser, HttpServletRequest) may expose internal representation by returning RssMessageRenderer.feed
EI org.apache.activemq.web.view.XmlMessageRenderer.getXstream() may expose internal representation by returning XmlMessageRenderer.xstream
EI2 org.apache.activemq.web.AjaxListener.setContinuation(Continuation) may expose internal representation by storing an externally mutable object into AjaxListener.continuation
EI2 new org.apache.activemq.web.LocalBrokerFacade(BrokerService) may expose internal representation by storing an externally mutable object into LocalBrokerFacade.brokerService
EI2 org.apache.activemq.web.MessageQuery.setMessage(Message) may expose internal representation by storing an externally mutable object into MessageQuery.message
EI2 new org.apache.activemq.web.QueueBrowseQuery(BrokerFacade, SessionPool) may expose internal representation by storing an externally mutable object into QueueBrowseQuery.sessionPool
EI2 org.apache.activemq.web.SessionPool.setConnection(Connection) may expose internal representation by storing an externally mutable object into SessionPool.connection
EI2 org.apache.activemq.web.WebClient.setProducer(MessageProducer) may expose internal representation by storing an externally mutable object into WebClient.producer
EI2 new org.apache.activemq.web.config.JNDIConfiguration(InitialContext) may expose internal representation by storing an externally mutable object into JNDIConfiguration.context
EI2 org.apache.activemq.web.view.XmlMessageRenderer.setXstream(XStream) may expose internal representation by storing an externally mutable object into XmlMessageRenderer.xstream
MS org.apache.activemq.web.WebClient.selectorName should be package protected

Multithreaded correctness Warnings

Code Warning
IS Inconsistent synchronization of org.apache.activemq.web.AjaxListener.lastAccess; locked 50% of time
IS Inconsistent synchronization of org.apache.activemq.web.RemoteJMXBrokerFacade.connector; locked 83% of time
IS Inconsistent synchronization of org.apache.activemq.web.SessionPool.connection; locked 50% of time
IS Inconsistent synchronization of org.apache.activemq.web.WebClient.producer; locked 80% of time

Performance Warnings

Code Warning
Bx Boxing/unboxing to parse a primitive org.apache.activemq.web.WebClient.initConnectionFactory(ServletContext)
Dm org.apache.activemq.web.MessageQuery.getBody() invokes inefficient new String(String) constructor
SBSC org.apache.activemq.web.HttpAuditLogEntry.toString() concatenates strings using + in a loop
SS Unread field: org.apache.activemq.web.AjaxWebClient.expireAfter; should this field be static?
SS Unread field: org.apache.activemq.web.MessageListenerServlet.readTimeoutParameter; should this field be static?
SS Unread field: org.apache.activemq.web.MessageServlet.oneShotParameter; should this field be static?
SS Unread field: org.apache.activemq.web.MessageServlet.readTimeoutParameter; should this field be static?
SS Unread field: org.apache.activemq.web.MessageServlet.readTimeoutRequestAtt; should this field be static?
WMI org.apache.activemq.web.HttpAuditLogEntry.toString() makes inefficient use of keySet iterator instead of entrySet iterator

Dodgy code Warnings

Code Warning
DLS Dead store to now in org.apache.activemq.web.AjaxWebClient.closeIfExpired()
DLS Dead store to jobScheduler in org.apache.activemq.web.BrokerFacadeSupport.isJobSchedulerStarted()
DLS Dead store to sync in org.apache.activemq.web.MessageServlet.doPost(HttpServletRequest, HttpServletResponse)

Details

DM_BOXED_PRIMITIVE_FOR_PARSING: Boxing/unboxing to parse a primitive

A boxed primitive is created from a String, just to extract the unboxed primitive value. It is more efficient to just call the static parseXXX method.

CT_CONSTRUCTOR_THROW: Be wary of letting constructors throw exceptions.

Classes that throw exceptions in their constructors are vulnerable to Finalizer attacks

A finalizer attack can be prevented, by declaring the class final, using an empty finalizer declared as final, or by a clever use of a private constructor.

See SEI CERT Rule OBJ-11 for more information.

DLS_DEAD_LOCAL_STORE: Dead store to local variable

This instruction assigns a value to a local variable, but the value is not read or used in any subsequent instruction. Often, this indicates an error, because the value computed is never used.

Note that Sun's javac compiler often generates dead stores for final local variables. Because SpotBugs is a bytecode-based tool, there is no easy way to eliminate these false positives.

DM_STRING_CTOR: Method invokes inefficient new String(String) constructor

Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter.  Just use the argument String directly.

DM_DEFAULT_ENCODING: Reliance on default encoding

Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

EI_EXPOSE_REP: May expose internal representation by returning reference to mutable object

Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.

EI_EXPOSE_REP2: May expose internal representation by incorporating reference to mutable object

This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

IS2_INCONSISTENT_SYNC: Inconsistent synchronization

The fields of this class appear to be accessed inconsistently with respect to synchronization.  This bug report indicates that the bug pattern detector judged that

A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held.  Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

MS_PKGPROTECT: Field should be package protected

A mutable static field could be changed by malicious code or by accident. The field could be made package protected to avoid this vulnerability.

PA_PUBLIC_PRIMITIVE_ATTRIBUTE: Primitive field is public

SEI CERT rule OBJ01-J requires that accessibility to fields must be limited. Otherwise, the values of the fields may be manipulated from outside the class, which may be unexpected or undesired behaviour. In general, requiring that no fields are allowed to be public is overkill and unrealistic. Even the rule mentions that final fields may be public. Besides final fields, there may be other usages for public fields: some public fields may serve as "flags" that affect the behavior of the class. Such flag fields are expected to be read by the current instance (or the current class, in case of static fields), but written by others. If a field is both written by the methods of the current instance (or the current class, in case of static fields) and from the outside, the code is suspicious. Consider making these fields private and provide appropriate setters, if necessary. Please note that constructors, initializers and finalizers are exceptions, if only they write the field inside the class, the field is not considered as written by the class itself.

SBSC_USE_STRINGBUFFER_CONCATENATION: Method concatenates strings using + in a loop

The method seems to be building a String using concatenation in a loop. In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.

Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 5) explicitly.

For example:

// This is bad
String s = "";
for (int i = 0; i < field.length; ++i) {
    s = s + field[i];
}

// This is better
StringBuffer buf = new StringBuffer();
for (int i = 0; i < field.length; ++i) {
    buf.append(field[i]);
}
String s = buf.toString();

SS_SHOULD_BE_STATIC: Unread field: should this field be static?

This class contains an instance final field that is initialized to a compile-time static value. Consider making the field static.

SE_NO_SUITABLE_CONSTRUCTOR_FOR_EXTERNALIZATION: Class is Externalizable but doesn't define a void constructor

This class implements the Externalizable interface, but does not define a public void constructor. When Externalizable objects are deserialized, they first need to be constructed by invoking the public void constructor. Since this class does not have one, serialization and deserialization will fail at runtime.

SE_BAD_FIELD: Non-transient non-serializable instance field in serializable class

This Serializable class defines a non-primitive instance field which is neither transient, Serializable, or java.lang.Object, and does not appear to implement the Externalizable interface or the readObject() and writeObject() methods.  Objects of this class will not be deserialized correctly if a non-Serializable object is stored in this field.

WMI_WRONG_MAP_ITERATOR: Inefficient use of keySet iterator instead of entrySet iterator

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.