SamB440 / ForcePack

Spigot/Velocity plugin to aid resource pack deployment and enforcement, among other utilities.
https://fortitude.islandearth.net/category/forcepack
GNU General Public License v3.0
53 stars 21 forks source link

Fix a NPE when a user don't exist for packetevents #68

Open rexlManu opened 5 months ago

rexlManu commented 5 months ago

I added a check if the user is null for packetevents to fix if the user don't exist for packetevents and instead return -1.

Also I changed the packetevents declaration since they changed it with 2.3.1 and ongoing.

Generally speaking I would recommend to completely ignore users that don't exist in packetevents, since they always will be fake players. If you want I can create a pr that addresses this!

SamB440 commented 5 months ago

Where is getProtocolVersion called that causes such an error? I've already had to do so much work to fix issues with fake player plugins such as PlayerJoinEvent being called by them...

rexlManu commented 5 months ago

I'm operating a plugin that creates fake player and registers them via the player list (nms class).

Here is the full log:

14:30:37 ERROR]: Could not pass event PlayerJoinEvent to ForcePack v1.3.4 java.lang.NullPointerException: Cannot invoke "com.convallyria.forcepack.spigot.libs.pe-api.protocol.player.User.getClientVersion()" because the return value of "com.convallyria.forcepack.spigot.libs.pe-api.manager.player.PlayerManager.getUser(Object)" is null at com.convallyria.forcepack.spigot.util.ProtocolUtil.getProtocolVersion(ProtocolUtil.java:19) ~[ForcePack-1.3.4.jar:?] at com.convallyria.forcepack.spigot.ForcePackSpigot.getPacksForVersion(ForcePackSpigot.java:80) ~[ForcePack-1.3.4.jar:?] at com.convallyria.forcepack.spigot.listener.ResourcePackListener.onPlayerJoin(ResourcePackListener.java:216) ~[ForcePack-1.3.4.jar:?] at com.destroystokyo.paper.event.executor.asm.generated.GeneratedEventExecutor13.execute(Unknown Source) ~[?:?] at org.bukkit.plugin.EventExecutor$2.execute(EventExecutor.java:77) ~[paper-api-1.20.4-R0.1-SNAPSHOT.jar:?] at co.aikar.timings.TimedEventExecutor.execute(TimedEventExecutor.java:81) ~[paper-api-1.20.4-R0.1-SNAPSHOT.jar:git-Paper-497] at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[paper-api-1.20.4-R0.1-SNAPSHOT.jar:?] at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:54) ~[paper-1.20.4.jar:git-Paper-497] at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.callEvent(PaperPluginManagerImpl.java:126) ~[paper-1.20.4.jar:git-Paper-497] at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:615) ~[paper-api-1.20.4-R0.1-SNAPSHOT.jar:?] at net.minecraft.server.players.PlayerList.placeNewPlayer(PlayerList.java:345)

rexlManu commented 5 months ago

packetevents checks like this if a player is fake and doenst inject into it. https://github.com/retrooper/packetevents/blob/2.0/api%2Fsrc%2Fmain%2Fjava%2Fcom%2Fgithub%2Fretrooper%2Fpacketevents%2Futil%2FFakeChannelUtil.java#L22

SamB440 commented 5 months ago

I think this hides the problem instead of fixing it, it would be better to adjust PlayerJoinEvent to check if the user is null. I already check if the player has metadata "NPC" and return if so.