javasonarqubedtosonarqube-scansonarqube-ops

SonarQube: "Store a copy of "productAllergenInfos""


I have the following DTO:

@Data
@RequiredArgsConstructor
public class MenuItemExpandedDTO {
private UUID uuid;
private List<ModifierGroupDTO> modifierGroupDtoList;
private List<AllergenInfo> allergenInfoList;

public MenuItemExpandedDTO(
        PropertiesDTO propertiesDto,
        List<ModifierGroupDTO> modifierGroupDtoList,
        List<AllergenInfo> allergenInfoList
) {
    this.uuid = propertiesDto.getUuid();
    this.modifierGroupDtoList = modifierGroupDtoList;
    this.allergenInfoList = allergenInfoList;
  }
}

In SonarQube analysis, I get a Vulnerability due to allergenInfoList as it is stated

"Message: Store a copy of allergenInfoList"

So, I am not sure what the problem is, but before fixing this error, I am wondering what is wrong with that code? In some pages, it is recommended to initialize the list e.g. private List<AllergenInfo> allergenInfoList = Collections.emptyList(). But it is not a way I follow in my projects. So, what is the problem with this code?


Solution

  • SonarQube is telling you to be cautious when receiving Lists in a constructor. Why? Because the caller holds a reference to that List and it can do the following with it if it is not immutable:

    1. Change the List content by adding or removing elements to it, actually affecting your MenuItemExpandedDTO.
    2. Change the objects contained in the List if they are not immutable. This means that AllergenInfo objects in the List can be changed affecting your MenuItemExpandedDTO object.

    To tackle 1., you can simply store a copy of the List as SonarQube suggests:

    public MenuItemExpandedDTO(
            PropertiesDTO propertiesDto,
            List<ModifierGroupDTO> modifierGroupDtoList,
            List<AllergenInfo> allergenInfoList
    ) {
        this.uuid = propertiesDto.getUuid();
        this.modifierGroupDtoList = new ArrayList<>(modifierGroupDtoList);
        this.allergenInfoList = new ArrayList<>(allergenInfoList);
      }
    }
    

    Tackling 2. is trickier and the simplest and more reliable solution is to use immutable objects. You can read more about this and how to design your classes so that you have immutable objects at https://www.baeldung.com/java-immutable-object.

    public class MenuItemExpandedDTO {
        private final UUID uuid;
        private final List<ModifierGroupDTO> modifierGroupDtoList;
        private final List<AllergenInfo> allergenInfoList;
    
        public MenuItemExpandedDTO(PropertiesDTO propertiesDto,
                                   List<ModifierGroupDTO> modifierGroupDtoList,
                                   List<AllergenInfo> allergenInfoList) {
            this.uuid = propertiesDto.getUuid();
            this.modifierGroupDtoList = new ArrayList<>(modifierGroupDtoList);
            this.allergenInfoList = new ArrayList<>(allergenInfoList);
        }
    
        public UUID getUuid() {
            return UUID;
        }
    
        public List<ModifierGroupDTO> getModifierGroupDtoList() {
            return new ArrayList<>(modifierGroupDtoList);
        }
    
        public List<AllergenInfo> getAllergenInfoList() {
            return new ArrayList<>(allergenInfoList);
        }
    }
    

    Keep in mind that ModifierGroupDTO and AllergenInfo must also be immutable so that MenuItemExpandedDTO is 100% immutable.