Skip to content

Commit

Permalink
Fix ACL Delete Logic (#15112)
Browse files Browse the repository at this point in the history
Problem:

When deleting existing ACLs as part of processing a Write Request
containing a 'replace all' list operation, the existing logic was
incorrectly passing a pointer to the last entry that we had processed in
the new provided list when calling LogEvent.

Not only is this logging the wrong entry beyond the first iteration of
the loop, it also does not work if the new list being provided to the caller
is actually empty.

This caused it to send an error back to the client.

Fix:

Correctly read out the i-th entry from the existing ACL list and log
that before deleting that entry.

Testing:

Validated by using reproducing the original error in the REPL, and
validating that it succeeds after.
  • Loading branch information
mrjerryjohns authored Feb 12, 2022
1 parent 1970d64 commit 2e7d65b
Showing 1 changed file with 5 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -486,10 +486,13 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP

while (i < oldCount)
{
AccessControl::Entry entry;

--oldCount;
ReturnErrorOnFailure(GetAccessControl().ReadEntry(oldCount, entry, &accessingFabricIndex));
ReturnErrorOnFailure(
LogAccessControlEvent(entry, aDecoder.GetSubjectDescriptor(), AccessControlCluster::ChangeTypeEnum::kRemoved));
ReturnErrorOnFailure(GetAccessControl().DeleteEntry(oldCount, &accessingFabricIndex));
ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
AccessControlCluster::ChangeTypeEnum::kRemoved));
}
}
else if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
Expand Down

0 comments on commit 2e7d65b

Please sign in to comment.