If AccessSection directly returns its modifyable list, callers can
modify the list outside of AccessSection. This is bad because this way
it is possible to violate assumptions of the AccessSection class. E.g.
AccessSection makes sure that the permission list cannot contain
duplicate permissions. Having duplicate permissions in the permission
list can lead to severe problems. E.g. duplicate permissions on an
access section of the All-Projects project make the permissions of the
All-Project project unreadable and Gerrit effectively stops working.
This is because ProjectState#getLocalAccessSections() filters out some
permissions on the All-Projects project by getting the list of
permissions from AccessSection and setting the filtered list back on
AccessSection. If the list that was retrieved from AccessSection
contained duplicate permissions, setting back the list failed with
IllegalArgumentException since
AccessSection#setPermissions(List<Permission>) doesn't allow duplicate
permissions.
A similar issue was already fixed by change I5222cd9174 and change
Ief24c6e82f.
Ideally we would return an ImmutableList from
AccessSection.getPermissions() but we cannot use ImmutableList in
classes that are used by the GWT UI.
Change-Id: I61f0baf6deb5c4ba1b609aacaabeb1f149f444d9
Signed-off-by: Edwin Kempin <ekempin@google.com>
f3c75eae24