Skip to content
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

allowed_mentions wiped on PATCH request #1864

Closed
Jiralite opened this issue Aug 4, 2020 · 9 comments
Closed

allowed_mentions wiped on PATCH request #1864

Jiralite opened this issue Aug 4, 2020 · 9 comments

Comments

@Jiralite
Copy link
Contributor

Jiralite commented Aug 4, 2020

When a PATCH request is performed to a message (that is making use of allowed_mentions), allowed_mentions is wiped. This means that the mention border will appear to anyone mentioned implying they were mentioned.

A simple way to reproduce this would be to:

  1. Create a message that mentions yourself
  2. Making use of allowed_mentions, ensure that no one is to be mentioned
  3. Send the message
  4. Edit the message
  5. Reload the client (as the mention border will not update immediately)

This can also be reproduced in the client by suppressing embeds as this involves making a PATCH request:

  1. Create a message that mentions yourself and has an embed (or a link that comes with a preview)
  2. Making use of allowed_mentions, ensure that no one is to be mentioned
  3. Send the message
  4. Suppress embeds on the message by clicking the "x"
  5. Reload the client (as the mention border will not update immediately)

Upon following the above steps for either method, you will find that the mention border has appeared!

Similarly, both the above methods can be reproduced via replies with the mention toggle set to off. Simply reply to yourself (or anyone else with the mention toggle set to off) and edit the message or suppress embeds on the message and reload the client - the mention border will appear.

I believe this can be fixed by Discord supplying which users or roles should not be mentioned via an allowed_mentions property in the message object or alike. This way, we can utilise this every time we need to perform a PATCH request to ensure allowed_mentions does not become wiped and retains which users or roles should not be mentioned.

@advaith1
Copy link
Contributor

advaith1 commented Aug 4, 2020

The issue seems to be that if you patch a message without the allowed_mentions property, then it resets it, while it should leave it unmodified and only reset if null is sent. When suppressing embeds via the client, it only sends the flags property.

@xXBuilderBXx
Copy link

xXBuilderBXx commented Aug 23, 2020

This issue also effects modifying a message it will reset the allowed mentions property.
Lib devs are saying that this is a Discord issue because it should not edit properties that you don't specify.
Image

@SinisterRectus
Copy link
Contributor

I think that was supposed to be fixed with #1419/#1483? You can include the field in the patch data, but I don't know why it's cleared when omitted.

@xXBuilderBXx
Copy link

Ok so this was supposed to be fixed but apparently not good to know also i'm using discord.net and they don't include any allowed mentions when modifying a message so it should never be reset.

@advaith1
Copy link
Contributor

yeah it's a bug but in the meantime you can just include the current properties in the patch (which would probably be better for the library to handle)

@kajfik
Copy link

kajfik commented Sep 30, 2020

It seems that suppressing embeds on a message that contains a mention does ping in certain circumstances.

We have managed to reproduce the ping with these steps:

  • Person A using Android has their app open and is not looking at channel X in server Y (they are instead looking at another channel in another server)
  • Person B using desktop posts a message that contains "@​everyone" and a link to image such as "@​everyone (sorry for the ping) https://i.imgur.com/ECaZgF2.png" to channel X on server Y
  • Person B doesn't have permission to mention @​everyone so this message doesn't show a ping notification
  • Person C using desktop is an Administrator on server Y and removes the image embeds from message posted by person B
  • A ping notification is shown to person A and the message shows with orange ping border
  • Persons B and C will see the orange ping border after they reload their client.

In our testing pesron A was using OnePlus 8 Pro with Android 10 and Discord app version 40.8 (1277), and persons B and C were using the latest version of the Stable desktop client.
Person A sees a ping notification after Person C removes embed
The message shows with orange ping border to person A
The message shows with no orange to person C
The message shows with orange ping border to person C after they refresh their desktop client with Ctrl+R

In case that there's something with the message that causes this bug, here is the exact copy of the message we have tested it with:

@everyone (sorry for the ping) I was just contacted by a scammer trying to scam me out of my Steam account using a "pending ban" scam while impersonating a Valve employee. 
I'm posting this in all servers I'm in since I don't know where he found me :Ü™️
Screenshots:
http://i.imgur.com/rsasYXv.png
http://i.imgur.com/NL0aiqA.png
https://i.imgur.com/wdnDix1.png

We have tested this issue after it happened on our server and caused a sudden large influx of users which forced us to enable slowmode. If there's any other details I can provide that would help with reproduction please let me know.

@night
Copy link
Member

night commented Apr 8, 2021

This looks to be working as intended. allowed_mentions are per-request and do not persist on messages. You will need to pass the expected allowed_mentions with your PATCH as well.

@night night closed this as completed Apr 8, 2021
@Rapptz
Copy link
Contributor

Rapptz commented Apr 8, 2021

@night the issue is that if you don't pass allowed_mentions it gets wiped. This doesn't happen with any other field, for example if you don't pass content your message content does not disappear. Has this been fixed or is this really just intended? The behaviour is surprising for everyone if so.

@night
Copy link
Member

night commented Apr 9, 2021

I see. In the next API deploy we will stop reparsing content for mentions in PATCHes when it does not change

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

No branches or pull requests

7 participants