How to solve EI_EXPOSE_REP2 and why exactly its wrong

Question

I ran the FindBugs in my project, and I got the following warning:

new foo.pkg.Key(Client, Product, Date, String) may expose internal representation by storing an externally mutable object into Key.expireDate MALICIOUS_CODE EI_EXPOSE_REP2 60 Medium

Key is an Entity which have a Date expireDate, with its respective getter and setter, and it uses them in the constructor.

Actually, I just return a (Date) dataNascimento.clone(), and use the same strategy in the setter.

Questions (in backwards logical-order):

• Is that the better way of doing this?
• What was wrong with the previous code?
• Why exactly is it wrong to do this?
• Is it because Date is a mutable type?

Solution 1

I would recommend performing your (Date) dataNascimento.clone() call in the constructor (either directly, or via your setter).

Yes, FindBugs is warning you due to the fact that data is mutable. You may have the clone calls in your setters and getters, but you would still get the warning, since you could still might be able alter the the date inside the constructor.

Solution 2

Change the parameter-type to an immutable like LocalDate (Java 8).

Benefit: Nobody can modify this (im-mutable) date afterwards.

Bug description

See Spotbugs' docs > Bug description for EI_EXPOSE_REP2: EI2: 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.

Note: you can also lookup descriptions like this in FindBugs' bug descriptions.

Fix: use immutable date objects like LocalDate

To fix the FindBugs issue on immutable parameters like in call new foo.pkg.Key(Client, Product, Date, String), change the constructor to accept an immutable date object, like:

• Joda Time's DateTime:

  All the main datetime classes are immutable (like String) and cannot be changed after creation.

• since Java 8 LocalDate, or other immutables of Java 8 Date & Time API (JSR 310). See the new Time API's concepts in article by Ben Evans and Richard Warburton (2014): Java SE 8 Date and Time:

  Immutable-value classes. One of the serious weaknesses of the existing formatters in Java is that they aren’t thread-safe. This puts the burden on developers to use them in a thread-safe manner and to think about concurrency problems in their day-to-day development of date-handling code. The new API avoids this issue by ensuring that all its core classes are immutable and represent well-defined values.

var today = LocalDate.now();
var key = new Key(client, product, today, "immutable date - can't change")`

// no modification possible:
today.setYear(1988); // no such modifier exists

Also, the constructor and field in your entity class need to be designed for immutability like:

package foo.pkg;

public class Key {
    private LocalDate expiryDate; // now immutable (before: Date)

    // remaining fields omitted

    public Key(Client client, Product product, LocalDate expiryDate, String s) {
       this.expiryDate = expiryDate;
       
      // remaining initialization omitted
    }

}

See related questions

• Findbugs issues with mutability of Date object in Java
• Does the FindBugs EI_EXPOSE_REP bug only concern Date?