Mutable members should not be stored on ArrayList - Sonar vulnerability

Viewed 68

I have sonar "Mutable members should not be stored or returned directly" issue in my java class. I have fixed it by below code but still sonar is giving me the same error

public class UserInfoImpl
{
    List<UserInfo> userDetails = Collections.emptyList();

    void perform()
    {

            List<UserInfo> userDetails = service.getUserDetails();
            userDetails = new ArrayList<>(userDetails);
            this.userDetails = Collections.unmodifiableList(userDetails);
       
    }

 
    public final List<UserInfo> getUserDetailList()
    {
        return userDetails;  // sonar error happens at this line
    }
}

does anyone have any idea?

2 Answers

This rule protects against a method being able to change the state of your UserInfoImpl instance after calling one of its getter functions.

There are two possibilities:

  1. A caller can change the returned list. You have possibly already prevented that by using Collections.unmodifiableList, but that is done in a method called perform() - how does Sonar know that perform() is always called before getUserDetailList? Why not return Collections.unmodifiableList inside the body of your getUserDetailList method?

  2. Even if you ensure that the returned list is always unmodifiable, the caller can still modify the UserInfo instances contained in the list. To prevent that possibility, ensure that UserInfo is unmodifiable, or make a copy of each UserInfo that you put into the list returned by getUserDetailList.

You are returning a reference to userDetails. By doing this you break encapsulation which can potentially bring your UserInfoImpl class in an inconsistent state. For example when a client (other class that uses your class) calls clear on the returned list.

// client A
var list = userInfoImpl.getUserDetailList();
list.clear();

// client B
var list = userInfoImpl.getUserDetailList();
list.get(0); // IndexOutOfBoundsException

Other clients that use the same object can now get unexpected IndexOutOfBoundsException

To solve this you can return a copy of the list.

Related