Sonar Violation: Security - Array is stored directly

      public void setMyArray(String[] array) {

            this.array = array;

      }

     

Solution

      public void setMyArray1(String[] newArray) {

            if(newArray == null) {

                  this.newArray = new String[0];

            } else {

                  this.newArray = Arrays.copyOf(newArray, newArray.length);

            }

      }

Why should we avoid to store array directly?

Array stored by your object also held by the caller and the caller subsequently modifies this array, the array stored in the object (and hence the object itself) will change.

The solution is to make a copy within the object when it gets passed. This is called defensive copying. A subsequent modification of the collection won't affect the array stored within the object.

It's also good practice to normally do this when returning a collection. Otherwise the receiver could perform a modification and affect the stored instance.

Note

This obviously applies to all mutable collections (and in fact all mutable objects) - not just arrays. Note also that this has a performance impact which needs to be assessed alongside other concerns.

Empty arrays and collections should be returned instead of null

Returning null instead of an actual array or collection forces callers of the method to explicitly test for nullity, making them more complex and less readable. Moreover, in many cases, null is used as a synonym for empty.

public static Result[] getResults() {

  return null;                             // Non-Compliant

}

public static void main(String[] args) {

  Result[] results = getResults();

  if (results != null) {                   // Nullity test required to prevent NPE

    for (Result result: results) {

      /* ... */

    }

  }

}

should be refactored into:

public static Result[] getResults() {

  return new Result[0];                    // Compliant

}

public static void main(String[] args) {

  for (Result result: getResults()) {

    /* ... */

  }

}

This rule also applies to collections:

public static List<Result> getResults() {

  return null;                             // Non-Compliant

}

should be refactored into:

public static List<Result> getResults() {

  return Collections.EMPTY_LIST;           // Compliant

}

Return an empty collection instead of null.

Throwable and Error classes should not be caught

Throwableis the superclass of all errors and exceptions in Java. Error is the superclass of all errors which are not meant to be caught by applications.

Catching either Throwable or Error will also catch OutOfMemoryError or InternalError from which an application should not attempt to recover.

Only Exception and its subclasses should be caught.

try { /* ... */ } catch (Throwable t) { /* ... */ }    // Non-Compliant

try { /* ... */ } catch (Error e) { /* ... */ }        // Non-Compliant

try { /* ... */ } catch (Exception e) { /* ... */ }    // Compliant

Catch Exception instead of Error.

Why should we not caught Throwable and Error?

Usually Errors are problems you cannot possibly recover from, like OutOfMemoryError. There's nothing to do by catching them, so you should usually let them escape, and bring down the virtual machine.