Skip to content

Commit 4ec0095

Browse files
committed
MSC2781: Down with the fallbacks
Signed-off-by: Nicolas Werner <[email protected]>
1 parent 81c7893 commit 4ec0095

File tree

1 file changed

+151
-0
lines changed

1 file changed

+151
-0
lines changed
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# MSC2781: Deprecate fallbacks in the specification
2+
3+
Currently replies require clients to send and parse a fallback representation of
4+
the replied to message. Something similar is planned for edits. While at least
5+
in theory fallbacks should make it simpler to start supporting replies in a new
6+
client, they actually introduce a lot of complexity and implementation issues
7+
and block a few valuable features. This MSC proposes to deprecate and eventually
8+
remove those fallbacks. It is an alternative to
9+
[MSC2589](https://github.com/matrix-org/matrix-doc/pull/2589), which intends to
10+
double down on fallbacks.
11+
12+
### Issues with the current fallbacks
13+
14+
#### Stripping the fallback
15+
16+
To reply to a reply, a client needs to strip the existing fallback of the first
17+
reply. Otherwise replies will just infinitely nest replies. [While the spec doesn't necessarily require that](https://github.com/matrix-org/matrix-doc/issues/1541),
18+
not doing so risks running into the event size limit, but more importantly, it
19+
just leads to a bad experience for clients actually relying on the fallback.
20+
21+
Stripping the fallback sadly is not trivial. Most clients did that wrong at some
22+
point. At the point of writing this, FluffyChat seems to fail stripping the
23+
fallback, while Riot/Element did fail to do so in the past, when `<mx-reply>`
24+
tags were not evenly matched. Since clients can send arbitrary byte sequences,
25+
every client needs to basically parse untrusted input and sanitize it. A normal
26+
html parser will fail stripping a `</mx-reply><mx-reply></mx-reply>` sequence in
27+
some cases. If you use a regex to strip the fallback, you may be able to be
28+
attacked using regex DOS attacks (although that is somewhat mitigated by the
29+
maximum event size). In the end there are quite a few edge cases, that are not
30+
covered in the existing specification, since it doesn't specify the exact rules
31+
to strip a fallback at all.
32+
33+
Stripping the fallback from body is even more complicated, since there is no way
34+
to distinguish a quote from a reply reliably.
35+
36+
#### Creating a new fallback
37+
38+
To create a new fallback, a client needs to add untrusted html to its own
39+
events. This is an easy attack vector to inject your own content into someone
40+
elses reply. While this can be prevented with enough care, since Riot basically
41+
had to fix this issue twice, it can be expected that other clients can also be
42+
affected by this.
43+
44+
#### Requirement of html for replies
45+
46+
The spec requires rich replies to have a fallback using html:
47+
48+
> Rich replies MUST have a format of org.matrix.custom.html and therefore a formatted_body alongside the body and appropriate msgtype.
49+
50+
This means you can't reply using only a `body` and you can't reply with an
51+
image, since those don't have a `formatted_body` property currently. This means
52+
a text only client, that doesn't want to display html, still needs to support
53+
html anyway and that new features are blocked, because of fallbacks.
54+
55+
This is also an issue with edits, where the combination of edit fallback and
56+
reply fallback is somewhat interesting: You need to send a fallback, since the
57+
event is still a reply, but you can't send a reply relation, since this is an
58+
edit. So for clients relying on the edit fallback, you send a broken reply
59+
fallback, that doesn't get stripped, even when the client supports rich replies.
60+
61+
#### Replies leak history
62+
63+
A reply includes the `body` of another event. This means a reply to an event can
64+
leak data to users, that joined this room at a later point, but shouldn't be
65+
able to see the event because of visibility rules or encryption. While this
66+
isn't a big issue, there is still an issue about it: https://github.com/matrix-org/matrix-doc/issues/1654
67+
68+
Replies are also sometimes localized. In those cases they leak the users
69+
language selection for their client, which may be personal information.
70+
71+
#### Low value of replies
72+
73+
The above issues would be somewhat acceptable, if reply fallbacks would provide
74+
some value to clients. Generally they don't though. The Qt html renderer breaks
75+
on `<mx-reply>` tags, so you need to strip them anyway to render replies.
76+
Bridges usually try to bridge to native replies, so they need to strip the reply
77+
part (https://github.com/matrix-org/matrix-doc/issues/1541). Even the IRC bridge
78+
seems to send a custom fallback, because the default fallback is not that
79+
welcome to the IRC crowd.
80+
81+
Clients that actually use the fallback (original Riot/Android for example) tend
82+
to do so for quite some while, because the fallbacks are "good enough", but in
83+
the end that provides a worse experience for everyone involved. For example you
84+
couldn't see what image was replied to for the longest time. Just "X replied to
85+
an image with: I like this one" is not valuable, when 3 images were sent.
86+
87+
Only replies to actual text messages have a somewhat reasonable fallback. The
88+
other ones do not provide any more value than a plain "this is a reply" tag,
89+
unless the client also already supports event links.
90+
91+
## Proposal
92+
93+
Deprecate the rich reply fallback. Clients should stop sending them and should
94+
consider treating `<mx-reply>` parts as either something to be unconditionally
95+
stripped or as something to be escaped as invalid html. In the future the
96+
fallback should be removed from the spec completely with only a note left, that
97+
it may exist in old events.
98+
99+
Furthermore, no fallback should be used for edits, since just adding an asterisk
100+
before the same message does not provide much value to users and it complicates
101+
the implementation of edits for no tangible benefit.
102+
103+
The fallback for more niche features, like in room verification can stay, since
104+
that feature is actually somewhat hard to implement and some clients may never
105+
support encryption.
106+
107+
As a result of this, you would be able to reply with an image or edit a video.
108+
New clients would also be able to implement edits and replies more easily, as
109+
they can sidestep a lot of pitfalls.
110+
111+
## Potential issues
112+
113+
Obviously you can't remove the fallback from old events. As such clients would
114+
still need to do something with them in the near future. I'd say just not
115+
handling them in a special way should be okay after some unspecified period of
116+
time.
117+
118+
Clients not implementing rich replies or edits may show some slightly more
119+
confusing messages to users as well. I'd argue though that in most cases, the
120+
reply is close enough to the replied to message, that you would be able to guess
121+
the correct context. Replies would also be somewhat easier to implement and
122+
worst case, a client could very easily implement a little "this is a reply"
123+
marker to at least mark replies visually.
124+
125+
Same applies to edits. If 2 very similar messages appear after one another,
126+
someone new to online messaging would assume, this is a correction to the
127+
previous message. That may even be more obvious to them than if the message was
128+
prefixed with a `*`, since that has been confusing to users in the past. Since
129+
edits would now look exactly like a normal message, they would also be
130+
considerably easier to implement, since you jus need to replace the former
131+
message now, similar to a redaction, and not merge `content` and `new_content`.
132+
133+
## Alternatives
134+
135+
[MSC2589](https://github.com/matrix-org/matrix-doc/pull/2589): This adds the
136+
reply text as an addional key. While this solves the parsing issues, it
137+
doesn't address the other issues with fallbacks.
138+
139+
One could also just stick with the current fallbacks and make all clients pay
140+
the cost for a small number of clients actually benefitting from them.
141+
142+
## Security considerations
143+
144+
Removing the fallback from the spec may lead to issues, when clients experience
145+
the fallback in old events. This should not add any security issues the
146+
client didn't already have from interpreting untrusted html, though. In all
147+
other cases this should **reduce** security issues.
148+
149+
## Unstable prefix
150+
151+
Seems unnecessary, since this only removes stuff.

0 commit comments

Comments
 (0)