Fix NPE when banning/unbanning person that isn't known #652

svaningelgem · 2025-03-12T19:14:36Z

So when the player isn't known, you can ban them, but when pardoning them, it will give a NPE.
This fix resolves that nasty error and shows a nicer error message.

Closes #650

Before:

[16:52:43] [Server thread/ERROR]: Could not pass event RevokePunishmentEvent to AdvancedBan v2.3.0
java.lang.NullPointerException: Cannot invoke "com.mojang.authlib.GameProfile.getId()" because "gameProfile" is null
 at net.minecraft.server.players.UserBanList.getKeyForUser(UserBanList.java:29) ~[paper-1.21.jar:1.21-106-3a47518]
 at net.minecraft.server.players.UserBanList.getKeyForUser(UserBanList.java:10) ~[paper-1.21.jar:1.21-106-3a47518]
 at net.minecraft.server.players.StoredUserList.remove(StoredUserList.java:64) ~[paper-1.21.jar:1.21-106-3a47518]
 at org.bukkit.craftbukkit.ban.CraftProfileBanList.pardon(CraftProfileBanList.java:193) ~[paper-1.21.jar:1.21-106-3a47518]
 at org.bukkit.craftbukkit.ban.CraftProfileBanList.pardon(CraftProfileBanList.java:162) ~[paper-1.21.jar:1.21-106-3a47518]
 at AdvancedBan-Bundle-2.3.0-RELEASE.jar/me.leoko.advancedban.bukkit.listener.InternalListener.onRevokePunishment(InternalListener.java:36) ~[AdvancedBan-Bundle-2.3.0-RELEASE.jar:?]
 at com.destroystokyo.paper.event.executor.asm.generated.GeneratedEventExecutor123.execute(Unknown Source) ~[?:?]
 at org.bukkit.plugin.EventExecutor$2.execute(EventExecutor.java:77) ~[paper-api-1.21-R0.1-SNAPSHOT.jar:?]
 at co.aikar.timings.TimedEventExecutor.execute(TimedEventExecutor.java:80) ~[paper-api-1.21-R0.1-SNAPSHOT.jar:1.21-106-3a47518]
 at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[paper-api-1.21-R0.1-SNAPSHOT.jar:?]
 at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:54) ~[paper-1.21.jar:1.21-106-3a47518]
 at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.callEvent(PaperPluginManagerImpl.java:131) ~[paper-1.21.jar:1.21-106-3a47518]
 at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:628) ~[paper-api-1.21-R0.1-SNAPSHOT.jar:?]
 at AdvancedBan-Bundle-2.3.0-RELEASE.jar/me.leoko.advancedban.bukkit.BukkitMethods.lambda$callRevokePunishmentEvent$8(BukkitMethods.java:375) ~[AdvancedBan-Bun...

After:

No player is known by the name '...'

…ft universe

Hopefuls · 2025-03-12T22:51:36Z

This doesn't solve the original issue, refer to my previous comment:

Are you able to reproduce this under spigot?

As this may cause issues on other sections using said getId if it somehow manages to pass a previous check to ensure the name is valid.

if this is reproducible under spigot, then I'm glad to look further into it. If this isn't, then this is likely an issue towards paper.

Hopefuls

Please describe the following provided in the above comment:

How does this resolve the original issue? The original issue refers to something outside of AdvancedBan not correctly handling parsing a username. Can this be reproduced under spigot?

svaningelgem · 2025-03-28T11:37:44Z

Hi @Hopefuls : this does not fix the original issue of course. Because that needs to be done in paper then.

However, I left it here (and didn't respond) because I consider it a bad habbit to just let exceptions bubble up. Especially if it's as clear as this one here.
Instead, I strongly suggest to the devs I'm working with to output meaningful errors to their users.

As in this case: the library should protect against the bubbling exception and present a clean alternative message to the end-user.
No matter if spigot has it or not... (spigot has 15% market share, paper 64%)

And actually paper is correct. It's an exception as the username is incorrect. So they should throw it. But the plugins should catch the exception.
To be honest, they should throw a "UsernameIncorrectError" or so, throwing a NPE is dirty.

I will have a look tonight what happens when I run it on a spigot server, but I'm assuming it will be similar. But I keep an open mind, so let's see later.

svaningelgem · 2025-04-02T06:09:12Z

I ran this against spigot, and same thing happened:

[08:07:58] [Server thread/ERROR]: Could not pass event RevokePunishmentEvent to AdvancedBan v2.3.0
org.bukkit.event.EventException: null
        at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:310) ~[spigot-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[spigot-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:601) ~[spigot-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:588) ~[spigot-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at me.leoko.advancedban.bukkit.BukkitMethods.lambda$callRevokePunishmentEvent$8(BukkitMethods.java:375) ~[?:?]
        at org.bukkit.craftbukkit.v1_21_R1.scheduler.CraftTask.run(CraftTask.java:82) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at org.bukkit.craftbukkit.v1_21_R1.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:415) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.MinecraftServer.c(MinecraftServer.java:1425) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.dedicated.DedicatedServer.c(DedicatedServer.java:406) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.MinecraftServer.a(MinecraftServer.java:1321) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.MinecraftServer.y(MinecraftServer.java:1071) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:318) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
Caused by: java.lang.NullPointerException: Cannot invoke "com.mojang.authlib.GameProfile.getId()" because "var0" is null
        at net.minecraft.server.players.GameProfileBanList.b(SourceFile:30) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.players.GameProfileBanList.a(SourceFile:9) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at net.minecraft.server.players.JsonList.c(JsonList.java:61) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at org.bukkit.craftbukkit.v1_21_R1.ban.CraftProfileBanList.pardon(CraftProfileBanList.java:145) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at org.bukkit.craftbukkit.v1_21_R1.ban.CraftProfileBanList.pardon(CraftProfileBanList.java:114) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4344-Spigot-a759b62-19bf846]
        at me.leoko.advancedban.bukkit.listener.InternalListener.onRevokePunishment(InternalListener.java:36) ~[?:?]
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) ~[?:?]
        at java.base/java.lang.reflect.Method.invoke(Method.java:580) ~[?:?]
        at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        ... 12 more

Hopefuls · 2025-04-12T17:05:15Z

Was able to reproduce with the same username, this can be reproduced the following way:

Ban someone using a non-supported character in a username (such as ^ or !)
Attempt to unban the same player

The error originates from:

banlist.pardon(e.getPunishment().getName());

it is not that this is an easy fix to just try catch, it is more worrying that this is not properly handled in the server software itself. This is a bug from bukkit (Unless modified by purpur in this case).

    at net.minecraft.server.players.UserBanList.getKeyForUser(UserBanList.java:29) ~[purpur-1.21.1.jar:1.21.1-2329-803bf62]

    at net.minecraft.server.players.UserBanList.getKeyForUser(UserBanList.java:10) ~[purpur-1.21.1.jar:1.21.1-2329-803bf62]
    at net.minecraft.server.players.StoredUserList.remove(StoredUserList.java:64) ~[purpur-1.21.1.jar:1.21.1-2329-803bf62]
    at org.bukkit.craftbukkit.ban.CraftProfileBanList.pardon(CraftProfileBanList.java:193) ~[purpur-1.21.1.jar:1.21.1-2329-803bf62]
    at org.bukkit.craftbukkit.ban.CraftProfileBanList.pardon(CraftProfileBanList.java:162) ~[purpur-1.21.1.jar:1.21.1-2329-803bf62]

Hopefuls · 2025-04-12T17:07:42Z

bukkit/src/main/java/me/leoko/advancedban/bukkit/listener/InternalListener.java

+    private void ban(BanList banlist, PunishmentEvent e) {
+        try {
+            banlist.addBan(e.getPunishment().getName(), e.getPunishment().getReason(), new Date(e.getPunishment().getEnd()), e.getPunishment().getOperator());
+        } catch (NullPointerException ex) {
+            Bukkit.getLogger().severe("No player is known by the name '" + e.getPunishment().getName() + "'");
+        }
+    }


Can this be further specified to only catch when the error originates from:

Cannot invoke "com.mojang.authlib.GameProfile.getId()" because "var0" is null
As this may catch-all issues that are not related to this issue.

(Also apply to the pardon too)

Thanks for your help!

I can give you access to the PR if you want as I'm not able to fix this within the next week.

I think I now fixed it better. The problem is that I don't want to check for a certain text in the error message. Neither would I like to go parsing the stack trace. Nor do something with reflection.

The negative thing with the current implementation is that a user has to have been logged on before he's allowed to be banned...

Neither option truly satisfies me, even more because there is no real internal method that I can query (without the use of reflection / fork-specific calls) to check the existence of the username.

Fix NPE when banning/unbanning person that isn't known in the minecra…

b9bddaa

…ft universe

Hopefuls self-assigned this Mar 27, 2025

Hopefuls self-requested a review March 27, 2025 21:10

Hopefuls requested changes Mar 27, 2025

View reviewed changes

Hopefuls added the Changes requested label Mar 27, 2025

Hopefuls requested changes Apr 12, 2025

View reviewed changes

First checking if a user by that name exists.

e58d0bc

svaningelgem requested a review from Hopefuls April 30, 2025 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix NPE when banning/unbanning person that isn't known #652

Fix NPE when banning/unbanning person that isn't known #652

svaningelgem commented Mar 12, 2025 •

edited

Loading

Hopefuls commented Mar 12, 2025 •

edited

Loading

Hopefuls left a comment

svaningelgem commented Mar 28, 2025

svaningelgem commented Apr 2, 2025

Hopefuls commented Apr 12, 2025

Hopefuls Apr 12, 2025 •

edited

Loading

svaningelgem Apr 12, 2025

svaningelgem Apr 30, 2025

Fix NPE when banning/unbanning person that isn't known #652

Are you sure you want to change the base?

Fix NPE when banning/unbanning person that isn't known #652

Conversation

svaningelgem commented Mar 12, 2025 • edited Loading

Hopefuls commented Mar 12, 2025 • edited Loading

Hopefuls left a comment

Choose a reason for hiding this comment

svaningelgem commented Mar 28, 2025

svaningelgem commented Apr 2, 2025

Hopefuls commented Apr 12, 2025

Hopefuls Apr 12, 2025 • edited Loading

Choose a reason for hiding this comment

svaningelgem Apr 12, 2025

Choose a reason for hiding this comment

svaningelgem Apr 30, 2025

Choose a reason for hiding this comment

svaningelgem commented Mar 12, 2025 •

edited

Loading

Hopefuls commented Mar 12, 2025 •

edited

Loading

Hopefuls Apr 12, 2025 •

edited

Loading