Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Reply is highlighted even without actual mention when domain matches username #6202

Closed
pacien opened this issue Oct 12, 2019 · 8 comments
Closed

Comments

@pacien
Copy link
Contributor

pacien commented Oct 12, 2019

Forwarded from element-hq/element-web#11132, as highlights are computed server-side.

Description

My username matches my domain (MXID of the form @<username>:<username>.tld).
As a result, all replies to messages from any other user on my homeserver are highlighted in my Riot session, flooding me with urgent notifications without intended mentions.

Steps to reproduce

Let @alice:alice.tld, @bob:alice.tld and @caroline:alice.tld be three users on the homeserver alice.tld. All three are members of a room in which:

  1. Bob sends a message that does not mention Alice.
  2. Caroline replies to Bob's message using the reply feature, still not mentionning Alice.
  3. Alice's Riot highlights Caroline's message and Alice is notified, even though nobody intended to mention her.

I haven't tested it, but the issue may probably also arise when someone's username is an infix of someone else's username: @pierre:domain.tld might get notified when someone replies to a message from @jean.pierre@domain.tld. To confirm.

Remediation?

Do not allow partial matches on MXIDs contained in messages for highlighting purposes.
A highlight should require the whole MXID to match.

Version information

  • Homeserver: personal one
  • Version: 1.3.1
  • Platform: all
@turt2live
Copy link
Member

Let's track this from the client perspective for now: element-hq/element-web#7874

@matejdro
Copy link

@turt2live It doesn't look like linked riot web issue is related to this at all. Also the issue is on the backend (synapse controls the notifications), so why track it from the client?

@lieuwex
Copy link

lieuwex commented Aug 17, 2020

Asked this first on element-hq/element-web#7874 but was deemed off-topic, so reposting here:

Is there any reason why synapse doesn't first strip the fallback before checking for push rules?

@clokep
Copy link
Member

clokep commented Aug 17, 2020

@lieuwex The specification seems to say that's up to a client (quoting from that link):

Clients which support rich replies MUST strip the fallback from the event before rendering the event.

@lieuwex
Copy link

lieuwex commented Aug 17, 2020

@clokep Doesn't that only suggest for rendering the content. Not for applying push rules?

For push rules I think it's more intuitive to strip the fallback before applying the rules, since it would solve issues like this one.
I didn't find something in the spec that tells something about stripping the fallback before applying the push rules, so maybe this is more a spec thing. But I don't see a reason why it would be against the spec either.

A client would strip the fallback (iff the client support rich replies), this idea could also be extended onto the server behavior on push rules.
One could see the notification producer as a client on its own, "rendering" the message content to a notification.

@clokep
Copy link
Member

clokep commented Aug 18, 2020

A client would strip the fallback (iff the client support rich replies), this idea could also be extended onto the server behavior on push rules.
One could see the notification producer as a client on its own, "rendering" the message content to a notification.

Ah, I see your point here. It's a bit confusing when it specifically says "client"! 😄

Looks like matrix-org/matrix-doc#2736 got filed to clarify what servers are supposed to do here.

@lieuwex
Copy link

lieuwex commented Aug 18, 2020

What a coincidence that that issue is opened recently. But that issue seems like a good one to use.

@JuniorJPDJ
Copy link

There's more apropriate issue for this problem ;)
matrix-org/matrix-spec-proposals#2735
Problem consists from both of them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants