This fixes the test `ThreadedMemoryReactorClock` to support
`removeSystemEventTrigger`, which in turn allows the shutdown triggers
to run when `HomeServer.shutdown()` is called in the tests. Previously,
`ThreadedMemoryReactorClock.removeSystemEventTrigger` raised a
`NotImplementedError`,
`cleanup_test_reactor_system_event_triggers` cleared the triggers by
hand so that a homeserver could be garbage collected. `shutdown()` now
removes them itself, so the helper goes.
We can also remove all the `tearDown` implementations that tried to
manually trigger the shutdown as its not longer necessary to do so
manually. They also didn't work, as they indexed `reactor.triggers` as
`[eventType][phase]` when the keys are `[phase][eventType]`
Follows on from #20289 and #20433. Properly fixes handling of concurrent
requests (on single workers).
This tries to fix two cases:
1. We are *computing* responses for multiple syncs at the same time,
which currently causes the cache to get cross-contaminated.
2. Two syncs concurrent syncs on the wire (but don't overlap during
processing). When the second request is processed we correctly forget
any entries from the first request, however we currently still update
the cache with members returned on the second request. However, there is
no guarantee that the second request is what the client actually reads.
We use three rules to handle concurrent requests:
1. A response only uses cached members recorded at or before the
request's `since`. Members recorded after it cannot be from an ancestor
sync response.
2. Once a response has been built, we forget all already cached members
recorded after its `since`. This handles the client reusing an older
`since` token. Doing it at the end of the build rather than the start
also forgets members recorded by responses built at the same time.
3. We track the highest position seen for the device (`_newest`), from
the `since` of each request and the `next_batch` of each built response.
A response only records its members if its `next_batch` is after that
position. This handles responses that end at the same position, or that
finish out of order.
With these rules, when concurrent requests happen we throw away all
existing cached members and don't record any further members until one
sync has advanced sufficiently such that there are no longer any
concurrent sync requests.
Note that ideally we'd have some sort of per-connection identity so that
we can track this properly (like in sliding sync). However, that is a
bigger change. Instead, we note that we just need to detect concurrent
requests, and not update the cache in those cases.
Also note, can only detect concurrent requests that go to the same
worker.
Follows: #18375
This bugfix is a prerequisite for MSC4354 (Sticky Events, Experimental
Feature tracking issue #19409) changes.
- Add a failing test for join->join being newly_joined
- Fix the join->join transition being treated as a new join
(when prev_events didn't coincidentally mask the problem)
Rationale:
`event_edges` / `prev_events` does NOT link a membership event
to its previous state.
(It could even be someone else's membership!)
In most real-world cases, this would generate a `prev_membership` of
NULL
for every normal membership change.
It appears as though the author intended some kind of 'previous state'
relationship, but if you fix the first branch of the `UNION` query
to use `current_state_delta_stream` for this (which is the source of
state transitions), then it looks very redundant with the second
branch.
It looks like the real reason we want to use the snapshots table is for
out-of-band memberships (outlier memberships).
So we continue using that for those, but ONLY for those.
The `SUSPICIOUS` if block is removed because it appears to have been
papering over this issue by deduplicating the spurious `NULL` ->
non-null transitions,
but when you remove those duplicates at the source, it causes more
trouble
because it doesn't know that what it's 'deduplicating' is actually the
same membership transition.
This means if you join, then out-of-band leave (e.g. last member on the
server
to leave), that room would appear in `newly_joined` on a new initial
sync.
(Tests did in fact catch this, too.)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Closes#20116
> [!NOTE]
> This PR Follows #20115 which already implemented this for
`m.device_list_update`
> User IDs with localparts containing characters outside the range
U+0021 to U+007E, or with
> an empty localpart, are considered non-compliant. For current room
versions, servers must
> still accept events using such user IDs over federation; however they
SHOULD NOT forward
> such user IDs to clients when referenced outside the context of an
event. For example,
> device list updates from non-compliant user IDs would be dropped by
the receiving server.
>
> — [Spec
1.19](https://spec.matrix.org/v1.19/appendices/#historical-user-ids)
This PR prevents from forwarding non compliant user ids for the
following events:
- `m.signing_key_update` 8c3c35e95d
- `m.presence` 26a8dfe862
- `m.typing` 1110cf2d73
- `m.receipt` 9e2da8b0a6
- `m.direct_to_device` 9541232a30
Fixes https://github.com/element-hq/synapse/issues/18793
When the last local user leaves a room, Synapse clears
`current_state_events` and writes `null` `event_id` entries into the
`current_state_delta_stream`. The incremental sync code for MSC4222's
state_after skipped those `null` deltas entirely, and
`get_current_state_ids` also returned nothing (table already cleared).
Result: clients using `state_after` never learned about the leave.
Fix by falling back to state groups (which are preserved) to look up the
actual state at `end_token` for any cleared state keys.
### 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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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: Paul Chobert <paul@chobert.fr>
Co-authored-by: Quentin Gliech <quenting@element.io>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Several clients, including Element Web/Desktop and Element X, have
already implemented
[MSC4426](https://github.com/matrix-org/matrix-spec-proposals/pull/4426)
User Status profile fields. However, we have only implemented sending
profile fields down sync for local users
([MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429)/[MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262)),
since federation support for pushing profile field updates from server
to server has not yet been implemented
([MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259)).
This has created user confusion. The before mentioned clients use the
profile and/or profile field fetch endpoints to fetch missing profile
information, regardless of the user being local or remote. Since the
sync response isn't delivering these profiles, the client does a
fallback fetch, causing a one time update of the profile information
over federation.
This causes stale data to be cached in the client for the remote user,
which in the case of EW for example is never cleared.
This filtering should be temporary until MSC4259 is implemented and the
remote profile data also syncs via the profile update stream to clients.
I suggest making this not configurable so we don't have to maintain a
configuration property, as hopefully this would only be a case for a few
months at most.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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: Eric Eastwood <madlittlemods@gmail.com>
ESS deployments of MatrixRTC are going to run `lk-jwt-service` as an
application service so that it can manage delayed events. However,
setting up an application service is particularly inefficient, as
Synapse iterates through all the events of the rooms it covers since
their creation.
When an application service is configured with `url: null`, there is no
need to traverse all the events for it, so we can set up such
application services much faster without having to decide what to do for
other application services. Discussions about those are still open:
element-hq/synapse#11629 (see also matrix-org/synapse#1834).
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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))
In `/sync` there is a mode called lazy loaded (LL) members. In this mode
`/sync` does not return all memberships in the room, instead it only
returns a) membership changes, and b) the memberships of senders of
events in the timeline (if not previously sent).
Synapse implements a lazy-loaded member cache to avoid repeatedly
sending down the same memberships when a single user sends multiple
events. It does this by storing for each user/device the memberships it
has previously returned in an LRU cache. Note that it is fine for the
server to return the same membership, clients handle that fine, but it
is a waste of bandwidth.
The current lazy-loaded membership cache has three intertwined flaws:
1. It does not handle retries or clients re-using old tokens.
2. It incorrectly applies the membership cache to membership changes,
not just memberships added by lazy-loading.
3. It does not correctly handle multiple "sync streams" from the same
device.
In this PR we explicitly fix the first two issues in two commits, and
the third issue is mitigated:
1. Alongside each membership in the LL cache we store the `next_batch`
position of the response that carried it. A client that processed a
response syncs from its `next_batch` next. When a request arrives with a
`since` token, we delete every membership recorded after that token, as
they were sent in responses the client never processed. Memberships
recorded at or before `since` are kept.
2. We always return memberships that have changed, even if they appear
in the LL member cache.
These changes also half mitigate the issue of running multiple `/sync`
loops per-device (though we do not see this in the wild), as we'd at
least always return the changed membership states.
Fixes#19978 and #20278, supersedes #20279
This pull request fixes a few issues with the profile update stream,
when a user leaves a room. This is basically just mimicking what we
already had for when someone leaves a room, and they no longer share any
rooms, but in reverse - ie when we leave a room, and no longer share
rooms with some users. This was missed in the implementation when adding
the profile update stream for legacy and sliding sync.
[Fix missing profile update stream rows on user leaving
room](https://github.com/element-hq/synapse/commit/e953322226731a99718ebe13139b9c5f05ae4b62)
When a user leaves a room, profile update stream rows are generated with
the `LEFT_ROOM` action for each user in the room that no longer shares a
room with the user who left the room.
This also needs to happen in reverse. The user who left the room needs
to have a profile update stream row for each user they no longer share a
room with.
If we don't do this, clients may keep stale data around even after they
don't share a room with a user, which may mean they don't know when to
refetch profiles after re-joining a room with the stale profile data
user.
[Clear out old profile update stream rows when leaving a
room](https://github.com/element-hq/synapse/commit/e7642bf141b098c1eefaf74655e5e7e18415500b)
When we leave a room, ensure profile update stream rows are cleared out
for every user we no longer share a room with. This is the same as what
happens when someone else leaves a room, but in reverse. The only
remaining profile update stream row should be the `LEFT_ROOM` action.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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))
Previously the tokio runtime was stashed in a hidden attribute on the
reactor object, installed lazily by whichever Rust code first needed it,
and started via `callWhenRunning`.
Instead, we create a `RustRuntime` (accessible via
`HomeServer.get_rust_runtime()`) that holds any per-reactor Rust state,
such as the tokio runtime. It is constructed lazily on use. Rust
consumers (`HttpClient`, `VersionsHandler`, the Python DB pool wrapper)
now receive the runtime or reactor handle explicitly, and the
`reactor.run()` / manual-startup workarounds in tests are no longer
needed.
We also add helper wrappers in Rust for `Reactor` and `HomeServer` that
exposes the needed functionality.
The aim is to allow us to have a Rust-side clock (mainly to get the
current time), that respects the unit test per-reactor time management.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports the logcontext classes to Rust, and gives tokio tasks a captured
logcontext so that work running in (or spawned from) Rust is attributed
to the request that caused it.
1. **Add characterization tests for logcontext error messages and the
filter** — pins the exact `logcontext_error` message shapes, the
abuse-detection code paths and `LoggingContextFilter`'s observable
behaviour, *against the existing Python implementation* (this commit is
green on its own). These are the behavioural contract the port has to
satisfy.
2. **Port `ContextResourceUsage` to a Rust pyclass** — self-contained:
the new `synapse_rust.logcontext` module, the class, its stub and the
re-export.
3. **Move the logcontext storage and `LoggingContext` to Rust** — the
core change; the commit message carries detailed design notes.
Highlights:
- The slot is typed `Option<Py<LoggingContext>>`, with `None`
representing the sentinel. `_Sentinel`/`SENTINEL_CONTEXT` stay pure
Python (unchanged); thin wrappers on
`current_context`/`set_current_context` convert at the boundary, and
pyo3's extraction enforces the type (`TypeError` otherwise).
- The accounting is native: one `getrusage(RUSAGE_THREAD)` read per
switch via libc, inline `stop`/`start` bookkeeping for base
`LoggingContext`s, Python dispatch only for subclasses
(`BackgroundProcessLoggingContext`) so their overrides run. The thread
id comes from `PyThread_get_thread_ident` (the exact
`threading.get_ident()`
value) without calling into Python.
- The hot paths avoid per-operation allocation: names are `Py<PyString>`
(the per-log-record `str(context)`/`server_name` reads are INCREF-only),
error branches materialise strings only when hit.
4. **Attribute Rust-spawned work to the caller's logcontext** —
`create_deferred` captures the caller's context and scopes it onto the
spawned task via a tokio task-local (`LogContextHandle`);
`current_context()` gives the task-local read precedence, so
`LoggingContextFilter`/`pyo3-log` resolve the right context on worker
threads with no per-record stamping. `run_python_awaitable` restores the
captured context (via a `with_logcontext` helper, the Rust
`PreserveLoggingContext`) around Python called back from Rust, so e.g.
`runInteraction` from the Rust `/versions` handler accounts its DB usage
against the right request. Integration tests exercise both guarantees
through real production code paths.
Follow-up work on top of this (separate PR): porting
`BackgroundProcessLoggingContext` natively and removing further `Py<_>`
indirections. The fact that `BackgroundProcessLoggingContext` is a
subclass is what forces some of the warts in this PR: e.g. having to use
`Py<LoggingContext>` everywhere, etc.
We don't try (yet) to make this pure Rust, instead we see this as simply
maintaining the Python logcontext machinery when crossing, rather than
trying to make a Rust equivalent that can be used by pure Rust
dependencies. We probably do want to do that in future, as well as wire
up e.g. CPU recording on Rust side, but that is unnecessary for now.
Part of #18731 (Support Matrix 1.15).
Since Matrix 1.15, the
[spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv1roomsroomidhierarchy)
defines `allowed_room_ids` on the room summaries returned by `GET
/_matrix/client/v1/rooms/{roomId}/hierarchy`, so that clients can tell
whether a restricted room can be joined or only knocked at:
> **`allowed_room_ids`** — `[Room ID]` — If the room is a [restricted
room](https://spec.matrix.org/v1.19/client-server-api/#restricted-rooms),
these are the room IDs which are specified by the join rules. Empty or
omitted otherwise. Added in `v1.15`
>
> — [Matrix
Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv1roomsroomidhierarchy)
Synapse currently strips the field from the client `/hierarchy` response
before returning it — a guard added in matrix-org/synapse#12175 (2022),
correct at the time, when the spec defined the field for federation only
and it was leaking into client responses. The other two surfaces that
define the field (`/room_summary` and the federation `/hierarchy`)
already return it.
This PR removes the strip in `_RoomEntry.as_json` (and the now-unused
`for_client` parameter), so client hierarchy entries include
`allowed_room_ids` for restricted rooms, both local and received over
federation. The first commit adds the failing test coverage, the second
removes the strip.
---
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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))
Instead, treat them as absent fields as they feel like they should be.
The database implementation detail that these fields have a dedicated
column with `NULL`
when unset is kept to the storage layer.
The goal here is to reduce the amount of special casing needed for these
two original profile fields and treat them a little bit more like
regular profile fields.
Follows: #20003
Follows: #20147 (needed as a bugfix to continue sending them down
oldschool sync when they get deleted. Without #20147, this PR would
break that — which matches how custom profile fields were broken too.)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Application services that opt in to receiving ephemeral events
(`receive_ephemeral: true` in the
[registration](https://spec.matrix.org/v1.19/application-service-api/#registration),
added in Matrix v1.13 from
[MSC2409](https://github.com/matrix-org/matrix-spec-proposals/pull/2409))
are sent the private read receipts (`m.read.private`) of **every** user
in the rooms they are interested in — not just their own users.
The [Application Service
API](https://spec.matrix.org/v1.19/application-service-api/#pushing-ephemeral-data)
is explicit about this (*Pushing ephemeral data*, `m.receipt`):
> Private read receipts MUST only be sent for users matching one of the
application service's namespaces. Normal read receipts and threaded read
receipts are always sent.
This matches the [Client-Server
API](https://spec.matrix.org/v1.19/client-server-api/#private-read-receipts):
"Servers MUST NOT send the `m.read.private` receipt to any other user
than the one which originally sent it."
The restriction to namespaced users is safe because the appservice could
learn those receipts anyway by syncing as the user. For everyone else,
`m.read.private` exists precisely so that nobody — including bridges and
bots in the room — can observe it.
For example, take an appservice registered with:
```yaml
namespaces:
users:
- regex: "@_bridge_.*:example\\.org"
exclusive: true
```
When `@alice:example.org` (a regular user, not one of the appservice's)
and `@_bridge_bob:example.org` (a namespaced user) each send read
receipts in a bridged room, the appservice receives:
```json
{
"type": "m.receipt",
"room_id": "!room:example.org",
"content": {
"$event": {
"m.read": { "@alice:example.org": { "ts": 1436451550453 } },
"m.read.private": {
"@_bridge_bob:example.org": { "ts": 1436451550453 },
"@alice:example.org": { "ts": 1436451550453 }
}
}
}
}
```
- `m.read` from `@alice` — correct, public read receipts are always
sent.
- `m.read.private` from `@_bridge_bob` — correct, the user is within the
appservice's namespaces.
- `m.read.private` from `@alice` — **the leak**: their private read
receipt must not be sent to the appservice.
## History
- matrix-org/synapse#8437 (Oct 2020, Synapse 1.22.0) implemented MSC2409
ephemeral event delivery to appservices. No leak at that point: private
read receipts did not exist yet.
- matrix-org/synapse#10413 (Jul 2021, Synapse 1.40.0) added the initial,
experimental-flag-gated implementation of MSC2285 ("hidden" read
receipts) and filtered them out of the `/sync` path
(`filter_out_hidden`) — but not out of the appservice path in the same
file. This is where the leak originates, for servers with
`msc2285_enabled`.
- matrix-org/synapse#12168 (May 2022, Synapse 1.59.0) reworked this into
the `m.read.private` receipt type and `filter_out_private_receipts`; the
appservice path was again left unfiltered.
- matrix-org/synapse#13273 (Aug 2022, Synapse 1.65.0) moved to the
stable `m.read.private` identifier; the appservice path has leaked it
ever since.
---
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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))
Part of: https://github.com/element-hq/synapse/issues/19415
Return `M_APPSERVICE_LOGIN_UNSUPPORTED` error code instead of the
unstable `IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED` identifier.
> Servers MUST still allow application services to use the `/register`
endpoint with a login type of `m.login.application_service` even if they
don't support the Legacy Authentication API. In that case application
services MUST set the `"inhibit_login": true` parameter as they cannot
use it to log in as users. If the `inhibit_login` parameter is not set
to `true`, the server MUST return a 400 HTTP status code with an
`M_APPSERVICE_LOGIN_UNSUPPORTED` error code.
>
> [...]
>
> Application services MUST NOT use the `/login` endpoint if the server
doesn't support the Legacy authentication API. If `/login` is called
with the `m.login.application_service` login type the server MUST return
a 400 HTTP status code with an `M_APPSERVICE_LOGIN_UNSUPPORTED` error
code.
>
> — [Matrix v1.19, Application Service
API](https://spec.matrix.org/v1.19/application-service-api/#registration)
Synapse returns the correct 400 on both endpoints, but with the unstable
identifier.
Before:
```
POST /_matrix/client/v3/login {"type": "m.login.application_service", ...} # appservice with MSC4190 device management
POST /_matrix/client/v3/register {"type": "m.login.application_service", ...} # without "inhibit_login": true
400 {"errcode": "IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED"}
```
After:
```
POST /_matrix/client/v3/login {"type": "m.login.application_service", ...} # appservice with MSC4190 device management
POST /_matrix/client/v3/register {"type": "m.login.application_service", ...} # without "inhibit_login": true
400 {"errcode": "M_APPSERVICE_LOGIN_UNSUPPORTED"}
```
A Sister PR exists in MAS:
https://github.com/element-hq/matrix-authentication-service/pull/5961 ;
when delegation is enabled, `/login` reaches MAS instead of Synapse, and
MAS currently answers `m.login.application_service` with `M_UNKNOWN`.
## The bug
With the experimental
[MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222)
implementation enabled (`use_state_after`) and lazy-loading of room
members, an incremental `/sync` could disclose state from **after** the
user's leave in a left room's `state_after`.
1. Alice syncs with `lazy_load_members: true` and
`use_state_after=true`.
2. Bob sends a message in a room they share.
3. Alice leaves the room.
4. Bob updates his per-room displayname
5. Alice does an incremental sync covering steps 2–3. Alice's
`state_after` contains Bob's post-leave membership event from step 4
Alice should not see the new per-room display name of Bob.
## The fix
Copy what has been done for `_compute_state_delta_for_full_sync`: pass
`joined` down and, for rooms the user is no longer joined to, fetch the
memberships as of `end_token` via state groups (`get_state_ids_at`)
instead of current state.
With `enable_set_displayname: false` (or `enable_set_avatar_url:
false`), refusing a profile change returned the right errcode with the
wrong status:
```
PUT /_matrix/client/v3/profile/@alice:example.com/displayname (displayname already set)
→ 400 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
DELETE /_matrix/client/v3/profile/@alice:example.com/displayname
→ 400 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
```
With this fix:
```
PUT /_matrix/client/v3/profile/@alice:example.com/displayname (displayname already set)
→ 403 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
DELETE /_matrix/client/v3/profile/@alice:example.com/displayname
→ 403 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
```
The spec defines the [403 response of `PUT
/_matrix/client/v3/profile/{userId}/{keyName}`](https://spec.matrix.org/v1.19/client-server-api/#put_matrixclientv3profileuseridkeyname)
as "The server is unwilling to perform the operation, either due to
insufficient permissions or **because profile modifications are
disabled**", while 400 is reserved for malformed input (`M_BAD_JSON`,
`M_MISSING_PARAM`, …).
Clients seem to rely on `errcode` field more than the HTTP Status Code,
that change seems safe.
When a client (correctly) calls the `DELETE` endpoint to remove a custom
profile field (like Element X does with `m.status`), we incorrectly
don't include it in the sync response in legacy sync. This was due to
the fact that we cleaned up the sent fields down to what fields the
profile currently has.
Always ensure any fields in `ProfileUpdateAction.UPDATE` are sent down,
as `null` values for profile fields which have been deleted.
Fixes an issue where clearing a user status from Element X does not
reflect in the user status being cleared on Element Web.
Note, target is the v1.160.0 release branch due to customer commitments,
and this fixes web and mobile clients not working together correctly.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [ ] 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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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))
This PR implements support for profile updates over Sliding Sync:
https://github.com/matrix-org/matrix-spec-proposals/pull/4262. This pr
may be easier to review as a whole than commit by commit.
This builds on the legacy sync profile updates feature
https://github.com/element-hq/synapse/pull/19556, specifically the
profile updates stream it added.
Submitting for early review to get consensus on implementation. There
are some things we would like to add still, from spec, mainly:
* > Homeservers should only consider a profile field update "accepted"
by a client
> once the client returns with a new /sync request with the next /sync
token,
> NOT just after sending down the profile update. The client may never
receive
> response due to network conditions, or a bug in the client
implementation.
* > When a room enters this subset in this connection for the first
time, all requested
> fields from profiles of users in that room MAY be sent down. This
gives the client
> a base set of information for which future field updates can be
applied on top of.
> The homeserver MAY omit some fields and profiles if it believes that
the client has
> already received them, likewise repeat profiles MAY be sent down based
on homeserver
> implementation.
* > Finally, if the list of fields expands to cover a new field ID,
those fields should
> be sent down for all users that are within the current room subset.
Future incremental
> updates will then include changes to this field.
* Additionally, we would need to implement a lazy loading cache similar
to the legacy sync. (not part of MSC as such)
Depending on review these could either be added to this pr, or to keep
this pr from not growing too much, be added in a follow-up pr, as they
are more enhancement to this base sliding sync profile updates
functionality than a part of the core functionality.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [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:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [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: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Olivier 'reivilibre' <olivier@librepush.net>
Co-authored-by: Olivier 'reivilibre <oliverw@element.io>
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>
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>
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>
Erik Johnston