Sonar Violation: Security Array is stored directly

Question

Sonar Violation: Security - Array is stored directly

public void setMyArray(String[] myArray) { 
  this.myArray = myArray; 
}

Solution:

public void setMyArray(String[] newMyArray) { 
  if(newMyArray == null) { 
    this.myArray = new String[0]; 
  } else { 
   this.myArray = Arrays.copyOf(newMyArray, newMyArray.length); 
  } 
}

[or]

public void setMonths(String[] months)
{
this.months = months.clone();
}

which of the above is perfect and doesnt cause any hindrance?

Please precise your question: both solutions are valid and help to solve the issue raised by this rule but both have pro's and con's, so unless you precise what you are looking for it is impossible to answer your question. — benzonico, Mar 09 '15 at 08:12
@benzonico i have an assignment to fix this sonar violation, so will clone() work properly for arrays? without creating any future hindrance? — Vignesh Rajagopal, Mar 09 '15 at 10:04

score 0 · Answer 1 · answered Mar 09 '15 at 15:49

clone() will work properly as long as you don't pass null value, then if your application does authorize null value to be passed to this method, you should probably check for this.

if(months==null) {
  this.months = null;
} else {
  this.months = months.clone();
}

score 0 · Answer 2 · edited May 23 '17 at 10:28

Both ways are fine in this case. Clone method should not cause problems for a non-null array of strings as such because java Strings themselves are immutable value objects. But if the array contains some other objects/collections, then deep copy would be a better way.

It would help you to understand the cause of this sonar violation. An array is a mutable object and a defensive copy is a way to handle such scenarios. You can google for defensive copy, deep copy, clone method, etc. Take a look at this question. In case of an array of custom cloneable objects/ collections, a defensive deep copy would be required.

Sonar Violation: Security Array is stored directly

2 Answers2