-
Notifications
You must be signed in to change notification settings - Fork 415
MSC4169: Backwards-compatible redaction sending using /send
#4169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tulir
wants to merge
8
commits into
main
Choose a base branch
from
tulir/send-redact-backwards-compatibility
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
cc79f45
Proposal for backwards-compatible redaction sending using `/send`
tulir d3ee606
Clarify dependencies and add todo
tulir f22bb89
Remove mass redaction support in favor of MSC4343 and add unstable fe…
tulir 1422f83
Merge branch 'main' into tulir/send-redact-backwards-compatibility
tulir 94ec653
Undeprecate /redact
tulir cbd5f10
Apply suggestions from code review
tulir c4bc1f4
Reword introduction
tulir b9ccea4
Repeat why not /redact in alternatives section
tulir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| # MSC4169: Backwards-compatible redaction sending using `/send` | ||
| [MSC2174] moved the `redacts` key from the top level to inside `content` for `v11` and later room versions. It | ||
| also defined forwards- and backwards-compatibility for receiving by defining | ||
| that servers will copy the key to both locations when serving events to clients, regardless of room version. | ||
| The `/redact` endpoint is also forwards-compatible because it works in all room | ||
| versions. However, `/send` was not made backwards-compatible. This means that | ||
| clients can't switch to using `/send` unless they keep track of room versions. | ||
|
|
||
| The inability to use `/send` in all room versions makes it more difficult for | ||
| clients to abstract event sending without special-casing redactions and to | ||
| implement features such as self-destructing messages (utilising [MSC4140]'s | ||
| delayed events functionality, which is only implemented for `/send`). | ||
|
|
||
| [MSC4140]: https://github.com/matrix-org/matrix-spec-proposals/pull/4140 | ||
| [MSC2174]: https://github.com/matrix-org/matrix-spec-proposals/pull/2174 | ||
|
|
||
| ## Proposal | ||
| The proposed solution is to have the server move the `redacts` key from the | ||
| content to the top level of the event when `/send/m.room.redaction` is called | ||
| in a pre-v11 room. | ||
|
|
||
| ## Potential issues | ||
| Servers that don't support this MSC may behave unexpectedly if a client tries | ||
| to redact using `/send` in an old room. Synapse currently throws an assertion | ||
| error (which turns into HTTP 500) when trying to redact with /send in a pre-v11 | ||
| room, which shouldn't cause any issues other than the request failing. Clients | ||
| can avoid issues by confirming server support using `/versions` first. | ||
|
|
||
| ## Alternatives | ||
| ### Client-side switching | ||
| Clients could remember room versions themselves and adjust the endpoint based | ||
| on that. This works fine for thick clients which cache data locally anyway, but | ||
| does not work for things like bots which have minimal local state. | ||
|
|
||
| ### Just use `/redact` | ||
| `/redact` works fine for all room versions. We could discourage using `/send` | ||
| for redactions and prefer `/redact` instead. | ||
tulir marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ### Deprecate `/redact` | ||
| In addition to allowing `/send`, we could encourage everyone to switch over by | ||
| formally deprecating `/redact`. However, there are some other sugar APIs like | ||
| `/ban` which do nothing `/state` can't, so `/redact` is also kept fully | ||
| supported for now. A future MSC may choose to deprecate it. | ||
|
|
||
| ## Security considerations | ||
| This allows clients to send arbitrary content in pre-v11 redaction events. | ||
| It shouldn't cause any security issues, as it's already possible in v11+ rooms | ||
| or with a modified server. | ||
|
|
||
| ## Unstable prefix | ||
| The endpoint itself has no unstable prefix as it can already be used to send | ||
| redactions in v11+ rooms. Support for the transformation in old room versions | ||
| can be detected using the `com.beeper.msc4169` unstable feature flag in the | ||
| `/versions` response. The feature flag should continue to be advertised after | ||
| the MSC is accepted until the server advertises support for the stable spec | ||
| release that includes this MSC. | ||
|
|
||
| ## Dependencies | ||
| None. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see MSC2174 specifically calling out either
/redactor/send. Was this an issue with server implementation?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there was any issue with
/redact, but servers did obviously have to make changes to the endpoint in room v11 even though the MSC didn't call it out explicitly.