Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object

Question

I'm getting this error when running checkstyle on my code for the following lines:

@Override
public String[] getDescriptions() {
    return DESCRIPTIONS;
}

but DESCRIPTIONS IS NOT mutable. It's defined as:

private static final String[] DESCRIPTIONS = new String[NUM_COLUMNS];

static {
   // In a loop assign values to the array.
   for (int i = 0; i < NUM_COLUMNS; ++i) {
       DESCRIPTIONS[i] = "Some value";
   }
}

This is the complete error message:

"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."

Related Question: Link

The items inside DESCRIPTIONS are mutable. – bmargulies Dec 18 '13 at 00:29 — bmargulies, Dec 18 '13 at 00:29

Mik378 · Accepted Answer · 2019-04-30T16:25:13.843

7

Arrays and some collections are not immutable in the sense that their content still remains mutable.

Immutability in Java only concerns object's reference assignment, not its deep content.

Try this:

@Override
public String[] getDescriptions() {
    return Arrays.copyOf(DESCRIPTIONS, DESCRIPTIONS.length);
}

BTW, caution to java naming convention.. : descriptions, not DESCRIPTIONS

edited Apr 30 '19 at 16:25

answered Dec 18 '13 at 00:32

Mik378

21,881
15
82
180

1

@Sotirios Delimanolis Yes, unmodifiable collections shouldn't be concerned about this kind of mutability. – Mik378 Dec 18 '13 at 00:33
This is the naming convention my team follows (VERY STRICTLY). Thanks for the quick explanation and possible solution. – user1071840 Dec 18 '13 at 00:36
@user1071840 Very strange convention ;) You're welcome. – Mik378 Dec 18 '13 at 00:36
2

I believe the convention is right: DESCRIPTIONS is a **static final** String[] for which I think they aimed to make it a constant – Adrian Shum Dec 18 '13 at 02:47
@Adrian Shum The concept of mutating a variable (in this case an array) with uppercases (even `final`) would confuse me. The following answer explains why and I totally agree: http://stackoverflow.com/a/18641425/985949 – Mik378 Dec 18 '13 at 09:18
@Mik378 I can understand your concern, and its a pity that Java is providing only `Foo * const` equivalent in C++, but not `const Foo *`. However, if we look into the **semantic** meaning of OP, if he aimed to have that array as constant and not-changing (although language-wise it is not guarding), I still find it OK to use ALL_CAPITAL_UNDERSCORE_SEPARATED convention (Although I think changing to an immutable list will be a much safer choice). – Adrian Shum Dec 19 '13 at 04:11
@Adrian Shum Yes, I would rather opt for an unmodifiable list to stay consistent with the concept. – Mik378 Dec 19 '13 at 10:32

score 1 · Answer 2 · answered Dec 18 '13 at 00:31

The reference variable is final so you cannot assign another array to DESCRIPTIONS. However, the object itself is mutable (arrays are always mutable), final or not. If you return the reference, then you lose control over the contents your variable, violating encapsulation.

You would need either to return a copy of the array, or don't return the array at all, providing a method to get a specific element of the array instead, if that's good enough.

Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object

2 Answers2