mirror of
https://github.com/the-draupnir-project/Draupnir.git
synced 2026-06-03 22:31:17 +00:00
Mjolnir would apply stale ACL to rooms during batching (#331)
* banListTest would applyACL before rules appeared in `/state`. Mjolnir will call applyServerAcls several times while a policy list is being updated, sometimes concurrently. This means a request to set a server ACL in a room which has old data can finish after a more recent recent request with the correct ACL. This means that the old ACL gets applied to the rooms (for a moment). This is a follow up from https://github.com/matrix-org/mjolnir/pull/314/commits/551065815e65e6117d7e8105afd6f5da9b87c8af * Only allow one invocation of applyServerAcls at a time as to not conflict with each other by using a promise chain. We don't use the throttle queue because we don't want to be blocked by other background tasks. We don't make another throttle queue because we don't want throttling and we don't want to delay the ACL application, which can happen even with throttle time of 0.
This commit is contained in:
@@ -261,20 +261,23 @@ describe('Test: ACL updates will batch when rules are added in succession.', fun
|
||||
mjolnir.joinRoom(banListId);
|
||||
this.mjolnir!.watchList(Permalinks.forRoom(banListId));
|
||||
const acl = new ServerAcl(serverName).denyIpAddresses().allowServer("*");
|
||||
for (let i = 0; i < 200; i++) {
|
||||
const evilServerCount = 200;
|
||||
for (let i = 0; i < evilServerCount; i++) {
|
||||
const badServer = `${i}.evil.com`;
|
||||
acl.denyServer(badServer);
|
||||
await createPolicyRule(moderator, banListId, RULE_SERVER, badServer, `Rule #${i}`);
|
||||
// Give them a bit of a spread over time.
|
||||
await new Promise(resolve => setTimeout(resolve, 5));
|
||||
}
|
||||
// give the events a chance to appear in the response to `/state`, since this is a problem.
|
||||
await new Promise(resolve => setTimeout(resolve, 2000));
|
||||
|
||||
// We do this because it should force us to wait until all the ACL events have been applied.
|
||||
// Even if that does mean the last few events will not go through batching...
|
||||
await this.mjolnir!.syncLists();
|
||||
|
||||
// At this point we check that the state within Mjolnir is internally consistent, this is just because debugging the following
|
||||
// is a pita.
|
||||
const list: PolicyList = this.mjolnir.policyLists[0]!;
|
||||
assert.equal(list.serverRules.length, evilServerCount, `There should be ${evilServerCount} rules in here`);
|
||||
|
||||
// Check each of the protected rooms for ACL events and make sure they were batched and are correct.
|
||||
await Promise.all(protectedRooms.map(async room => {
|
||||
const roomAcl = await mjolnir.getRoomStateEvent(room, "m.room.server_acl", "");
|
||||
|
||||
Reference in New Issue
Block a user