Add support for stable prefixes for MSC2285: private read receipts #13273

SimonBrandner · 2022-07-14T07:35:45Z

Requires matrix-org/matrix-spec-proposals#2285
Requires matrix-org/matrix-js-sdk#2524
Requires matrix-org/matrix-react-sdk#8629

Do not merge until rest of the PRs are ready

Pull request is based on the develop branch
Pull request includes a changelog file.
Pull request includes a sign off
Code style is correct

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

synapse/rest/client/read_marker.py

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

synapse/handlers/receipts.py

…at/disable-rr

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

synapse/handlers/receipts.py

…at/disable-rr Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Stale

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

synapse/storage/databases/main/event_push_actions.py

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

…/disable-rr

synapse/rest/client/read_marker.py

clokep · 2022-08-04T20:16:41Z

synapse/storage/databases/main/event_push_actions.py

                SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions,
                    ep.highlight
                FROM (
                    SELECT room_id,
                        MAX(stream_ordering) as stream_ordering
                    FROM events
                    INNER JOIN receipts_linearized USING (room_id, event_id)
-                    WHERE receipt_type = 'm.read' AND user_id = ?
+                    WHERE {receipt_types_clause} AND user_id = ?


I think the ramifications of this change (and the ones below it) are that we might have HTTP pushed / emailed for something that was actually read using a private read receipt? Does that sound accurate?

So I am not sure I understand the code at hand though my thinking was that since both public and private RRs influence notifs in the same way, they should be added here. Was my thinking incorrect?

Was my thinking incorrect?

That's my understanding as well. I'm trying to figure out the user-impact of fixing this bug.

OK I double checked this; I think (before fixing this to look at all read receipt types) you could end up in this situation:

There are some new events in a room.

The user reads them and sends a private read receipt.

They have an email or HTTP pusher configured.

The private read receipt would not be considered read when checking for events to push.

The new events would be pushed even though they're before the private read receipt.

Good that we found it, but doesn't seem to be the end of the world!

Good that we found it

Thank you very much for doing that! I've totally missed it

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

synapse/rest/client/versions.py

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

clokep

This should be good! 👍

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

…nto SimonBrandner/feat/disable-rr

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

clokep · 2022-08-05T15:17:20Z

Filed #13468 to remove the unstable identifiers in the future.

@andrewdoh

Synapse 1.65.0 (2022-08-16) =========================== No significant changes since 1.65.0rc2. Synapse 1.65.0rc2 (2022-08-11) ============================== Internal Changes ---------------- - Revert 'Remove the unspecced `room_id` field in the `/hierarchy` response. ([\matrix-org#13365](matrix-org#13365))' to give more time for clients to update. ([\matrix-org#13501](matrix-org#13501)) Synapse 1.65.0rc1 (2022-08-09) ============================== Features -------- - Add support for stable prefixes for [MSC2285 (private read receipts)](matrix-org/matrix-spec-proposals#2285). ([\matrix-org#13273](matrix-org#13273)) - Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`, `ORG.MATRIX.MSC3848.NOT_JOINED`, and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in [MSC3848](matrix-org/matrix-spec-proposals#3848). ([\matrix-org#13343](matrix-org#13343)) - Use stable prefixes for [MSC3827](matrix-org/matrix-spec-proposals#3827). ([\matrix-org#13370](matrix-org#13370)) - Add a new module API method to translate a room alias into a room ID. ([\matrix-org#13428](matrix-org#13428)) - Add a new module API method to create a room. ([\matrix-org#13429](matrix-org#13429)) - Add remote join capability to the module API's `update_room_membership` method (in a backwards compatible manner). ([\matrix-org#13441](matrix-org#13441)) Bugfixes -------- - Update the version of the LDAP3 auth provider module included in the `matrixdotorg/synapse` DockerHub images and the Debian packages hosted on packages.matrix.org to 0.2.2. This version fixes a regression in the module. ([\matrix-org#13470](matrix-org#13470)) - Fix a bug introduced in Synapse v1.41.0 where the `/hierarchy` API returned non-standard information (a `room_id` field under each entry in `children_state`). ([\matrix-org#13365](matrix-org#13365)) - Fix a bug introduced in Synapse 0.24.0 that would respond with the wrong error status code to `/joined_members` requests when the requester is not a current member of the room. Contributed by @andrewdoh. ([\matrix-org#13374](matrix-org#13374)) - Fix bug in handling of typing events for appservices. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#13392](matrix-org#13392)) - Fix a bug introduced in Synapse 1.57.0 where rooms listed in `exclude_rooms_from_sync` in the configuration file would not be properly excluded from incremental syncs. ([\matrix-org#13408](matrix-org#13408)) - Fix a bug in the experimental faster-room-joins support which could cause it to get stuck in an infinite loop. ([\matrix-org#13353](matrix-org#13353)) - Faster room joins: fix a bug which caused rejected events to become un-rejected during state syncing. ([\matrix-org#13413](matrix-org#13413)) - Faster room joins: fix error when running out of servers to sync partial state with, so that Synapse raises the intended error instead. ([\matrix-org#13432](matrix-org#13432)) Updates to the Docker image --------------------------- - Make Docker images build on armv7 by installing cryptography dependencies in the 'requirements' stage. Contributed by Jasper Spaans. ([\matrix-org#13372](matrix-org#13372)) Improved Documentation ---------------------- - Update the 'registration tokens' page to acknowledge that the relevant MSC was merged into version 1.2 of the Matrix specification. Contributed by @moan0s. ([\matrix-org#11897](matrix-org#11897)) - Document which HTTP resources support gzip compression. ([\matrix-org#13221](matrix-org#13221)) - Add steps describing how to elevate an existing user to administrator by manipulating the database. ([\matrix-org#13230](matrix-org#13230)) - Fix wrong headline for `url_preview_accept_language` in documentation. ([\matrix-org#13437](matrix-org#13437)) - Remove redundant 'Contents' section from the Configuration Manual. Contributed by @dklimpel. ([\matrix-org#13438](matrix-org#13438)) - Update documentation for config setting `macaroon_secret_key`. ([\matrix-org#13443](matrix-org#13443)) - Update outdated information on `sso_mapping_providers` documentation. ([\matrix-org#13449](matrix-org#13449)) - Fix example code in module documentation of `password_auth_provider_callbacks`. ([\matrix-org#13450](matrix-org#13450)) - Make the configuration for the cache clearer. ([\matrix-org#13481](matrix-org#13481)) Internal Changes ---------------- - Extend the release script to automatically push a new SyTest branch, rather than having that be a manual process. ([\matrix-org#12978](matrix-org#12978)) - Make minor clarifications to the error messages given when we fail to join a room via any server. ([\matrix-org#13160](matrix-org#13160)) - Enable Complement CI tests in the 'latest deps' test run. ([\matrix-org#13213](matrix-org#13213)) - Fix long-standing bugged logic which was never hit in `get_pdu` asking every remote destination even after it finds an event. ([\matrix-org#13346](matrix-org#13346)) - Faster room joins: avoid blocking when pulling events with partially missing prev events. ([\matrix-org#13355](matrix-org#13355)) - Instrument `/messages` for understandable traces in Jaeger. ([\matrix-org#13368](matrix-org#13368)) - Remove an unused argument to `get_relations_for_event`. ([\matrix-org#13383](matrix-org#13383)) - Add a `merge-back` command to the release script, which automates merging the correct branches after a release. ([\matrix-org#13393](matrix-org#13393)) - Adding missing type hints to tests. ([\matrix-org#13397](matrix-org#13397)) - Faster Room Joins: don't leave a stuck room partial state flag if the join fails. ([\matrix-org#13403](matrix-org#13403)) - Refactor `_resolve_state_at_missing_prevs` to compute an `EventContext` instead. ([\matrix-org#13404](matrix-org#13404), [\matrix-org#13431](matrix-org#13431)) - Faster Room Joins: prevent Synapse from answering federated join requests for a room which it has not fully joined yet. ([\matrix-org#13416](matrix-org#13416)) - Re-enable running Complement tests against Synapse with workers. ([\matrix-org#13420](matrix-org#13420)) - Prevent unnecessary lookups to any external `get_event` cache. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#13435](matrix-org#13435)) - Add some tracing to give more insight into local room joins. ([\matrix-org#13439](matrix-org#13439)) - Rename class `RateLimitConfig` to `RatelimitSettings` and `FederationRateLimitConfig` to `FederationRatelimitSettings`. ([\matrix-org#13442](matrix-org#13442)) - Add some comments about how event push actions are stored. ([\matrix-org#13445](matrix-org#13445), [\matrix-org#13455](matrix-org#13455)) - Improve rebuild speed for the "synapse-workers" docker image. ([\matrix-org#13447](matrix-org#13447)) - Fix `@tag_args` being off-by-one with the arguments when tagging a span (tracing). ([\matrix-org#13452](matrix-org#13452)) - Update type of `EventContext.rejected`. ([\matrix-org#13460](matrix-org#13460)) - Use literals in place of `HTTPStatus` constants in tests. ([\matrix-org#13463](matrix-org#13463), [\matrix-org#13469](matrix-org#13469)) - Correct a misnamed argument in state res v2 internals. ([\matrix-org#13467](matrix-org#13467))

packaging changes: summary of upstream changes: Synapse 1.65.0 (2022-08-16) =========================== Features -------- - Add support for stable prefixes for [MSC2285 (private read receipts)](matrix-org/matrix-spec-proposals#2285). ([\#13273](matrix-org/synapse#13273)) - Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`, `ORG.MATRIX.MSC3848.NOT_JOINED`, and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in [MSC3848](matrix-org/matrix-spec-proposals#3848). ([\#13343](matrix-org/synapse#13343)) - Use stable prefixes for [MSC3827](matrix-org/matrix-spec-proposals#3827). ([\#13370](matrix-org/synapse#13370)) - Add a new module API method to translate a room alias into a room ID. ([\#13428](matrix-org/synapse#13428)) - Add a new module API method to create a room. ([\#13429](matrix-org/synapse#13429)) - Add remote join capability to the module API's `update_room_membership` method (in a backwards compatible manner). ([\#13441](matrix-org/synapse#13441))

SimonBrandner added 2 commits July 14, 2022 09:45

Switch to stable prefixes for MSC2285

83b7af1

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Add changelog

01fbb6d

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

SimonBrandner force-pushed the SimonBrandner/feat/disable-rr branch from b808bf6 to 01fbb6d Compare July 14, 2022 07:45

SimonBrandner marked this pull request as ready for review July 14, 2022 08:20

SimonBrandner requested a review from a team as a code owner July 14, 2022 08:20

clokep reviewed Jul 14, 2022

View reviewed changes

synapse/rest/client/read_marker.py Show resolved Hide resolved

SimonBrandner requested a review from clokep July 14, 2022 11:16

SimonBrandner added 3 commits July 14, 2022 13:35

Avoid leaking unstable private read receipts

cafe4be

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Delint

967f8ed

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Fix code

a9f2329

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

clokep reviewed Jul 18, 2022

View reviewed changes

synapse/handlers/receipts.py Outdated Show resolved Hide resolved

clokep reviewed Jul 18, 2022

View reviewed changes

synapse/handlers/receipts.py Show resolved Hide resolved

SimonBrandner added 2 commits July 23, 2022 13:53

Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…

08d874d

…at/disable-rr

Use a const

ba20652

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

SimonBrandner requested a review from clokep July 23, 2022 12:08

clokep reviewed Jul 25, 2022

View reviewed changes

synapse/handlers/receipts.py Outdated Show resolved Hide resolved

Convert list to tuple.

d18a485

clokep approved these changes Jul 25, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…

8323358

…at/disable-rr Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

clokep changed the title ~~Switch to stable prefixes for MSC2285~~ Switch to stable prefixes for MSC2285: private read receipts Jul 28, 2022

SimonBrandner added 7 commits July 29, 2022 07:29

Add background update to remove unstable private read receipts

404459f

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Remove handling for unstable private read receipts

57251c8

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Don't bump SCHEMA_VERSION

e12706c

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Remove READ_PRIVATE_UNSTABLE from ReceiptTypes

c8cf10e

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Assume return value

84299f6

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Remove non-relevant test

208e97a

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Don't bother with a migration script

a7c3a1c

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Test both stable and unstable private RRs

3612ceb

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

SimonBrandner requested a review from clokep August 4, 2022 08:10

Advertise unstable prefix

a37a17d

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

SimonBrandner added 3 commits August 4, 2022 16:01

Check for all receipt types

14b36a3

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Delint

746351c

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Further dlint

50bffe4

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

clokep reviewed Aug 4, 2022

View reviewed changes

synapse/storage/databases/main/event_push_actions.py Outdated Show resolved Hide resolved

SimonBrandner added 4 commits August 4, 2022 16:12

Remove (

1b66814

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

This won't work...

bcdbb5d

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Reformat...

0f498e0

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Get tests passing

660c776

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

SimonBrandner requested a review from clokep August 4, 2022 16:33

clokep added 2 commits August 4, 2022 16:06

Merge remote-tracking branch 'origin/develop' into SimonBrandner/feat…

b1ddf1a

…/disable-rr

Fix type hints.

3094b39

clokep reviewed Aug 4, 2022

View reviewed changes

clokep self-requested a review August 4, 2022 20:17

Put experimental flag back in place

011d6d2

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

clokep reviewed Aug 5, 2022

View reviewed changes

synapse/rest/client/versions.py Outdated Show resolved Hide resolved

Only advertise org.matrix.msc2285 if enabled

ee6512f

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

clokep approved these changes Aug 5, 2022

View reviewed changes

SimonBrandner added 3 commits August 5, 2022 14:45

Fix tests

2cd29bd

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

Merge remote-tracking branch 'origin/SimonBrandner/feat/disable-rr' i…

89d5bd4

…nto SimonBrandner/feat/disable-rr

Fix tests

f0c531d

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

clokep merged commit ab18441 into matrix-org:develop Aug 5, 2022

clokep mentioned this pull request Aug 5, 2022

Remove unstable identifiers for MSC2285 (private read receipts) #13468

Closed

SimonBrandner deleted the SimonBrandner/feat/disable-rr branch August 5, 2022 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for stable prefixes for MSC2285: private read receipts #13273

Add support for stable prefixes for MSC2285: private read receipts #13273

SimonBrandner commented Jul 14, 2022 •

edited

Loading

clokep Aug 4, 2022

SimonBrandner Aug 5, 2022

clokep Aug 5, 2022

clokep Aug 5, 2022

SimonBrandner Aug 5, 2022

clokep left a comment

clokep commented Aug 5, 2022

Add support for stable prefixes for MSC2285: private read receipts #13273

Add support for stable prefixes for MSC2285: private read receipts #13273

Conversation

SimonBrandner commented Jul 14, 2022 • edited Loading

Do not merge until rest of the PRs are ready

clokep Aug 4, 2022

Choose a reason for hiding this comment

SimonBrandner Aug 5, 2022

Choose a reason for hiding this comment

clokep Aug 5, 2022

Choose a reason for hiding this comment

clokep Aug 5, 2022

Choose a reason for hiding this comment

SimonBrandner Aug 5, 2022

Choose a reason for hiding this comment

clokep left a comment

Choose a reason for hiding this comment

clokep commented Aug 5, 2022

SimonBrandner commented Jul 14, 2022 •

edited

Loading