Project: ActiveMQ :: Web
SpotBugs version: 4.8.3
Code analyzed:
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)
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 |
Click on a warning row to see full context information.
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 |
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[]) |
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 |
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 |
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 |
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) |
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.
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.
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.
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.
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.
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.
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.
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.
A mutable static field could be changed by malicious code or by accident. The field could be made package protected to avoid this vulnerability.
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.
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();
This class contains an instance final field that is initialized to a compile-time static value. Consider making the field static.
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.
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.
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.