@Data with arrays generates FindBugs violation

[lombokissues]

Migrated from Google Code (issue 77)

[lombokissues]

:bust_in_silhouette: jacek99   :clock8: Dec 04, 2009 at 16:54 UTC

What steps will reproduce the problem?

1. Create a @ Data POJO with a String[] field (e.g. private String[] services;)
2. Run the FindBugs maven plugin against the code
3. Findbugs generates a warning on the generated code: "may expose internal representation by returning" your array MALICIOUS_CODE EI_EXPOSE_REP

The FindBugs glossary describes this as:

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

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.

What is the expected output? What do you see instead?

No FindBugs warnings

What version of the product are you using? On what operating system?

0.9.0, on Ubuntu 9.04 with JDK 6

Please provide any additional information below.

Found this after adding the Findbugs maven plugin to our pom.xml

[lombokissues]

:bust_in_silhouette: reinierz   :clock8: Dec 04, 2009 at 20:12 UTC

The only way to fix this is to duplicate the array.

Which is not a good solution - it's somewhat surprising that this will in fact happen, and for consistency we should then duplicate ALL mutable fields. But we can't. For two reasons:

(1) we have no idea which types are mutable and which types are immutable; mutability is not part of java's type system.

(2) Even if we did know, there's no general way to duplicate an object. clone() usually doesn't work.

So, good point, but we can't fix it, at least not without adding a heap of complexity into how lombok works.

[lombokissues]

:bust_in_silhouette: mclauss77   :clock8: Jul 01, 2013 at 13:30 UTC

This problem also adds for other situations, e.g. Date types. What about a specific annotation to get lombok provide a clone-based implementation? That might be generic enough...

[lombokissues]

:bust_in_silhouette: reinierz   :clock8: Jul 01, 2013 at 16:08 UTC

This will remain WontFix - this academicy model of thinking just doesn't fly in java. Array data is usually very large, so just duplicating it is crazy, especially as, in the vast majority of circumstances, that duplication is completely pointless as nobody actually messes with the data. More generally, if this is an issue you should not just duplicate the array*, you should change the API of the object so that it is no longer possible to just ask for the array data. For example, change to a list implementation, and wrap things in guava's ImmutableList. Telling lombok to clone isn't good enough. When should lombok clone?

• When the object is provided during object creation / calling of the set method.
• When the object is returned via a getter.

Cloning for both produces a huge amount of garbage collection cruft and in general results in ridiculous performance penalties. Again, this is working on the general basis that your mutable objects are large, but they tend to be.

For java.util.Date objects, the right solution is to not use date and instead roll with jodatime's date representations, or 'long', because those are both immutable. The solution is not to clone the incoming j.u.Date object on both edges of the set/get process. That's crazy. The solution for arraydata is to make it ImmutableList, and to actually USE the ImmutableList type in your data, or, alternatively, to accept in constructor/setter any list, but to have the field type be immutablelist. This makes it clear for those doing a get() call that the returned list is immutable.

*) You can't just create a single public copy or even cache this copy, because if you did, then 2 different codebases that both call getX() can see each other's modifications.

It's hugely impractical to try and automate this with lombok. Even if it was possible (and note that the feature page for this would be many tens of pages long to cover all the hairy details!), it's clearly no longer boilerplate.

[lombokissues]

:bust_in_silhouette: reinierz   :clock8: Feb 01, 2015 at 23:22 UTC

We stand by the advice that you should just in general tell findbugs to not warn on this, but a feature was just added to tell findbugs not to warn on anything that lombok generates. See:

issue #737

[lombokissues]

End of migration