Prevent historical state from being pushed to an application service via `/transactions` (MSC2716) #11265

MadLittleMods · 2021-11-05T22:10:26Z

Prevent historical state from being pushed to an application service via /transactions. We now mark historical state from the MSC2716 /batch_send endpoint as historical which makes it backfilled and have a negative stream_ordering so it doesn't get queried by /transactions.

Fix #11241

Complement tests: matrix-org/complement#221

Part of MSC2716: matrix-org/matrix-spec-proposals#2716

Dev notes

update_membership
update_membership_locked
_local_membership_update
create_event

create_and_send_nonmember_event
create_event

/sync looks for stream_ordering but excludes all outliers.

synapse/synapse/storage/databases/main/stream.py

Lines 519 to 527 in b09d90c

    
                       sql = """ 
        
                           SELECT event_id, instance_name, topological_ordering, stream_ordering 
        
                           FROM events 
        
                           WHERE 
        
                               room_id = ? 
        
                               AND not outlier 
        
                               AND stream_ordering > ? AND stream_ordering <= ? 
        
                           ORDER BY stream_ordering %s LIMIT ? 
        
                       """ % (

Whereas /transactions only cares about stream_ordering. Perhaps we should exclude outliers in /transactions?

synapse/synapse/storage/databases/main/appservice.py

Lines 358 to 368 in b09d90c

    
           def get_new_events_for_appservice_txn(txn): 
        
               sql = ( 
        
                   "SELECT e.stream_ordering, e.event_id" 
        
                   " FROM events AS e" 
        
                   " WHERE" 
        
                   " (SELECT stream_ordering FROM appservice_stream_position)" 
        
                   "     < e.stream_ordering" 
        
                   " AND e.stream_ordering <= ?" 
        
                   " ORDER BY e.stream_ordering ASC" 
        
                   " LIMIT ?" 
        
               )

`/sync` stack trace

`/transactions` stack trace

submit_event_for_as ->

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. 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.
~~Pull request includes a sign off~~
Code style is correct
(run the linters)

Part of #11241

Regression tests for matrix-org/synapse#11241 Synapse changes: matrix-org/synapse#11265

…n Complement Complement tests: matrix-org/complement#221

synapse/appservice/api.py

MadLittleMods · 2021-11-06T05:46:49Z

synapse/handlers/room_batch.py

@@ -221,6 +221,7 @@ async def persist_state_events_at_start(
                    action=membership,
                    content=event_dict["content"],
                    outlier=True,
+                    historical=True,


All of these changes are just piping historical down to create_event

update_membership update_membership_locked _local_membership_update create_event

MadLittleMods · 2021-11-06T05:46:57Z

synapse/handlers/room_batch.py

@@ -240,6 +241,7 @@ async def persist_state_events_at_start(
                    ),
                    event_dict,
                    outlier=True,
+                    historical=True,


All of these changes are just piping historical down to create_event

create_and_send_nonmember_event create_event

whatever else happens, please please please make sure that any parameters are added to the docstrings at each level, with the intended purpose clearly described.

Relatedly: as a general rule, I would say it is preferable for method parameters to describe a change in behaviour ("inhibit_sync_to_clent") rather than describe some bit of state that the function is free to interpret as it wishes: it's much clearer for a later reader to understand how things tie together, particularly when things then get modified later on. Obviously this isn't a hard-and-fast rule, but it's worth considering.

[Currently we have a bit of a mess in some places (FederationEventHander.backfilled being a prime example) where you have to dig through 150 methods to find out what a flag actually does, and then somehow reverse-engineer what it is supposed to do. Don't do that.]

please please please make sure that any parameters are added to the docstrings at each level, with the intended purpose clearly described.

Will do 🙇

Relatedly: as a general rule, I would say it is preferable for method parameters to describe a change in behaviour ("inhibit_sync_to_clent") rather than describe some bit of state that the function is free to interpret as it wishes

Seems reasonable. I've created a separate issue to track this #11300

synapse/appservice/api.py

erikjohnston · 2021-11-09T09:05:22Z

changelog.d/11265.bugfix

@@ -0,0 +1 @@
+Prevent [MSC2716](https://github.com/matrix-org/matrix-doc/pull/2716) historical state events from being pushed to an application service via `/transactions`.


I'm a bit confused why these would come down /transactions and not down /sync?

I was a little bit curious about this as well and saw that /transactions only cares about stream_ordering so it made sense why it still came down and didn't look into it further.

Diving into this more now, here are the details:

/sync looks for stream_ordering but excludes all outliers.

synapse/synapse/storage/databases/main/stream.py

Lines 519 to 527 in b09d90c

sql = """

SELECT event_id, instance_name, topological_ordering, stream_ordering

FROM events

WHERE

room_id = ?

AND not outlier

AND stream_ordering > ? AND stream_ordering <= ?

ORDER BY stream_ordering %s LIMIT ?

""" % (

Whereas /transactions only cares about stream_ordering. Perhaps we should exclude outliers in /transactions? I can tackle that in a separate PR if we decide.

synapse/synapse/storage/databases/main/appservice.py

Lines 358 to 368 in b09d90c

def get_new_events_for_appservice_txn(txn):

sql = (

"SELECT e.stream_ordering, e.event_id"

" FROM events AS e"

" WHERE"

" (SELECT stream_ordering FROM appservice_stream_position)"

" < e.stream_ordering"

" AND e.stream_ordering <= ?"

" ORDER BY e.stream_ordering ASC"

" LIMIT ?"

)

/sync stack trace

SyncRestServlet.on_GET

wait_for_sync_for_user

_wait_for_sync_for_user

current_sync_for_user

generate_sync_result

_generate_sync_entry_for_rooms

_get_rooms_changed

get_room_events_stream_for_rooms

get_room_events_stream_for_room

/transactions stack trace

notify_interested_services

_notify_interested_services

get_new_events_for_appservice

submit_event_for_as

submit_event_for_as ->

enqueue_event

_send_request

appservice.scheduler.send

create_appservice_txn

AppServiceTransaction.send

push_bulk

/transactions

Apologies for jumping in without context, but

Perhaps we should exclude outliers in /transactions?

the outlier flag seems like a poor way to decide whether we should push this data. (Yes, outliers shouldn't be sent over /transactions, but there are probably many other events which shouldn't be sent).

FederationEventHandler._process_received_pdu has a backfilled parameter (see

synapse/synapse/handlers/federation_event.py

Line 945 in b09d90c

backfilled: bool = False,

), whose purpose is slightly unclear, but I think one of its jobs is this sort of thing. Maybe we should use similar logic to that, somehow?

Whatever we decide on to make /sync and /transactions behave similarly, we can tackle in another PR. This PR is still good to ship on its own, i.e. It's a good idea to mark historical state as historical

FederationEventHandler._process_received_pdu has a backfilled parameter [...]
Maybe we should use similar logic to that, somehow?

For /transactions, we can't tell whether the event was backfilled. The only indication is that the stream_ordering would be negative which is what the /transactions code already takes into account. And is why the fix in this PR to mark the historical state events historical -> backfilled works to not show up in /transactions.

I just suggested adding outlier to /transactions query as well so it would match the query in /sync

Created #11394 to track this ⏩

erikjohnston · 2021-11-16T14:24:57Z

This approach seems reasonable to me as a stop gap before we try and use the same rules for sending events down /sync and /transactions

MadLittleMods · 2021-11-17T01:18:00Z

@erikjohnston Approval please 🙂

Do you want @richvdh to approve as well before merge?

richvdh · 2021-11-17T14:22:39Z

given erik has the context here, I'm happy to defer to him, if he has the bandwidth to look at it. I was just making driveby comments before!

MadLittleMods · 2021-11-18T20:16:49Z

Thanks for the review @erikjohnston and @richvdh 🐖

@tulir

Synapse 1.48.0 (2021-11-30) =========================== This release removes support for the long-deprecated `trust_identity_server_for_password_resets` configuration flag. This release also fixes some performance issues with some background database updates introduced in Synapse 1.47.0. No significant changes since 1.48.0rc1. Synapse 1.48.0rc1 (2021-11-25) ============================== Features -------- - Experimental support for the thread relation defined in [MSC3440](matrix-org/matrix-spec-proposals#3440). ([\#11161](matrix-org/synapse#11161)) - Support filtering by relation senders & types per [MSC3440](matrix-org/matrix-spec-proposals#3440). ([\#11236](matrix-org/synapse#11236)) - Add support for the `/_matrix/client/v3` and `/_matrix/media/v3` APIs from Matrix v1.1. ([\#11318](matrix-org/synapse#11318), [\#11371](matrix-org/synapse#11371)) - Support the stable version of [MSC2778](matrix-org/matrix-spec-proposals#2778): the `m.login.application_service` login type. Contributed by @tulir. ([\#11335](matrix-org/synapse#11335)) - Add a new version of delete room admin API `DELETE /_synapse/admin/v2/rooms/<room_id>` to run it in the background. Contributed by @dklimpel. ([\#11223](matrix-org/synapse#11223)) - Allow the admin [Delete Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#delete-room-api) to block a room without the need to join it. ([\#11228](matrix-org/synapse#11228)) - Add an admin API to un-shadow-ban a user. ([\#11347](matrix-org/synapse#11347)) - Add an admin API to run background database schema updates. ([\#11352](matrix-org/synapse#11352)) - Add an admin API for blocking a room. ([\#11324](matrix-org/synapse#11324)) - Update the JWT login type to support custom a `sub` claim. ([\#11361](matrix-org/synapse#11361)) - Store and allow querying of arbitrary event relations. ([\#11391](matrix-org/synapse#11391)) Bugfixes -------- - Fix a long-standing bug wherein display names or avatar URLs containing null bytes cause an internal server error when stored in the DB. ([\#11230](matrix-org/synapse#11230)) - Prevent [MSC2716](matrix-org/matrix-spec-proposals#2716) historical state events from being pushed to an application service via `/transactions`. ([\#11265](matrix-org/synapse#11265)) - Fix a long-standing bug where uploading extremely thin images (e.g. 1000x1) would fail. Contributed by @Neeeflix. ([\#11288](matrix-org/synapse#11288)) - Fix a bug, introduced in Synapse 1.46.0, which caused the `check_3pid_auth` and `on_logged_out` callbacks in legacy password authentication provider modules to not be registered. Modules using the generic module interface were not affected. ([\#11340](matrix-org/synapse#11340)) - Fix a bug introduced in 1.41.0 where space hierarchy responses would be incorrectly reused if multiple users were to make the same request at the same time. ([\#11355](matrix-org/synapse#11355)) - Fix a bug introduced in 1.45.0 where the `read_templates` method of the module API would error. ([\#11377](matrix-org/synapse#11377)) - Fix an issue introduced in 1.47.0 which prevented servers re-joining rooms they had previously left, if their signing keys were replaced. ([\#11379](matrix-org/synapse#11379)) - Fix a bug introduced in 1.13.0 where creating and publishing a room could cause errors if `room_list_publication_rules` is configured. ([\#11392](matrix-org/synapse#11392)) - Improve performance of various background database updates. ([\#11421](matrix-org/synapse#11421), [\#11422](matrix-org/synapse#11422)) Improved Documentation ---------------------- - Suggest users of the Debian packages add configuration to `/etc/matrix-synapse/conf.d/` to prevent, upon upgrade, being asked to choose between their configuration and the maintainer's. ([\#11281](matrix-org/synapse#11281)) - Fix typos in the documentation for the `username_available` admin API. Contributed by Stanislav Motylkov. ([\#11286](matrix-org/synapse#11286)) - Add Single Sign-On, SAML and CAS pages to the documentation. ([\#11298](matrix-org/synapse#11298)) - Change the word 'Home server' as one word 'homeserver' in documentation. ([\#11320](matrix-org/synapse#11320)) - Fix missing quotes for wildcard domains in `federation_certificate_verification_whitelist`. ([\#11381](matrix-org/synapse#11381)) Deprecations and Removals ------------------------- - Remove deprecated `trust_identity_server_for_password_resets` configuration flag. ([\#11333](matrix-org/synapse#11333), [\#11395](matrix-org/synapse#11395)) Internal Changes ---------------- - Add type annotations to `synapse.metrics`. ([\#10847](matrix-org/synapse#10847)) - Split out federated PDU retrieval function into a non-cached version. ([\#11242](matrix-org/synapse#11242)) - Clean up code relating to to-device messages and sending ephemeral events to application services. ([\#11247](matrix-org/synapse#11247)) - Fix a small typo in the error response when a relation type other than 'm.annotation' is passed to `GET /rooms/{room_id}/aggregations/{event_id}`. ([\#11278](matrix-org/synapse#11278)) - Drop unused database tables `room_stats_historical` and `user_stats_historical`. ([\#11280](matrix-org/synapse#11280)) - Require all files in synapse/ and tests/ to pass mypy unless specifically excluded. ([\#11282](matrix-org/synapse#11282), [\#11285](matrix-org/synapse#11285), [\#11359](matrix-org/synapse#11359)) - Add missing type hints to `synapse.app`. ([\#11287](matrix-org/synapse#11287)) - Remove unused parameters on `FederationEventHandler._check_event_auth`. ([\#11292](matrix-org/synapse#11292)) - Add type hints to `synapse._scripts`. ([\#11297](matrix-org/synapse#11297)) - Fix an issue which prevented the `remove_deleted_devices_from_device_inbox` background database schema update from running when updating from a recent Synapse version. ([\#11303](matrix-org/synapse#11303)) - Add type hints to storage classes. ([\#11307](matrix-org/synapse#11307), [\#11310](matrix-org/synapse#11310), [\#11311](matrix-org/synapse#11311), [\#11312](matrix-org/synapse#11312), [\#11313](matrix-org/synapse#11313), [\#11314](matrix-org/synapse#11314), [\#11316](matrix-org/synapse#11316), [\#11322](matrix-org/synapse#11322), [\#11332](matrix-org/synapse#11332), [\#11339](matrix-org/synapse#11339), [\#11342](matrix-org/synapse#11342)) - Add type hints to `synapse.util`. ([\#11321](matrix-org/synapse#11321), [\#11328](matrix-org/synapse#11328)) - Improve type annotations in Synapse's test suite. ([\#11323](matrix-org/synapse#11323), [\#11330](matrix-org/synapse#11330)) - Test that room alias deletion works as intended. ([\#11327](matrix-org/synapse#11327)) - Add type annotations for some methods and properties in the module API. ([\#11341](matrix-org/synapse#11341)) - Fix running `scripts-dev/complement.sh`, which was broken in v1.47.0rc1. ([\#11368](matrix-org/synapse#11368)) - Rename internal functions for token generation to better reflect what they do. ([\#11369](matrix-org/synapse#11369), [\#11370](matrix-org/synapse#11370)) - Add type hints to configuration classes. ([\#11377](matrix-org/synapse#11377)) - Publish a `develop` image to Docker Hub. ([\#11380](matrix-org/synapse#11380)) - Keep fallback key marked as used if it's re-uploaded. ([\#11382](matrix-org/synapse#11382)) - Use `auto_attribs` on the `attrs` class `RefreshTokenLookupResult`. ([\#11386](matrix-org/synapse#11386)) - Rename unstable `access_token_lifetime` configuration option to `refreshable_access_token_lifetime` to make it clear it only concerns refreshable access tokens. ([\#11388](matrix-org/synapse#11388)) - Do not run the broken MSC2716 tests when running `scripts-dev/complement.sh`. ([\#11389](matrix-org/synapse#11389)) - Remove dead code from supporting ACME. ([\#11393](matrix-org/synapse#11393)) - Refactor including the bundled relations when serializing an event. ([\#11408](matrix-org/synapse#11408))

@tulir

Synapse 1.48.0 (2021-11-30) =========================== This release removes support for the long-deprecated `trust_identity_server_for_password_resets` configuration flag. This release also fixes some performance issues with some background database updates introduced in Synapse 1.47.0. No significant changes since 1.48.0rc1. Synapse 1.48.0rc1 (2021-11-25) ============================== Features -------- - Experimental support for the thread relation defined in [MSC3440](matrix-org/matrix-spec-proposals#3440). ([\matrix-org#11161](matrix-org#11161)) - Support filtering by relation senders & types per [MSC3440](matrix-org/matrix-spec-proposals#3440). ([\matrix-org#11236](matrix-org#11236)) - Add support for the `/_matrix/client/v3` and `/_matrix/media/v3` APIs from Matrix v1.1. ([\matrix-org#11318](matrix-org#11318), [\matrix-org#11371](matrix-org#11371)) - Support the stable version of [MSC2778](matrix-org/matrix-spec-proposals#2778): the `m.login.application_service` login type. Contributed by @tulir. ([\matrix-org#11335](matrix-org#11335)) - Add a new version of delete room admin API `DELETE /_synapse/admin/v2/rooms/<room_id>` to run it in the background. Contributed by @dklimpel. ([\matrix-org#11223](matrix-org#11223)) - Allow the admin [Delete Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#delete-room-api) to block a room without the need to join it. ([\matrix-org#11228](matrix-org#11228)) - Add an admin API to un-shadow-ban a user. ([\matrix-org#11347](matrix-org#11347)) - Add an admin API to run background database schema updates. ([\matrix-org#11352](matrix-org#11352)) - Add an admin API for blocking a room. ([\matrix-org#11324](matrix-org#11324)) - Update the JWT login type to support custom a `sub` claim. ([\matrix-org#11361](matrix-org#11361)) - Store and allow querying of arbitrary event relations. ([\matrix-org#11391](matrix-org#11391)) Bugfixes -------- - Fix a long-standing bug wherein display names or avatar URLs containing null bytes cause an internal server error when stored in the DB. ([\matrix-org#11230](matrix-org#11230)) - Prevent [MSC2716](matrix-org/matrix-spec-proposals#2716) historical state events from being pushed to an application service via `/transactions`. ([\matrix-org#11265](matrix-org#11265)) - Fix a long-standing bug where uploading extremely thin images (e.g. 1000x1) would fail. Contributed by @Neeeflix. ([\matrix-org#11288](matrix-org#11288)) - Fix a bug, introduced in Synapse 1.46.0, which caused the `check_3pid_auth` and `on_logged_out` callbacks in legacy password authentication provider modules to not be registered. Modules using the generic module interface were not affected. ([\matrix-org#11340](matrix-org#11340)) - Fix a bug introduced in 1.41.0 where space hierarchy responses would be incorrectly reused if multiple users were to make the same request at the same time. ([\matrix-org#11355](matrix-org#11355)) - Fix a bug introduced in 1.45.0 where the `read_templates` method of the module API would error. ([\matrix-org#11377](matrix-org#11377)) - Fix an issue introduced in 1.47.0 which prevented servers re-joining rooms they had previously left, if their signing keys were replaced. ([\matrix-org#11379](matrix-org#11379)) - Fix a bug introduced in 1.13.0 where creating and publishing a room could cause errors if `room_list_publication_rules` is configured. ([\matrix-org#11392](matrix-org#11392)) - Improve performance of various background database updates. ([\matrix-org#11421](matrix-org#11421), [\matrix-org#11422](matrix-org#11422)) Improved Documentation ---------------------- - Suggest users of the Debian packages add configuration to `/etc/matrix-synapse/conf.d/` to prevent, upon upgrade, being asked to choose between their configuration and the maintainer's. ([\matrix-org#11281](matrix-org#11281)) - Fix typos in the documentation for the `username_available` admin API. Contributed by Stanislav Motylkov. ([\matrix-org#11286](matrix-org#11286)) - Add Single Sign-On, SAML and CAS pages to the documentation. ([\matrix-org#11298](matrix-org#11298)) - Change the word 'Home server' as one word 'homeserver' in documentation. ([\matrix-org#11320](matrix-org#11320)) - Fix missing quotes for wildcard domains in `federation_certificate_verification_whitelist`. ([\matrix-org#11381](matrix-org#11381)) Deprecations and Removals ------------------------- - Remove deprecated `trust_identity_server_for_password_resets` configuration flag. ([\matrix-org#11333](matrix-org#11333), [\matrix-org#11395](matrix-org#11395)) Internal Changes ---------------- - Add type annotations to `synapse.metrics`. ([\matrix-org#10847](matrix-org#10847)) - Split out federated PDU retrieval function into a non-cached version. ([\matrix-org#11242](matrix-org#11242)) - Clean up code relating to to-device messages and sending ephemeral events to application services. ([\matrix-org#11247](matrix-org#11247)) - Fix a small typo in the error response when a relation type other than 'm.annotation' is passed to `GET /rooms/{room_id}/aggregations/{event_id}`. ([\matrix-org#11278](matrix-org#11278)) - Drop unused database tables `room_stats_historical` and `user_stats_historical`. ([\matrix-org#11280](matrix-org#11280)) - Require all files in synapse/ and tests/ to pass mypy unless specifically excluded. ([\matrix-org#11282](matrix-org#11282), [\matrix-org#11285](matrix-org#11285), [\matrix-org#11359](matrix-org#11359)) - Add missing type hints to `synapse.app`. ([\matrix-org#11287](matrix-org#11287)) - Remove unused parameters on `FederationEventHandler._check_event_auth`. ([\matrix-org#11292](matrix-org#11292)) - Add type hints to `synapse._scripts`. ([\matrix-org#11297](matrix-org#11297)) - Fix an issue which prevented the `remove_deleted_devices_from_device_inbox` background database schema update from running when updating from a recent Synapse version. ([\matrix-org#11303](matrix-org#11303)) - Add type hints to storage classes. ([\matrix-org#11307](matrix-org#11307), [\matrix-org#11310](matrix-org#11310), [\matrix-org#11311](matrix-org#11311), [\matrix-org#11312](matrix-org#11312), [\matrix-org#11313](matrix-org#11313), [\matrix-org#11314](matrix-org#11314), [\matrix-org#11316](matrix-org#11316), [\matrix-org#11322](matrix-org#11322), [\matrix-org#11332](matrix-org#11332), [\matrix-org#11339](matrix-org#11339), [\matrix-org#11342](matrix-org#11342)) - Add type hints to `synapse.util`. ([\matrix-org#11321](matrix-org#11321), [\matrix-org#11328](matrix-org#11328)) - Improve type annotations in Synapse's test suite. ([\matrix-org#11323](matrix-org#11323), [\matrix-org#11330](matrix-org#11330)) - Test that room alias deletion works as intended. ([\matrix-org#11327](matrix-org#11327)) - Add type annotations for some methods and properties in the module API. ([\matrix-org#11341](matrix-org#11341)) - Fix running `scripts-dev/complement.sh`, which was broken in v1.47.0rc1. ([\matrix-org#11368](matrix-org#11368)) - Rename internal functions for token generation to better reflect what they do. ([\matrix-org#11369](matrix-org#11369), [\matrix-org#11370](matrix-org#11370)) - Add type hints to configuration classes. ([\matrix-org#11377](matrix-org#11377)) - Publish a `develop` image to Docker Hub. ([\matrix-org#11380](matrix-org#11380)) - Keep fallback key marked as used if it's re-uploaded. ([\matrix-org#11382](matrix-org#11382)) - Use `auto_attribs` on the `attrs` class `RefreshTokenLookupResult`. ([\matrix-org#11386](matrix-org#11386)) - Rename unstable `access_token_lifetime` configuration option to `refreshable_access_token_lifetime` to make it clear it only concerns refreshable access tokens. ([\matrix-org#11388](matrix-org#11388)) - Do not run the broken MSC2716 tests when running `scripts-dev/complement.sh`. ([\matrix-org#11389](matrix-org#11389)) - Remove dead code from supporting ACME. ([\matrix-org#11393](matrix-org#11393)) - Refactor including the bundled relations when serializing an event. ([\matrix-org#11408](matrix-org#11408))

MadLittleMods · 2022-05-12T18:18:08Z

synapse/appservice/api.py

@@ -231,13 +231,32 @@ async def push_bulk(
                json_body=body,
                args={"access_token": service.hs_token},
            )
+            if logger.isEnabledFor(logging.DEBUG):


I came back to this and was confused why I did this since it's guarding debug against debug.

AFAICT, this is to avoid potentially expensive string construction when we loop through all of the events here. We do this in a few other places in the codebase too.

Mark historical state as historical internally

95624a6

Part of #11241

MadLittleMods requested a review from a team as a code owner November 5, 2021 22:10

MadLittleMods marked this pull request as draft November 5, 2021 22:10

MadLittleMods removed the request for review from a team November 5, 2021 22:10

MadLittleMods mentioned this pull request Nov 5, 2021

Draft: Make sure historical state events don't come down /transactions for application services (MSC2716) matrix-org/complement#221

Closed

MadLittleMods added a commit to matrix-org/complement that referenced this pull request Nov 6, 2021

Make sure historical events aren't sent as AS transactions

7ab71d4

Regression tests for matrix-org/synapse#11241 Synapse changes: matrix-org/synapse#11265

Add useful debug logs while making a fake AS /transactions endpoint i…

c30a35b

…n Complement Complement tests: matrix-org/complement#221

MadLittleMods commented Nov 6, 2021

View reviewed changes

synapse/appservice/api.py Show resolved Hide resolved

MadLittleMods commented Nov 6, 2021

View reviewed changes

MadLittleMods changed the title ~~Draft: Prevent historical state from being pushed to an application service via /transactions~~ Prevent historical state from being pushed to an application service via /transactions Nov 6, 2021

MadLittleMods added the A-Application-Service Related to AS support label Nov 6, 2021

MadLittleMods changed the title ~~Prevent historical state from being pushed to an application service via /transactions~~ Prevent historical state from being pushed to an application service via /transactions (MSC2716) Nov 6, 2021

MadLittleMods marked this pull request as ready for review November 6, 2021 06:00

Add changelog

64df8cd

MadLittleMods force-pushed the madlittlemods/msc2716-no-push-historical-state branch from eaf4d9a to 64df8cd Compare November 6, 2021 06:01

MadLittleMods requested a review from a team November 6, 2021 07:17

erikjohnston reviewed Nov 9, 2021

View reviewed changes

Add exc_info to debug stack trace

e580d47

MadLittleMods requested a review from erikjohnston November 9, 2021 23:37

MadLittleMods mentioned this pull request Nov 10, 2021

Refactor function parameters to describe the behavior instead of the state which is interpreted in non-obvious ways #11300

Closed

4 tasks

Add historical to commentdoc

15e3246

MadLittleMods requested a review from richvdh November 11, 2021 07:42

richvdh removed their request for review November 17, 2021 14:22

erikjohnston approved these changes Nov 18, 2021

View reviewed changes

MadLittleMods merged commit 7ffddd8 into develop Nov 18, 2021

MadLittleMods deleted the madlittlemods/msc2716-no-push-historical-state branch November 18, 2021 20:16

MadLittleMods mentioned this pull request Nov 18, 2021

Incremental /sync and /transactions query events differently #11394

Open

MadLittleMods commented May 12, 2022

View reviewed changes

matrixbot mentioned this pull request Dec 21, 2023

Incremental /sync and /transactions query events differently element-hq/synapse#11394

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent historical state from being pushed to an application service via `/transactions` (MSC2716) #11265

Prevent historical state from being pushed to an application service via `/transactions` (MSC2716) #11265

MadLittleMods commented Nov 5, 2021 •

edited

Loading

MadLittleMods Nov 6, 2021

MadLittleMods Nov 6, 2021

richvdh Nov 10, 2021

MadLittleMods Nov 10, 2021

erikjohnston Nov 9, 2021

MadLittleMods Nov 9, 2021 •

edited

Loading

richvdh Nov 10, 2021

MadLittleMods Nov 11, 2021

MadLittleMods Nov 18, 2021

erikjohnston commented Nov 16, 2021

MadLittleMods commented Nov 17, 2021

richvdh commented Nov 17, 2021

MadLittleMods commented Nov 18, 2021

MadLittleMods May 12, 2022

	sql = """
	SELECT event_id, instance_name, topological_ordering, stream_ordering
	FROM events
	WHERE
	room_id = ?
	AND not outlier
	AND stream_ordering > ? AND stream_ordering <= ?
	ORDER BY stream_ordering %s LIMIT ?
	""" % (

	def get_new_events_for_appservice_txn(txn):
	sql = (
	"SELECT e.stream_ordering, e.event_id"
	" FROM events AS e"
	" WHERE"
	" (SELECT stream_ordering FROM appservice_stream_position)"
	" < e.stream_ordering"
	" AND e.stream_ordering <= ?"
	" ORDER BY e.stream_ordering ASC"
	" LIMIT ?"
	)

		@@ -0,0 +1 @@
		Prevent [MSC2716](https://github.com/matrix-org/matrix-doc/pull/2716) historical state events from being pushed to an application service via `/transactions`.

Prevent historical state from being pushed to an application service via /transactions (MSC2716) #11265

Prevent historical state from being pushed to an application service via /transactions (MSC2716) #11265

Conversation

MadLittleMods commented Nov 5, 2021 • edited Loading

Dev notes

/sync stack trace

/transactions stack trace

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MadLittleMods Nov 9, 2021 • edited Loading

Choose a reason for hiding this comment

/sync stack trace

/transactions stack trace

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikjohnston commented Nov 16, 2021

MadLittleMods commented Nov 17, 2021

richvdh commented Nov 17, 2021

MadLittleMods commented Nov 18, 2021

Choose a reason for hiding this comment

Prevent historical state from being pushed to an application service via `/transactions` (MSC2716) #11265

Prevent historical state from being pushed to an application service via `/transactions` (MSC2716) #11265

MadLittleMods commented Nov 5, 2021 •

edited

Loading

`/sync` stack trace

`/transactions` stack trace

MadLittleMods Nov 9, 2021 •

edited

Loading

`/sync` stack trace

`/transactions` stack trace