0

I have a member variable

private ArrayList<CalendarableItem>[] resourceColumns = null;

and getter for the same

 public ArrayList<CalendarableItem>[] getResourceColumns()
{
    return resourceColumns;
}

I am seeing findbugs for above getter method. Malicious code vulnerability : EI: Method returning array may expose internal representation

I found that I have to do a deep copy of array object to remove this error Malicious code vulnerability - May expose internal representation by incorporating reference to mutable object

I dont want to do clone of this object due to performance issue. Do we have any other better solution.

Community
  • 1
  • 1
MIM
  • 499
  • 3
  • 11
  • 30

2 Answers2

0

If you want the List to be immutable in depth, remove the getter. This getter returning the list give the possibility to remove, add, ... any items in it.

You could instead use something similar to an adapter to just gave the access you want. Like a specific getter or the size of the list but without giving the access to the list.

private List<CalendarableItem> resourceColumns = new ArrayList<>();

public CalendarableItem getCalendarableItem(int index){
     return resourceColumns.get(index);
}

public int getSize(){ return resourceColumns.size(); }

Your list will be private an immutable (for the moment). The only access possible are those you adapt in your class.

If you want to prevent the instance to be updated, you could return a copy of it too because for the moment, the instance return is the one from the list (same reference).

EDIT : I have just notice that this was an Array of ArrayList, so this example is not quite functionnal like this, it was written for a simple ArrayList. You need to update the adapter depending on the needs.

AxelH
  • 14,325
  • 2
  • 25
  • 55
0

Just a suggestion instead of using ArrayList<CalendarableItem>[] you should use List<List<CalendarableItem>>

Now coming back to your question, you can return clone of array so that in case any one make any changes to array it will not reflect in your initial array.

public ArrayList<CalendarableItem>[] getResourceColumns()
{
    return Arrays.copyOf(resourceColumns, resourceColumns.length);
}

If you want/need more control then instead of method getResourceColumns() you will need to write separate methods return object at specific array index etc.

  • Those are his words : _I dont want to do clone of this object due to performance issue_ and this will not perform a deep copy, only a copy of the collection – AxelH Jan 12 '17 at 08:05
  • Array and List are mutable objects hence he at least needs to clone array. ArrayList objects will be as is. So cloning array wont eat so much memory of course it depends upon array length as well. – Ninad Shaha Jan 12 '17 at 09:17
  • But your copied array will still contain the same reference (aka same list) so his test will still return _Method returning array may expose **internal representation**_ i think. Yes you will prevent a dev to remove or override an instance in the array, but you will still return the list so you will be able to empty it if you want. I believe OP want to prevent that because he talk about a deep copy – AxelH Jan 12 '17 at 09:33