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

FindBugs is a neat tool to monitor your code quality. The sooner or later you will probably hit this malicious code vulnerability: „Method-X may expose internal representation by storing/returning an externally mutable object into/from Field-Y“. In most cases, this relates to storing or returning values of type java.util.Date.

Experienced Java developers know that Date objects are mutable:

Date now = new Date();
now.setTime(1414141414141L);
System.out.println(now);

This can be taken advantage of for manipulations:

public class MyBean {
  private Date createdAt = new Date();
  public Date getCreatedAt() { return createdAt; }
  // no setCreatedAt()
}

Calling myBean.getCreatedAt().setYear(100) alters the internal state of the bean instance even though there is no setter for this property.

How to deal with this problem?

There are several solutions that come to my mind:

1. You just simply stop using Date-s in favor of Long-s to store timestamps because long values are known to be immutable.
2. You create defensive copies of the Date values in your getter and setter methods.
3. You promise never to use mutator methods on Date and then you can switch off the FindBugs rules EI_EXPOSE_REP and EI_EXPOSE_REP2.

Every one of these options comes with some challenges: The first one might not work out entirely as more often than not you are going to need true Date values anyway and then you need to convert the values. When it comes to the second option, quite a few developers miss the fact that Date values might be null and a simple „(Date) value.clone()“ is just not enough. Option 3 cannot be enforced. Or, can it?

jQAssistant to mind mutator methods

jQAssistant is a tool that lets you gain insights into the inner structure of you software application. Based on this structure it allows you to define and enforce constraints. To play around with the tool, you may use the installation at http://jqassistant.org/demo/java8/. Here you will find the structure of the JDK 8. The data is stored into a Neo4j graph database. Cypher is the name of the query language.

Before making queries you will need to know both the syntax of the query language and the model of the database. The latter can be achieved by clicking the icon with the 3 bubbles. Then you will see all available labels and relationship types.

For our purposes we can focus on the labels

• Class
• Method

and the relationship types

• DECLARES
• INVOKES

Step-by-step to the final query

As the final query cannot deny a certain level of complexity, I would like to compose it step-by-step. Let’s start with the Date class:

match
  (dateClass:Class)
where
  dateClass.fqn = "java.util.Date"
return
  dateClass

When you put this query into the JDK8 playground, you will get a blue circle with a number inside. A click into the circle brings up a properties panel where you can change to appearance of the node. Let’s choose „fqn“ instead of node id.

For the next step I would like to add the methods of Date to the query:

match
  (dateClass:Class)-[:DECLARES]->(method:Method)
where
  dateClass.fqn = "java.util.Date"
return
  dateClass, method

The result is a cloud of circles. Let’s also change the nodes to display the method name instead of the node id.

Now, I would like to include also the subclasses of java.util.Date into the query. The change is only minor:

match
  (dateClass:Class)-[:DECLARES]->(method:Method)
where
  dateClass.fqn in ["java.util.Date", "java.sql.Date",
                    "java.sql.Time", "java.sql.Timestamp"]
return
  dateClass, method

An attentive reader might remark that the subclass relationship can be derived from the model, too. That is correct, but this piece of information is not available when JQAssistant is applied not to the JDK but the own project. This is unfortunate!

The cloud of circles has been grown already to considerable size. It is time to restrict the query a bit.

match
  (dateClass:Class)-[:DECLARES]->(mutatorMethod:Method)
where
  dateClass.fqn in ["java.util.Date", "java.sql.Date",
                    "java.sql.Time", "java.sql.Timestamp"]
  and mutatorMethod.name =~ "set.*"
return
  dateClass, mutatorMethod

The result should look like this:

With all the preparatory work, last step to the final query is small: We search any method (someMethod) in any class (someClass) that calls (call) a mutator method on Date.

match
  (dateClass:Class)-[:DECLARES]->(mutatorMethod:Method),
  (someClass:Class)-[:DECLARES]->(someMethod:Method),
  (someMethod)-[call:INVOKES]->(mutatorMethod)
where
  dateClass.fqn in ["java.util.Date", "java.sql.Date",
                    "java.sql.Time", "java.sql.Timestamp"]
  and mutatorMethod.name =~ "set.*"
return someClass.fqn, someMethod.signature, call.lineNumber

Done! This query can now be incorporated into the project’s quality rules. This will break the Maven build whenever such a method invocation is detected. The FindBugs rule can now be muted – unless of course – you are using in your bean classes other unexpectedly mutable objects (arrays!). But that is a different story.