If presence is disabled after having been enabled, the presence states
in the database (and hence on clients) are frozen at whatever they were
when presence was last enabled: nothing writes to the presence stream
any more and /sync omits the presence section entirely, so clients show
the old presence states forever.
Fix this in two parts:
1. At startup, if presence is disabled but the database still contains
non-offline presence states, the presence writer sends out one final
round of updates marking those users as offline.
2. /sync no longer unconditionally omits presence when presence is
disabled: incremental syncs whose since token is behind the presence
stream still get the straggling updates. As the stream doesn't advance
while presence is disabled, clients catch up once and the check then
short-circuits to a token comparison.
Remote servers already handle this themselves by timing out our users
([`FEDERATION_TIMEOUT`](https://github.com/element-hq/synapse/blob/4d8905a15a417ed0054ec2533d243932d890bbbd/synapse/handlers/presence.py#L194-L198)),
so no federation changes are needed.
Note that this only fixes the issue if presence is fully disabled. If
set to `untracked` we still have the same issue, however since modules
would still write to presence we can't just clobber everything like we
do in this patch.
This does two things, first it adds a config flag to ignore rooms for
the purposes of presence routing.
Secondly, it changes the caching behaviour to try and improve the cache
hit ratio. Previously, the size of the `do_users_share_a_room` cache
(which stores pairs of users) needs to `O(n²)` for the number of online
users, which is infeasible for large servers.
Instead, we call `get_users_in_room` for both the syncing and updated
users. This sounds more expensive, but a) we will already have cached
the syncing user's rooms, and b) we will only calculate the updated
user's rooms once (rather than once per syncing user).
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- Impose limit of scheduled delayed events
- Update error codes to match latest draft of MSC4140
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Sync workers proxied a full `ReplicationPresenceSetState` call to the
presence writer on every sync request with `affect_presence=True` (via
`user_syncing`), and a `ReplicationBumpPresenceActiveTime` call on every
user action, even though the writer's presence timers only need feeding
every `SYNC_ONLINE_TIMEOUT` / `LAST_ACTIVE_GRANULARITY`. On busy clients
this amounts to tens of no-op replication calls per user per minute, and
the resulting per-update work is the dominant CPU cost on saturated
presence writers.
Track the last relayed `(state, timestamp)` per `(user, device)` on the
worker and suppress unchanged sync-driven repeats within a 25s relay
interval - deliberately below `SYNC_ONLINE_TIMEOUT` (30s) so the
writer's device `last_sync_ts`/`last_active_ts` timers stay fed and
users neither flap offline nor bounce `currently_active`. Genuine state
changes are relayed immediately, explicit (non-sync) set_state calls
always go through and reset the throttle, bumps that might un-idle a
device bypass it, and entries are evicted when a `USER_SYNC` stop is
sent so reconnecting devices are relayed afresh.
This gives the writer-CPU benefit that deployments currently obtain by
tightening `rc_presence` to ~1/29s, without dropping real presence
transitions.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Quentin Gliech <quenting@element.io>
A handful of places in the storage layer bound a value whose Python type
didn't match the declared column type — an `int` into a `TEXT` column,
or a `str` into a `BIGINT` column — and relied on psycopg2's loose
coercion to paper over the mismatch. These are latent correctness bugs:
they only work because the driver silently converts, and a stricter
driver that binds typed parameters rejects them outright.
Found during a Rust port of the database pool where the driver does not
coerce automatically.
Each fix binds the value with the type the column actually declares,
rather than depending on driver-specific coercion. All changes are
behaviour-preserving on psycopg2 and sqlite.
There is also a fix to the multi-writer id-gen tests where we forgot to
commit. This is tangential, but was found during the same effort.
### Changes
- **`device_lists_remote_extremeties.stream_id (TEXT)`** —
`_update_remote_device_list_cache_txn` (typed `stream_id: int`) bound an
int; store it as a string, matching the column and the sibling
`_update_remote_device_list_cache_entry_txn` (typed `stream_id: str`).
The old mismatch, when rejected, was swallowed inside the device-list
resync and hung `query_devices`.
- **`user_filters.filter_id (BIGINT)`** — `get_user_filter` bound the
raw `int | str` (a string, from sync requests). It already validates via
`int(filter_id)`; bind that int so it matches the column.
- **`rejections.last_check (TEXT)`** — both writers stored
`clock.time_msec()` (an int); store the timestamp as a string.
- **user-directory temp position** — the `populate_user_directory`
background update's staging column
`_temp_populate_user_directory_position.position` was `TEXT` but held an
int (read back into a `BIGINT` column). Declare it `BIGINT`. The temp
table is created and dropped within the background update, so there's no
migration.
- **`test_batched_state_group_storing`** — selected from
`state_group_edges` with a stringified `state_group`; bind the int
directly (the column is an integer).
- **Multi-writer id-generator tests** — constructing a
`MultiWriterIdGenerator` prunes stale `stream_positions` rows, but the
harness never committed, so the cleanup only survived because adbapi
keeps one connection per thread with its transaction open. Commit after
construction so it persists regardless of pool semantics (the delete is
legitimate work that should be committed anyway).
Adds `last_active_granularity`, `sync_online_timeout` and `idle_timeout`
options to the `presence` config section, controlling the previously
hard-coded `LAST_ACTIVE_GRANULARITY`, `SYNC_ONLINE_TIMEOUT` and
`IDLE_TIMER` constants (which remain as the defaults).
This is mainly useful on deployments that ratelimit how often syncs can
affect presence (`rc_presence`): the sync timeout must exceed the
ratelimit interval or users flap offline between syncs, so tuning down
presence traffic currently requires squeezing under the fixed 30s limit.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
`test_lock_contention` is a performance-regression canary (#16840): the
pathological behaviour it guards against spent ~30s spinning the CPU, vs
~0.5s when healthy. The 5s wall-clock alarm it used was calibrated on
SQLite, but against PostgreSQL a healthy run already takes 3-4s of
wall-clock time (500 sequential acquire/release cycles, each a real
database round-trip), so any CI load pushed it over the limit.
Add a `cpu_time` mode to `tests/utils.py`'s test_timeout, implemented
with
[`setitimer(ITIMER_PROF)`](https://docs.python.org/3/library/signal.html#signal.setitimer),
which budgets process CPU time instead of wall-clock time. Time spent
blocked on the database or lost to a loaded CI runner no longer counts,
while a regression to CPU-spinning still trips the alarm mid-spin. A
healthy run costs <1s of CPU on either database engine; the budget is
10s.
This also subsumes the RISC-V wall-clock carve-out from #18430, which is
removed.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This is a stepping stone before we can go full Rust everywhere. We're
providing a generic interface as we want database access to work in
Synapse and `synapse-rust-apps`. In `synapse-rust-apps`, we will use a
`tokio-postgres` based database connection pool so it's full Rust.
We want to avoid the situation where we have two database connection
pools (one for Python, one for Rust) as we've run into connection
exhaustion problems on Matrix.org before.
As an example of using it and sanity check for all this work (including
tests), I've also ported over the `/versions` handler to the Rust side
with database access. The `/versions` endpoint is the simplest endpoint
I could find that still had some database access. Hopefully the refactor
on `/versions` isn't that controversial as it's not really the point of
this PR. We can always remove it from this PR but it's just here as a
sanity check that all of this works.
### Why `runInteraction(...)`?
Using the same `runInteraction` pattern that we already have in Synapse
means we can port over existing Synapse code/endpoints without much
thought. But this pattern also makes sense because we want[^1]
transactions to have repeatable-read isolation (easy to think about,
less foot-guns). Having everything thappen in a function callback means
we can do retries for serialization/deadlock errors.
[^1]: To note: Ideally, we'd want the least isolation possible but the
problem is that there is no tooling to yell at you when your
queries/logic is wrong so repeatable-read isolation is a great balance.
> When an application receives this error message, it should abort the
current transaction and retry the whole transaction from the beginning.
The second time through, the transaction will see the
previously-committed change as part of its initial view of the database,
so there is no logical conflict in using the new version of the row as
the starting point for the new transaction's update.
>
> Note that only updating transactions might need to be retried;
read-only transactions will never have serialization conflicts.
>
> *--
https://www.postgresql.org/docs/current/transaction-iso.html#XACT-REPEATABLE-READ*
As a note, this strategy is less of an impedance mismatch (aligns more
closely) with Synapse so the glue code for the `python_db_pool` should
also be simpler.
### How does this interact with logcontext (`LoggingContext`)?
See [docs on log
contexts](https://github.com/element-hq/synapse/blob/4e9f7757f17ba81b8747b7f8f9646d17df145aa3/docs/log_contexts.md)
for more background.
We already support normal logging from Rust -> Python with `pyo3-log`
and `log` but as soon as we pass a thread boundary, everything is logged
against the `sentinel` log context. Normally, we want logs and CPU/DB
usage correlated with the request that spawned the work.
You can see how I took a stab at fixing this in
https://github.com/element-hq/synapse/pull/19846 by capturing the
logcontext in a Tokio task local and re-activating as necessary. For
example, in that PR, I reactivated the logcontext in
`run_python_awaitable(...)` which we use to call `runInteraction(...)`
from the Rust side which means all of the database usage is correlated
with the request as expected. It also means any `log:info!(...)` done in
`run_interaction(...)` is correlated correctly. But there needs to be a
better story for when you want to log everywhere else.
I haven't explored tracking CPU usage on the Rust side.
I've left all of this out of this PR as I think it will be better to
tackle this as a dedicated follow-up. For example, I'm thinking about
instead creating a new `LoggingContext` with the `parent_context` set to
the calling context and try to avoid needing to call
`set_current_context(...)` on the Python side where possible (like
tracking CPU).
### Testing strategy
Added some tests that exercise some `async` Rust handlers for the
`/versions` endpoint:
```
SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.rest.client.test_versions.VersionsTestCase
```
Real-world:
1. `poetry run synapse_homeserver --config-path homeserver.yaml`
1. `GET http://localhost:8008/_matrix/client/versions`
The port of event serialization to Rust (#19837) removed
`format_event_raw`, `format_event_for_client_v1`,
`format_event_for_client_v2` and
`format_event_for_client_v2_without_room_id` from
`synapse.events.utils`, but there are modules in the wild that import
them from there.
Reimplement them as standalone pyfunctions in Rust, operating directly
on the Python dict so the original semantics are preserved exactly
(in-place mutation, returning the same dict, arbitrary non-JSON values
passing through, KeyError on a missing `unsigned` in the v1 format), and
re-export them from `synapse.events.utils`.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
### Summary
This moves the synchronous core of client event serialization out of
`synapse/events/utils.py` and into Rust
(`rust/src/events/serialize.rs`).
Event serialization is on the hot path for `/sync`, `/messages`, and
most client-facing endpoints. Previously it was a recursive pure-Python
routine (`_serialize_event` / `_inject_bundled_aggregations` /
`only_fields`) that interleaved
CPU-bound formatting with `async` DB/IO. This PR separates those two
concerns and moves the CPU-bound half to Rust:
- **Async prep stays in Python.**
`EventClientSerializer._prepare_serialization` does all DB/IO up front:
batch-fetching redaction events and running the registered module
`unsigned`-callback hooks, for the top-level events *and* every bundled
sub-event (edits and thread latest events, which are themselves
serialized). The admin/MSC4354 config is resolved once via
`_update_config`, rather than re-checked on every recursive call as the
old code did.
- **The synchronous core moves to Rust.** Given an event plus the
pre-fetched `redaction_map`, `unsigned_additions`, and
`bundle_aggregations`, the Rust code produces the client JSON entirely
in Rust — including the v1/v2 format transforms, `only_event_fields`
filtering, redaction handling, and recursive bundled aggregations.
### Details
- The Rust entry point is a single batch function, `serialize_events`,
taking a list of `(event, membership)` pairs. The three lookup maps are
shared across the whole batch, so they're read out of Python and
converted to Rust
structures **once** per batch rather than once per event.
`EventClientSerializer.serialize_event` (singular) is a thin wrapper
that calls it with a one-element list.
- `SerializeEventConfig` is now a Rust `pyclass`, and the old
`event_format` callable is replaced by the `EventFormat` enum (`Raw` /
`ClientV1` / `ClientV2` / `ClientV2WithoutRoomId`). Call sites in
`rest/admin/events.py` and
`rest/client/{notifications,room,sync}.py` are updated to pass the enum.
`make_config_for_admin` and MSC4354 enablement now go through
`SerializeEventConfig.for_admin()` / `with_msc4354()`.
- New accessors on `EventInternalMetadata` (`redacted_by`, `txn_id`,
`device_id`, `token_id`, `delay_id`, `soft_failed`,
`policy_server_spammy`) expose to Rust the fields the serializer reads.
- The `_split_field` unit tests move from `tests/events/test_utils.py`
to a Rust test in `serialize.rs`, since the implementation moved.
### Behaviour
This is intended to be a behaviour-preserving refactor — the Rust core
mirrors the previous Python output (field ordering, v1 key promotion,
redaction placement per room version, null-`redacts` handling,
transaction-ID gating).
Existing serialization, relations, and sync tests pass unchanged.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Co-authored-by: Eric Eastwood <erice@element.io>
This means you can use `get_success(...)` anywhere regardless
of what kind of work needs to be done.
Spawning from adding some more async Rust things in
https://github.com/element-hq/synapse/pull/19846 and wanting something
more standard instead of the custom `till_deferred_has_result(...)` that
has crept in to a few files.
Alternative to https://github.com/element-hq/synapse/pull/19867 spurred
on by [this
comment](https://github.com/element-hq/synapse/pull/19867#discussion_r3441774685)
from @erikjohnston
### How does this work?
Previously, `get_success(...)` just ran in a hot-loop advancing the
Twisted reactor clock which didn't give any time for other threads to do
some work or acquire the GIL if necessary (whenever there is a hand-off
from Rust to Python, we need the GIL).
Now, `get_success(...)` loops until we see a result (until we hit the
~0.1s real-time timeout). In the loop, we call
[`time.sleep(0)`](https://docs.python.org/3/library/time.html#time.sleep)
which will "Suspend execution of the calling thread [...]" (CPU and GIL)
to allow other threads to do some work. Then like before, we advance the
Twisted reactor clock to run any scheduled callbacks which includes
anything the other threads may have scheduled.
### Does this slow down the entire test suite?
Seems just as fast as before. There is minutes variance in what we had
before and after but both are within the same range of each other.
(see PR for actual before/after timings)
Introduced in: #17847
This 10-second wall-clock timeout was troublesome as it fails flakily on
slow/struggling CI runners, like the
default ones for private GitHub repositories.
The loop also silently relied on the reactor advance in `make_request`,
whereas we could just deterministically advance the reactor the known
amount of times
instead.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Change `/org.matrix.msc3814.v1/dehydrated_device/[device_id]/events` to
accept GET requests instead of POST.
The original version of
[MSC3814](https://github.com/matrix-org/matrix-spec-proposals/pull/3814)
said we should delete keys after returning them from this endpoint, but
it is being updated to say we should not delete them, and therefore the
appropriate verb is GET.
Synapse already doesn't delete anything, so we just need to change to a
GET with a `next_batch` query param. (Currently it is a POST with
`next_batch` in the JSON content.)
This code was initially written by @ara4n and Claude, but both he and I
have read it and think it makes sense. I am far from a Synapse expert,
so feel free to tell me it's all wrong and point me in the right
direction.
I don't know what system tests will be affected by this, but I guess we
will see when the CI runs (right?).
This is a change to an unstable endpoint so no need for notifications
about breaking changes or similar.
Part of https://github.com/element-hq/element-meta/issues/2704
### Pull Request Checklist
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Matthew Hodgson <matthew@matrix.org>
This reports the total count of users (split by appservice) which is
meant to be the monthless counterpart to the MAU metric.
Context:
> So this is largely for billing purposes and wanting to know the change
in the number of users. If a user is deactivated then we no longer want
to count them. Consumers *might* want to count appservice users, and
maybe count them based on the service (perhaps you change more for users
under bridge X or bridge Y).
>
> *-- https://github.com/element-hq/synapse/pull/19848#discussion_r3402216234*
Follows: #19487
Part of: MSC4354 whose experimental feature tracking issue is #19409
This PR implements the Sliding Sync (MSC4186) extension described in
MSC4354, allowing sliding sync clients
to receive sticky events in a reliable way.
The logic is much the same as for oldschool sync (implementation in
#19487),
although in the sliding sync extension, the client can choose their own
limit
and must control their own pagination through an extra token in the
extension request/response bodies.
Note this does not yet send down existing sticky events in the
room when the room has been newly-joined.
This newly-discovered gap is tracked at #19662 and will be addressed for
both current sync and MSC4186 SSS soon.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
Not aware of an open ticket for this.
I came across it when I accidentally broke the feature even more (as
part of another piece of work),
then discovered there weren't tests for this.
So this is overall a low-priority drive-by fix.
Requires a fix to SyTest https://github.com/matrix-org/sytest/pull/1426
(as it depended on the bug).
<ol>
<li>
Add a test for purging rooms with `delete_local_events=False` \
Parameterised by room version, this test currently succeeds
on v2 but fails on v12.
This is because the condition checking for local events relies
on the old event ID format, which has not been used since v2.
</li>
<li>
Fix delete_local_events=False for room versions above v2 \
The event ID format changing means that we have to rely on `sender`
to know the origin of an event
</li>
</ol>
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Fixes: #19844
Concretely, this changes `ResponseCache` to unset cache entries once
they resolve to a `Failure`.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
The sytest `After /purge_history users still get pushed for new
messages` is flaky. The flakiness exposes a real bug rather than a
test-timing issue.
Notification counts are stored in two places: `event_push_actions` (one
row per unread event) and `event_push_summary` (aggregate counts
populated periodically by `_rotate_notifs`, which runs on a 30-second
timer). `_purge_history_txn` deletes the purged events' rows from
`event_push_actions` but never adjusts `event_push_summary` (only the
full-room `purge_room` drops that table).
So the result depends on a race: if rotation hasn't fired, counts come
live from `event_push_actions`, the purge removes the right rows, and
the count is correct. If rotation fires before the purge — more likely
under the slower
multi-postgres/workers/asyncio CI config — the events get folded into
`event_push_summary`, the purge then deletes the underlying
`event_push_actions` rows but leaves the summary untouched, and the
count comes out inflated.
### Fix
Before deleting the rotated rows from `event_push_actions`, decrement
`event_push_summary` by the amount attributable to the events being
deleted. The decrement mirrors the counting logic in
`_rotate_notifs_before_txn`: only rows that were already rotated
(`stream_ordering <= event_push_summary_stream_ordering`) and that fall
after the summary's receipt are subtracted, so it stays correct in the
presence of read receipts and unread/highlight rows. The SQL avoids
`UPDATE ... FROM` and CTEs so it works on both SQLite and Postgres.
End-of-purge cache invalidation already covers
`get_unread_event_push_actions_by_room_for_user`.
### Tests
Adds `test_count_aggregation_after_purge`, which forces a rotation
before purging and asserts the aggregate count reflects only the
surviving events, covering read receipts and a subsequent re-rotation.
It fails (`3 != 1`) without the fix.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
I.e. `default_config("test")` and `default_config("test", False)` are so
opaque and hard to connect the dots with.
Spawning from trying to figure out what the `server_name` is set as for
our `HomeserverTestCase` in order to reference it in
https://github.com/element-hq/synapse/pull/19848#discussion_r3397455309
This was all done through claude.
## Fix flaky `test_edu_large_messages_not_splitting_one_user` (`TooLong`
under `trial -jN`)
### Problem
The "Build .deb packages" CI step intermittently failed with:
```
twisted.protocols.amp.TooLong
...in
tests.rest.client.test_sendtodevice.SendToDeviceTestCase.test_edu_large_messages_not_splitting_one_user
```
The deb build runs the suite with `twisted.trial -j2`. In that mode,
worker log events are shipped to the manager process over Twisted's AMP
protocol, which encodes each value with a 2-byte length prefix — so any
single log line of **64 KiB or more** raises `TooLong`.
### Root cause
Not a bug in the to-device/EDU logic, and **not** debug logging enabled
by the build (it runs at the default `ERROR` level). It's a **log-level
leak between tests sharing a `-j2` worker**:
- `tests/logging/test_loggers.py::ExplicitlyConfiguredLoggerTestCase`
calls `root_logger.setLevel(logging.DEBUG)` directly and never restores
it (no `setUp`/`tearDown`/`addCleanup`).
- When that test runs before
`test_edu_large_messages_not_splitting_one_user` **in the same worker
process**, the root logger is left at `DEBUG`.
- That test deliberately builds an EDU of exactly `SOFT_MAX_EDU_SIZE -
1` (65 535) bytes. Storing it triggers `synapse/storage/database.py`'s
`[SQL values]` DEBUG log, which dumps the full query params — producing
a **65 708-byte**
line that overflows AMP's cap.
It looks "flaky" purely because of `-j2` scheduling: whether the two
tests land on the same worker, and in what order.
### Fix
Three commits:
1. **Restore the root logger level in
`ExplicitlyConfiguredLoggerTestCase`** — `addCleanup` to put the level
back. Fixes the root cause.
2. **Truncate oversized log lines in the test log handler**
(`ToTwistedHandler.emit`) — caps lines at 1000 chars so no debug line
can break `trial -jN`, regardless of which query is logged (defense in
depth).
3. **Truncate values in `[SQL values]` debug logging** — caps the logged
param repr at 1000 chars (guarded by `isEnabledFor(DEBUG)` to keep the
hot path lazy). Keeps production debug logs sane too.
### Testing
- The reproduction (`trial -j2 tests.logging.test_loggers <the EDU
test>`) went from **~3/8 failing** to **10/10 passing**.
- Confirmed the root level is restored after the logging tests, and that
the `[SQL values]` line is now capped (~50 KB with a `[truncated]`
marker, was 65 708).
- `ruff` + `mypy` clean; `tests.logging.test_loggers` and
`tests.rest.client.test_sendtodevice` pass (14/14).
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Eric Eastwood <erice@element.io>
This is in prep for converting the event serialization to Rust.
This is a fairly mechanical port, except that we store the appservice ID
rather than the appservice object. This avoids us having to store a
`Py<..>` (or port the appservice object over).
Fix#2860
Also cleans up comments around plans to define `ALLOWED_SCHEMES` as we can rely on Bleach's `ALLOWED_PROTOCOLS` defaults (`http`, `https` and `mailto`).
Follow on from https://github.com/element-hq/synapse/pull/19701.
Unfortunately serde has a bug when using `#[serde(flatten)]` with
`arbitrary-precision` feature when handling integers that fit in a i128
when doing `serde_json::from_value`. See
https://github.com/serde-rs/serde/issues/2230.
The `depythonize` hits the same issue. To fix this we make it so we only
parse events from strings and not values.
This is based on https://github.com/element-hq/synapse/pull/18416, which
got reverted (#19614) due to it incorrectly rejecting to-device messages
to users with many devices (and thus breaking message sending).
Fix https://github.com/element-hq/synapse/issues/17035
A to-device message content looks like:
```jsonc
{
"@user:domain": {"device1": {...}, "device2": {...}},
...
}
```
The previous PR would split up into multiple EDUs, each with a subset of
the users. However, if one user's entry was too large it would not
further split it up and then error out.
The main change in this PR is to allow splitting up a single user into
multiple EDUs.
Other changes:
1. Rename to `SOFT_MAX_EDU_SIZE` to indicate that we sometimes send EDUs
with larger size than that, and its more a target than a hard limit.
2. Check early if any to-device message (to a specific device) is too
large to send, even if we're not going to send it over federation. This
ensures that we catch issues where clients try to send too large
to-device.
This still means that if a client send a large individual to-device
message it will fail, but I don't believe we ever send such large
to-device messages (normally they're in the range of a few KB).
---
I ended up changing the implementation a bunch to make it easy to reuse
the code to split up dictionaries. Instead of repeatedly splitting up
the EDU until each bit fits into the size, we instead record the size of
each entry in the dict and instead split up based on cumulative size.
This means we call `encode_canonical_json` on each entry rather than
once on the entire struct, but its not significantly slower to do so.
--
cc @MatMaul @MadLittleMods
---------
Co-authored-by: Mathieu Velten <matmaul@gmail.com>
Co-authored-by: mcalinghee <mcalinghee.dev@gmail.com>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Ports the event class to Rust.
The main difference here are:
1. There is now a single event class
2. We now validate a lot more at event construction time than we
previously did (we basically checked nothing before). This required some
changes to the tests, including
https://github.com/matrix-org/sytest/pull/1423
Reviewable commit-by-commit.
### Overview of Event Rust structure
The format of the event struct in Rust is quite different than that in
Python.
The top-level looks like:
```rust
pub struct Event {
/// The parsed event JSON.
fields: FormattedEvent,
/// The event ID. For format v1 this is read directly from the JSON;
/// for v2+ it is computed from the canonical-JSON hash at
/// construction time and cached here.
event_id: Arc<str>,
/// Synapse-internal per-event state that lives outside the federated
/// JSON (e.g. outlier flag, soft-failure, stream positions).
#[pyo3(get)]
internal_metadata: EventInternalMetadata,
/// The room version this event was parsed for.
#[pyo3(get)]
room_version: &'static RoomVersion,
/// `None` for accepted events; otherwise a short reason set by auth
/// when the event was rejected.
rejected_reason: Option<Box<str>>,
}
```
which includes the actual parsed event in `FormattedEvent`, plus the
rest of the event metadata.
```rust
pub struct FormattedEvent<E = Arc<EventFormatEnum>> {
#[serde(default)]
pub signatures: Signatures,
#[serde(default)]
pub unsigned: Unsigned,
#[serde(flatten)]
pub specific_fields: E,
#[serde(flatten)]
pub common_fields: Arc<EventCommonFields>,
}
```
The struct is further split into the common fields, format specific
fields, plus the signatures and unsigned. We split out the signature and
unsigned fields as they are mutable, so when we clone the event we can
still share the common and specific fields and only copy signature and
unsigned.
The `specific_fields` are the fields that depend on the format version.
They can either be a specific format (e.g. `E = EventFormatV1`) or a
type-erased enum `EventFormatEnum` that is across all room versions:
```rust
pub enum EventFormatEnum {
V1(EventFormatV1),
V2V3(EventFormatV2V3),
V4(EventFormatV4),
VMSC4242(EventFormatVMSC4242),
}
```
For example:
```rust
/// Shared flat-list encoding of `auth_events` and `prev_events`, reused
/// by every format from v2/v3 onwards.
#[derive(Serialize, Deserialize)]
pub struct SimpleAuthPrevEvents {
pub auth_events: Vec<String>,
pub prev_events: Vec<String>,
}
/// Version-specific fields for room versions 3-10.
#[derive(Serialize, Deserialize)]
pub struct EventFormatV2V3 {
pub room_id: Box<str>,
#[serde(flatten)]
pub auth_prev_events: SimpleAuthPrevEvents,
}
```
### Dev notes
As discussed in
[`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$3gTjDO440GbAz57cXcCawwiyFLiD0crrarvS1uhzKOY?via=jki.re&via=element.io&via=matrix.org)
---------
Co-authored-by: Eric Eastwood <erice@element.io>
As per the spec, a room with m.room.name value that is absent, null or
empty should be treated as if there is no m.room.name event at all:
https://spec.matrix.org/v1.17/client-server-api/#mroomname
This fetches the full m.room.name event and checks the content.name
instead of only checking the existence of the m.room.name event. This
results in correctly sending heroes for those rooms.
Fixes: https://github.com/element-hq/synapse/issues/19447
Signed-off-by: Joe Groocock <me@frebib.net>
when an access token had a refresh token associated to it in the
database, deleting this refresh token (for example when deleting the
device using it) would cascade delete the access token, which wouldn't
be returned by the sql query that was supposed to delete it on its own,
and an empty array was passed to the cache invalidation function.
Fixes: #19689
# What
This PR fixes a bug I found when I run synapse (from dockerhub) and
register a `check_event_allowed` callback and my client makes use of the
mentions field in messages (`cinny:latest`). The bug doesn't appear when
the `check_event_allowed` callback is not loaded.
After some digging I noticed that the current validation of the mentions
doesn't work when an event has been frozen with `event.freeze()`. For
the messages this seems to happen when a the `check_event_allowed` is
registered (but not otherwise), see [where the event is frozen for
check_event_allowed
callback](https://github.com/element-hq/synapse/blob/b0fc0b7a612a42e6f15b87dee2a1db4c383645fb/synapse/module_api/callbacks/third_party_event_rules_callbacks.py#L289)
and [where the validation function is
called](https://github.com/element-hq/synapse/blob/b0fc0b7a612a42e6f15b87dee2a1db4c383645fb/synapse/handlers/message.py#L1404).
To have a minimal reproduction example, the following scripts fails on
`develop` but succeeds in this branch:
``` python
from synapse.api.room_versions import RoomVersions
from synapse.events import EventBase, make_event_from_dict
from synapse.events.validator import EventValidator
from tests.utils import default_config
def make_message_event(content: dict) -> EventBase:
return make_event_from_dict(
{
"room_id": "!room:test",
"type": "m.room.message",
"sender": "@alice:test",
"content": content,
"auth_events": [],
"prev_events": [],
"hashes": {"sha256": "aGVsbG8="},
"signatures": {},
"depth": 1,
"origin_server_ts": 1000,
},
room_version=RoomVersions.V9,
)
event = make_message_event(
{
"msgtype": "m.text",
"body": "@moderator:example.com hello",
"m.mentions": {"user_ids": ["@moderator:jailbreak-challenge.aqtiveguard.com"]},
}
)
EventValidator().validate_new(event, default_config) # Ok
event.freeze()
EventValidator().validate_new(event, default_config) # throws
# pydantic_core._pydantic_core.ValidationError: 1 validation error for Mentions
# Input should be a valid dictionary or instance of Mentions [type=model_type, input_value=immutabledict({'user_ids'...nge.aqtiveguard.com',)}), input_type=immutabledict]
# For further information visit https://errors.pydantic.dev/2.12/v/model_type
```
# How
I made the validation logic also validate the transformation performed
by the freezing process, namely:
- `immutabledict` validates as `dict`. (was already implemented for
POWER_LEVELS)
- `tuple` validates as array (added this to the validator in this PR).
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Co-authored-by: Olivier 'reivilibre <oliverw@matrix.org>
When we port the `Event` class to Rust, the constructor will check for
the existence of required fields. To support that, we tidy up the test
code where we construct fake events to add all the required fields.
There should be no behavioural changes.
Review commit-by-commit.
