element-hq / element-hq/synapse

Disallow `/batch_send` usage in a room where the room version doesn't support it or is not the room creator

Open
#13,216 0 comments 0 reactions 0 assignees View on GitHub
S-Tolerable T-Defect
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

This issue has been migrated from [#13216](https://github.com/matrix-org/synapse/issues/13216).

---

### Description

Currently, we just don't persist the insertion, batch, and marker events but we shouldn't allow the `/batch_send` usage in the first place to make it more obvious.

### Steps to reproduce

I created a test case [here](https://github.com/matrix-org/synapse/commit/e952a4b83fff1b1b7ce5e600a0c9099156a409f9) that reproduces this issue 100% of the time. It's a near copy-paste of another one of the batch send test cases, but with different assertion logic.

### Homeserver

Branch based off of https://github.com/matrix-org/synapse/commit/0c95313a448ab38629a13443ea9b3e0e5cc65d39

### Synapse Version

{"server_version":"1.61.0 (b=develop,f33356e8f8)","python_version":"3.10.4"}

### Installation Method

_No response_

### Platform

NixOS. Running in Python virtualenv.

### Relevant log output

```shell
$ trial tests.rest.client.test_room_batch
tests.rest.client.test_room_batch
RoomBatchTestCase
test_room_batch_event_id_persistence ... [FAIL]
test_same_state_groups_for_whole_historical_batch ... [OK]
test_sync_while_batch_importing ... [OK]

===============================================================================
[FAIL]
Traceback (most recent call last):
File "/home/sumner/projects/beeper/synapse/tests/rest/client/test_room_batch.py", line 337, in test_room_batch_event_id_persistence
self.assertEqual(
File "/home/sumner/projects/beeper/.venv/lib/python3.10/site-packages/twisted/trial/_synctest.py", line 422, in assertEqual
super().assertEqual(first, second, msg)
File "/nix/store/40n9pd613v8fc3x39yjrgs1i7d4q8yl0-python3-3.10.4/lib/python3.10/unittest/case.py", line 845, in assertEqual
assertion_func(first, second, msg=msg)
File "/nix/store/40n9pd613v8fc3x39yjrgs1i7d4q8yl0-python3-3.10.4/lib/python3.10/unittest/case.py", line 838, in _baseAssertEqual
raise self.failureException(msg)
twisted.trial.unittest.FailTest: None != '$YtSa2mJ7KNSHoiLf7tf63Jn7rOnxmezUts1gaE-pKFk'

tests.rest.client.test_room_batch.RoomBatchTestCase.test_room_batch_event_id_persistence
-------------------------------------------------------------------------------
Ran 3 tests in 2.834s

FAILED (failures=1, successes=2)
```

### Anything else that would be useful to know?

_No response_

Contributor guide

Open the contributing guide

Research direction

Start with tests/rest/client/test_room_batch.py and run trial tests.rest.client.test_room_batch, including the reproduction linked in the issue. Trace the /batch_send request handling and its existing room-version and creator checks. Done means unsupported rooms or non-creators are rejected instead of silently discarding insertion, batch, and marker events.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.