The validation of `client_secret` params accepts invalid values: any
value that includes a char in `[0-9a-zA-Z.=_-]` is accepted, for example
`"café"` is accepted.
Instead synapse should only accept if all chars are within
`[0-9a-zA-Z.=_-]`, not just one.
This is the current validator:
```python
ClientSecretStr = Annotated[
str,
StringConstraints(
pattern="[0-9a-zA-Z.=_-]",
min_length=1,
max_length=255,
strict=True,
),
]
```
Unfortunately, Pydantic only defines the `pattern` argument as:
> ### pattern
> A regex pattern that the string must match.
Which is extremely imprecise.
Here is a little script to verify the behavior:
```python
from typing import Annotated
from pydantic import BaseModel, StringConstraints, ValidationError
def check(pattern: str) -> None:
ClientSecretStr = Annotated[
str,
StringConstraints(pattern=pattern, min_length=1, max_length=255, strict=True),
]
class Body(BaseModel):
client_secret: ClientSecretStr
try:
Body.model_validate({"client_secret": "café"})
print(f"pattern = {pattern!r}: 'café' ACCEPTED <-- should have been rejected")
except ValidationError:
print(f"pattern = {pattern!r}: 'café' rejected")
check("[0-9a-zA-Z.=_-]") # before the fix (unanchored)
check("^[0-9a-zA-Z.=_-]+$") # after the fix
```
which would output
```
pattern = '[0-9a-zA-Z.=_-]': 'café' ACCEPTED <-- should have been rejected
pattern = '^[0-9a-zA-Z.=_-]+$': 'café' rejected
```
## History
This is a regression of a previously reported and fixed bug:
- matrix-org/synapse#6766 (2020) reported that Synapse did not enforce
the spec's `client_secret` regex at all — with real-world fallout:
FluffyChat had started sending secrets containing `:` because nothing
rejected them. Fixed by introducing `assert_valid_client_secret`
(matrix-org/synapse#6767).
- matrix-org/synapse#13188 (Synapse 1.66.0) ported the account endpoints
to Pydantic and transcribed the regex without anchors/quantifier;
Pydantic v1's `re.match` semantics meant only the *first* character was
validated.
- #19071 (Synapse 1.142.0) migrated to Pydantic v2, whose *search*
semantics weakened it further to "any one character anywhere".
---
### 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: Quentin Gliech <quenting@element.io>
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))
Follow-up to #20149, which fixed the 500 when *getting* a profile field
for a user with no `profiles` row. The same crash was still reachable
when *setting* one via `PUT /_matrix/client/v3/profile/{userId}/{field}`
(as server admin). This PR splits that case in two:
* **The user exists but has no `profiles` row** (e.g. profile erased
upon deactivation):
* Before: `500 M_UNKNOWN` (`TypeError: cannot unpack non-sequence
NoneType` in the profile size check).
* After: `200`, the profile row is recreated with the field set.
* **The user does not exist at all**:
* Before: `500 M_UNKNOWN` (same crash).
* After: `404 M_NOT_FOUND`, without conjuring up an orphan profile row.
Fixing the crash also surfaced a latent SQLite-only bug where a field
set on a freshly created profile row was stored under the wrong key,
making it 404 on `GET` right after a successful `PUT`.
### 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`.
Part of https://github.com/element-hq/synapse/issues/18118
## What the spec says
Since Matrix v1.13 (introduced by
[MSC4178](https://github.com/matrix-org/matrix-spec-proposals/pull/4178)),
the `400` response of [`POST
/_matrix/client/v3/account/3pid/email/requestToken`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3account3pidemailrequesttoken)
lists, among the "Error codes that can be returned":
> - `M_THREEPID_MEDIUM_NOT_SUPPORTED`: The homeserver does not support
adding email addresses.
> - `M_INVALID_PARAM`: The email address given was not valid.
and [`POST
/_matrix/client/v3/account/3pid/msisdn/requestToken`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3account3pidmsisdnrequesttoken)
likewise:
> - `M_THREEPID_MEDIUM_NOT_SUPPORTED`: The homeserver does not support
adding phone numbers.
> - `M_INVALID_PARAM`: The phone number given was not valid.
## What was missing
Synapse implemented the headline case (unsupported medium), but around
it:
- A malformed email address or country code was reported with the
generic `M_BAD_JSON` instead of `M_INVALID_PARAM`. The email validator
deliberately kept `M_BAD_JSON` "to ensure backward compatibility of HTTP
error codes" (matrix-org/synapse#13687, 2022) — that predates Matrix
v1.13, which now lists `M_INVALID_PARAM` for this case.
- On the msisdn variant, the unsupported-medium check ran after the
denied/in-use checks, so a request wrong in two ways reported the other
fault; the email variant checks it first.
## What this PR changes
- Malformed email addresses and country codes on
`/account/3pid/{email,msisdn}/requestToken` are reported with
`M_INVALID_PARAM`. Both flow through the existing errcode translation as
`value_error`: the email validator raises a plain `ValueError`, and the
country-code constraint (`ISO3166_1_Alpha_2`) declares its own error via
pydantic-core's `custom_error_schema`.
- On the msisdn variant the unsupported-medium check now runs before the
denied/in-use checks, as on the email variant.
- The country-code type is renamed from `ISO3116_1_Alpha_2` to
`ISO3166_1_Alpha_2` (typo in the standard's number).
[`/account/password/email/requestToken`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3accountpasswordemailrequesttoken)
(not covered by the v1.13 change) shares the email request body model,
so a malformed email there is now also reported with `M_INVALID_PARAM`
instead of `M_BAD_JSON`. Its `400` response is described as "the request
was invalid" and only names `M_SERVER_NOT_TRUSTED` explicitly ("can be
returned if…") rather than restricting the server to a fixed list, and
`M_INVALID_PARAM` is the spec's generic code for "A parameter that was
specified has the wrong value" ([other error
codes](https://spec.matrix.org/v1.19/client-server-api/#other-error-codes)).
Apply the `rc_reports` rate limit to the room reporting endpoint, [`POST
/_matrix/client/v3/rooms/{roomId}/report`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3roomsroomidreport)
(added in Matrix v1.13).
The spec marks this endpoint as **Rate-limited: Yes** (clients must
expect a `429 M_LIMIT_EXCEEDED`), and homeservers [SHOULD implement rate
limiting](https://spec.matrix.org/v1.19/client-server-api/#rate-limiting)
in general, but Synapse currently applies no limit here. The sibling
user reporting endpoint already uses `rc_reports`, so this reuses the
same limit instead of introducing a new config option.
Changes:
- Move the room report logic from `ReportRoomRestServlet` into a new
`ReportsHandler.report_room`, mirroring the existing `report_user`. The
rate limit is checked before the room existence lookup, so it bounds the
DB work a caller can trigger and cannot be used to tell existing rooms
from non-existing ones.
- The servlet keeps the existing behaviour of returning `200` regardless
of room existence when `msc4277_enabled` is set (the spec allows this
since v1.18).
- Add a regression test covering the `429` response and the per-user
rate limit override.
## 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.
This PR adds a suite of tests for Synapse state events. It covers key
scenarios around creation, updates and state consistency to prevent
regressions in event processing and serialization.
The primary goal is to increase test coverage.
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.
Room topics are dropped from the search index whenever the
`event_search` background reindex runs (e.g. after a search index
rebuild, or when the background update is re-run on an upgraded
homeserver), making topics unsearchable even though the live write path
indexes them correctly.
The cause is a trailing comma in `_background_reindex_search`, which
turns the topic `value` into a 1-tuple instead of a string:
https://github.com/element-hq/synapse/blob/14c96c0f5444cbe28b6ac0361cf94216b8d352db/synapse/storage/databases/main/search.py#L211-L213
The downstream `if not isinstance(value, str): continue` guard then
silently skips *every* `m.room.topic` event, so no topic ever reaches
`event_search` during a reindex.
The regression was introduced in #18195, which added rich-text topic
support (MSC3765) to the reindex path.
### Problem Example
1. A room has topic "project roadmap".
2. An admin rebuilds the search index (or the `event_search` background
update re-runs).
3. `m.room.message` and `m.room.name` events are reindexed fine, but
every `m.room.topic` event is skipped.
4. Searching for "project roadmap" with key `content.topic` returns 0
results — the topic is permanently unsearchable until the event is sent
again.
---
### 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))
For the Admin API /users endpoint, the response is meant to exclude the
approval flag when MSC3866 is enabled, but the filter that applies the
exclusion never actually got used. Fix things so that it does.
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))
Drive-by fix for a problem noticed when doing some other work.
This only triggers for 'custom'/generic fields, not the built-in
`displayname` and `avatar_url`.
Not seeing an open issue for it.
Return a 404, not 500, when looking up a profile field on a missing
profile
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
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>
We have an internal usage of `/scheduled_tasks` that would like to fetch
multiple actions at once (janitor).
We also make it so that invalid `status` values now return a 400 rather
than a 500.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
*Spawning from
https://github.com/element-hq/synapse/pull/19057#discussion_r2427537811,*
Fix tests that use `homeserver_to_use=GenericWorkerServer` not being
able to be run standalone
Fix https://github.com/element-hq/synapse/issues/15671 (previously
https://github.com/matrix-org/synapse/issues/15671)
Before this change:
```shell
$ poetry run trial tests.storage.test_rollback_worker.WorkerSchemaTests.test_rolling_back
tests.storage.test_rollback_worker
WorkerSchemaTests
test_rolling_back ... [ERROR]
===============================================================================
[ERROR]
Traceback (most recent call last):
File "synapse/tests/unittest.py", line 129, in new
return code(orig, *args, **kwargs)
File "synapse/tests/unittest.py", line 223, in setUp
return orig()
File "synapse/tests/unittest.py", line 398, in setUp
self.hs = self.make_homeserver(self.reactor, self.clock)
File "synapse/tests/storage/test_rollback_worker.py", line 54, in make_homeserver
hs = self.setup_test_homeserver(homeserver_to_use=GenericWorkerServer)
File "synapse/tests/unittest.py", line 669, in setup_test_homeserver
hs = setup_test_homeserver(
File "synapse/tests/server.py", line 1260, in setup_test_homeserver
prepare_database(
File "synapse/synapse/storage/prepare_database.py", line 167, in prepare_database
raise UpgradeDatabaseException(EMPTY_DATABASE_ON_WORKER_ERROR)
synapse.storage.prepare_database.UpgradeDatabaseException: Uninitialised database: run the main synapse process to prepare the database schema before starting worker processes.
tests.storage.test_rollback_worker.WorkerSchemaTests.test_rolling_back
-------------------------------------------------------------------------------
Ran 1 tests in 0.034s
FAILED (errors=1)
```
### What was the problem before?
[`PREPPED_SQLITE_DB_CONN`](https://github.com/element-hq/synapse/blob/1a1af7b622f219ba0f2501709298caa230ad3912/tests/server.py#L1247-L1262)
is a process global and shared between all tests. Whichever test first
calls `setup_test_homeserver(...)` builds the template database for the
whole trial run.
`prepare_database(...)` has a built-in check to refuse upgrading the
database ["to avoid multiple workers doing it at
once."](https://github.com/element-hq/synapse/blob/1a1af7b622f219ba0f2501709298caa230ad3912/synapse/storage/prepare_database.py#L164-L167)
and throw `UpgradeDatabaseException`.
This means that if we happen to first run a test that uses a worker
(`homeserver_to_use=GenericWorkerServer`), `prepare_database(...)` will
just throw its `UpgradeDatabaseException`. And since
`PREPPED_SQLITE_DB_CONN` is assigned before `prepare_database(...)`, it
will never try to prepare again and the rest of the tests will fail.
The registration of `QuarantinedMediaStream` in
`ReplicationCommandHandler._streams_to_replicate` was missed when the
stream was added, so an instance configured as the
quarantined_media_changes stream writer never sent RDATA/POSITION for it
unless it was the main process.
Also add the stream to the `instance_map` config validation.
Stream was introduced in
https://github.com/element-hq/synapse/pull/19558
Fixes https://github.com/element-hq/synapse/issues/20080
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Fix `RemoteJoinHelper` signing events with mismatched room version
compared to the `room_version` arg. The default room version on
`develop` is `11` but the `RemoteJoinHelper` `room_version` arg defaults
to `10` (room version mismatch). This mismatch wasn't present where this
fix was developed
(https://github.com/element-hq/synapse-private/pull/136) as the default
room version was only recently bumped to `11` via
https://github.com/element-hq/synapse/pull/18680 (not even in a release
yet).
Fixes the CI being broken on `develop` :x::
```
[ERROR]
Traceback (most recent call last):
File "/home/runner/work/synapse/synapse/tests/federation/test_federation_join_upgraded_room.py", line 298, in test_no_transfer_when_tombstone_does_not_match
join_helper.join(local_user_id, local_user_tok)
File "/home/runner/work/synapse/synapse/tests/federation/_remote_join.py", line 350, in join
self._test_case.helper.join(remote_room_id, local_user_id, tok=local_user_tok)
File "/home/runner/work/synapse/synapse/tests/rest/client/utils.py", line 195, in join
return self.change_membership(
File "/home/runner/work/synapse/synapse/tests/rest/client/utils.py", line 333, in change_membership
assert channel.code == expect_code, (
builtins.AssertionError: Expected: 200, got: 400, PUT /_matrix/client/r0/rooms/!remote-room:other.example.com/state/m.room.member/@user1:test?access_token=syt_dXNlcjE_JSEarRndiAGqtydnrdLn_33qlLD -> resp: b'{"errcode":"M_UNKNOWN","error":"No create event in state"}'
tests.federation.test_federation_join_upgraded_room.FederationJoinUpgradedRoomTestCase.test_no_transfer_when_tombstone_does_not_match
```
These tests were originally introduced
https://github.com/element-hq/synapse-private/pull/136 (developed
private as this was part of the Synapse security release) and introduced
into the public codebase via
https://github.com/element-hq/synapse/commit/cbc6934821aab314506c5957223c60c74eeda091
Johannes Marbach