-
Bug
-
Resolution: Fixed
-
Blocker
-
4.5.16
-
-
Yes
-
Empty show more show less
-
Yes
It seems we have discovered quite a serious bug related to the handling of user groups. When there are recursive groups in Magnolia the MgnlUser#addSubgroups method ends up in a recursive loop resulting in a stackoverflowerror.
The method's JavaDoc claims to prevent against such infinite loops, however this is unfortunately not the case. The fix seems extremely simple. Just replace this code:
// and recursively add more subgroups
addSubgroups(allGroups, man, subgroups);
allGroups.addAll(subgroups);
with:
allGroups.add(groupName);
// and recursively add more subgroups
addSubgroups(allGroups, man, subgroups);
Because in the current code the 'allGroups' collection remains empty and no checking against recursive groups is done at all.
In our temporary hotfix we have added a check to see if there actually are any subgroups:
allGroups.add(groupName); if (!subgroups.isEmpty()) { // and recursively add more subgroups addSubgroups(allGroups, man, subgroups); }
I have attached our MgnlUser.java with our fix (note: tabs instead of spaces so please reformat according to your conventions). It was build against Magnolia 4.5.16 but I think the issue is present in all recent Magnolia versions.
Our situation: our client has a lot of groups in Magnolia and in some cases we have a recursive situation where a subgroup contains a parent group somewhere in the tree. I reproduced this bug locally with a very simple test case using 2 groups where group A contains group B as a subgroup and group B again contains group A. The current unit tests of the MgnlUser class do not contain such a recursive group setup. They should.