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?
SonarQube is telling you to be cautious when receiving List
s 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:
List
content by adding or removing elements to it, actually affecting your MenuItemExpandedDTO
.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.