[
https://issues.apache.org/jira/browse/SLING-10145?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17285226#comment-17285226
]
Bertrand Delacretaz commented on SLING-10145:
---------------------------------------------
Always Try Again After Lunch - I found a way to expose the issue, by
duplicating the {{remove *}} statement, test added in commit c2e8c53. Not a
likely case but wrong code is wrong, fixed now.
> Wrong handling of "modified" field in AclUtil.setPrincipalAcl
> -------------------------------------------------------------
>
> Key: SLING-10145
> URL: https://issues.apache.org/jira/browse/SLING-10145
> Project: Sling
> Issue Type: Bug
> Components: Repoinit
> Affects Versions: Repoinit JCR 1.1.32
> Reporter: Bertrand Delacretaz
> Assignee: Bertrand Delacretaz
> Priority: Minor
> Fix For: Repoinit JCR 1.1.34
>
>
> While applying the SLING-9090 patch I mistakenly changed
> {code}
> } else if (action == AclLine.Action.REMOVE_ALL) {
> if (removePrincipalEntries(acl, principalName, getJcrPaths(session,
> line.getProperty(PROP_PATHS)))) {
> modified = true;
> }
> {code}
> To
> {code}
> } else if (action == AclLine.Action.REMOVE_ALL) {
> modified = removePrincipalEntries(acl, principalName, getJcrPaths(session,
> line.getProperty(PROP_PATHS)));
> {code}
> which can cause {{modified}} to be reset if {{removePrincipalEntries}}
> returns false, even if another loop iteration has set it true.
> The code should OR {{modified}} with the result of the function call:
> {code}
> } else if (action == AclLine.Action.REMOVE_ALL) {
> modified |= removePrincipalEntries...
> {code}
> The existing tests have not detected this bug, I'll need to augment them to
> do so before fixing that bug.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
