LuckPerms

LuckPerms

41.4k Downloads

Verbose: rate limit in-game output

Artuto opened this issue · 7 comments

commented

Hi, I had this issue where some plugins literally spam permission checks (for example, ChatControl with their Packet Rules Bypass check).

I turned on LP's verbose to debug a permission missing for a player and my client ended crashing because LP was just sending too much info.

I'd request a way to limit the verbosity of this command. Maybe an internal ratelimiter, for example, if a permission check is repeated 4 times in the last 1 second and is the same permission only show 1 message, not 4.

Another way around would be a boolean in the config file.

commented

I've used utilities where identical messages (two or more messages that are the same) are blocked, sometimes with a count like "(x count)", in the log. Since that counter wouldn't work like that here, maybe... save the last message, and for every duplicate, increment an internal counter and then stop; when a non duplicate message comes through, send a message about the number of times the last one was repeated (Last message repeated N times for instance) and then continue (send the new one, save it as the "last sent", let the whole thing repeat). It'd probably be pretty easily implemented, although I haven't looked into the LP code - wherever the messages are sent, switch to a helper method that does this, and compare to the last with String.equals(...) or String.equalsIgnoreCase(...) to dupe-check.

commented

You can always filter out a certain permission when you enable verbose mode e.g. /lp verbose on <username> & !chatcontrol.bypass

Though I feel like it isn't a bad idea to limit the rate at which some permissions are sent to the user. On Sponge it is pretty crazy with all the internal GriefPrevention checks (again this is easily resolved by filtering them out).

commented

A rate limit won’t hurt. If you’re getting 1000+ messages a second, you can’t tell any difference anyways.

commented

I've spent a while considering what I think is best here. Just going to dump my thoughts:

  1. The /lp verbose command is meant to be verbose - it's kinda in the name. Admittedly this makes reading the output of the command in-game without any filters a bit tricky, but that's why the filters are there. You can filter the output down to a specific player + specific plugin with very little effort, and it then becomes very readable in-game.

  2. As for it's ability to "potentially crash clients" - yeah that's not too ideal, but I'm not convinced rate limiting the output is the best solution to that. Perhaps changing the command structure to favour use of the /lp verbose record functionality would work - the verbose web viewer obviously doesn't have the same crashing issue, it can handle thousands of entries easily.

  3. I like the Last message repeated N times idea, but I don't think it will work in practice. Plugins which are repeatedly checking permissions for all players on the server are likely to be doing so in a big loop, so subsequent checks are going to be targeting a different player anyway. It also wouldn't help in the Sponge GriefPrevention scenario, GPs checks are pretty much always for different permissions each time.

commented

I agree with Luck here, it doesn't really make sense to add a rate limit when LP provides 2 solutions to this problem.

  1. Use a filter
  2. Use /lp verbose record as this will not send any data to the client other than the URL to view data after paste.

Also, a rate limit would only apply to Spigot/Paper servers. For Sponge servers, contexts may be used so even though you may see duplicate permissions, it doesn't mean it is the same lookup.

commented

What if there was an option to disable the stack trace data that is displayed on hover? Or if you could enable that with the command e.g. /lp verbose debug <filter> or something and have stack trace disabled by default (most users likely won't care about this anyway).

It's likely this extra data is causing the crashing issue.

commented

Added to the above issue for consideration. ^