Skip to content

Update PluginMessageMessenger.java#2674

Closed
ghost wants to merge 1 commit into
masterfrom
unknown repository
Closed

Update PluginMessageMessenger.java#2674
ghost wants to merge 1 commit into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Oct 18, 2020

Verify the event is cancelled

Verify the event is cancelled
@lucko
Copy link
Copy Markdown
Member

lucko commented Oct 19, 2020

Is this addressing a particular bug? It shouldn't be necessary to check for cancellations - other plugins should have no reason to cancel our messages - and if they do, then they should probably be cancelled via the LuckPerms events.

@lucko lucko closed this Oct 19, 2020
@linsaftw
Copy link
Copy Markdown

linsaftw commented Oct 19, 2020

Hi, plugins like ExploitFixer limit the amount of PluginMessages because players can spam them with stuff like books/custompayload to cause the server to crash, and it is a good practice to always check that events are cancelled, it makes other developers life a pain when developers don't ignore on cancellation.

Cancellation is super important and has to be done before anything else is done, join event is important to be properly cancelled too as machines can be used to spam connections.

Anyways, why do you want to run LuckPerms when it's cancelled?

@lucko
Copy link
Copy Markdown
Member

lucko commented Oct 20, 2020

Messages sent from players are ignored:

https://github.com/lucko/LuckPerms/blob/7d47b31bcb0502e5137494910b4e061d7040be16/bungee/src/main/java/me/lucko/luckperms/bungee/messaging/PluginMessageMessenger.java#L96-L98

They're only accepted in the other "direction" - from the backend server. It would therefore not be possible for a malicious user to exploit/packet spam this system.

It is good practice to consider whether events are cancelled and how to react when that is the case. It is not good practice to blindly ignore cancelled events always - otherwise that would just be the default behaviour, wouldn't it?

The reason for not ignoring cancelled events in this case was explained in my previous response:

It shouldn't be necessary to check for cancellations - other plugins should have no reason to cancel our messages - and if they do, then they should probably be cancelled via the LuckPerms events.

@ghost ghost deleted the patch-1 branch October 20, 2020 20:37
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

Successfully merging this pull request may close these issues.

2 participants