From 2cfc37bb895ff7e4c6e9d2e98bb6b3de206687d1 Mon Sep 17 00:00:00 2001 From: twisti Date: Tue, 17 Mar 2026 16:28:54 +0100 Subject: [PATCH 1/2] feat: enhance packet listener interfaces with early packet handling and improved error management --- .../api/surf-api-bukkit-api.api | 1 + .../NmsClientboundPacketListener.java | 22 +- .../api/nms/listener/NmsPacketListener.java | 16 +- .../NmsServerboundPacketListener.java | 8 +- .../impl/nms/SurfBukkitNmsBridgeImpl.kt | 22 +- .../SurfBukkitPacketListenerApiImpl.kt | 195 +++++++++----- .../bukkit/server/packet/PacketApiLoader.kt | 1 + .../packet/listener/PlayerChannelInjector.kt | 239 +++++++----------- 8 files changed, 282 insertions(+), 222 deletions(-) diff --git a/surf-api-bukkit/surf-api-bukkit-api/api/surf-api-bukkit-api.api b/surf-api-bukkit/surf-api-bukkit-api/api/surf-api-bukkit-api.api index a48e023a9..a7c157bfd 100644 --- a/surf-api-bukkit/surf-api-bukkit-api/api/surf-api-bukkit-api.api +++ b/surf-api-bukkit/surf-api-bukkit-api/api/surf-api-bukkit-api.api @@ -2072,6 +2072,7 @@ public final class dev/slne/surf/surfapi/bukkit/api/nms/bridges/packets/player/t public abstract interface class dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsClientboundPacketListener : dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsPacketListener { public abstract fun handleClientboundPacket (Ldev/slne/surf/surfapi/bukkit/api/nms/listener/packets/NmsPacket;Lorg/bukkit/entity/Player;)Ldev/slne/surf/surfapi/bukkit/api/packet/listener/listener/PacketListenerResult; + public fun handleEarlyClientboundPacket (Ldev/slne/surf/surfapi/bukkit/api/nms/listener/packets/NmsPacket;Lorg/bukkit/entity/Player;)Ldev/slne/surf/surfapi/bukkit/api/packet/listener/listener/PacketListenerResult; } public abstract interface class dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsPacketListener { diff --git a/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsClientboundPacketListener.java b/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsClientboundPacketListener.java index 2f24a478e..5d551e16c 100644 --- a/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsClientboundPacketListener.java +++ b/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsClientboundPacketListener.java @@ -4,10 +4,28 @@ import dev.slne.surf.surfapi.bukkit.api.nms.listener.packets.NmsPacket; import dev.slne.surf.surfapi.bukkit.api.packet.listener.listener.PacketListenerResult; import org.bukkit.entity.Player; +import org.jetbrains.annotations.ApiStatus; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; @NmsUseWithCaution +@NullMarked public interface NmsClientboundPacketListener extends - NmsPacketListener { + NmsPacketListener { - PacketListenerResult handleClientboundPacket(Packet packet, Player player); + @ApiStatus.OverrideOnly + PacketListenerResult handleClientboundPacket(Packet packet, Player player); + + @ApiStatus.OverrideOnly + default PacketListenerResult handleEarlyClientboundPacket(Packet packet, @Nullable Player player) { + if (player != null) { + return handleClientboundPacket(packet, player); + } else { + throw new IllegalStateException( + "No player is available for this clientbound packet yet. " + + "This can happen during early connection phases such as login. " + + "Override handleEarlyClientboundPacket(...) if your listener should handle packets before a Player exists." + ); + } + } } diff --git a/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsPacketListener.java b/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsPacketListener.java index 2468fbf51..e392f24ad 100644 --- a/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsPacketListener.java +++ b/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsPacketListener.java @@ -3,14 +3,20 @@ import com.google.common.reflect.TypeToken; import dev.slne.surf.surfapi.bukkit.api.nms.NmsUseWithCaution; import dev.slne.surf.surfapi.bukkit.api.nms.listener.packets.NmsPacket; +import dev.slne.surf.surfapi.shared.api.util.InternalSurfApi; +import org.jetbrains.annotations.ApiStatus; +import org.jspecify.annotations.NullMarked; @NmsUseWithCaution +@NullMarked public interface NmsPacketListener { - default Class getPacketClass() { - TypeToken typeToken = new TypeToken(getClass()) { - }; + @ApiStatus.Internal + @InternalSurfApi + default Class getPacketClass() { + TypeToken typeToken = new TypeToken(getClass()) { + }; - return typeToken.getRawType(); - } + return typeToken.getRawType(); + } } diff --git a/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsServerboundPacketListener.java b/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsServerboundPacketListener.java index cf1040c50..b4933286c 100644 --- a/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsServerboundPacketListener.java +++ b/surf-api-bukkit/surf-api-bukkit-api/src/main/java/dev/slne/surf/surfapi/bukkit/api/nms/listener/NmsServerboundPacketListener.java @@ -5,9 +5,11 @@ import dev.slne.surf.surfapi.bukkit.api.packet.listener.listener.PacketListenerResult; import org.bukkit.entity.Player; import org.jetbrains.annotations.ApiStatus.OverrideOnly; +import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @NmsUseWithCaution +@NullMarked public interface NmsServerboundPacketListener extends NmsPacketListener { @@ -19,7 +21,11 @@ default PacketListenerResult handleEarlyServerboundPacket(Packet packet, @Nullab if (player != null) { return handleServerboundPacket(packet, player); } else { - return PacketListenerResult.CONTINUE; + throw new IllegalStateException( + "No player is available for this serverbound packet yet. " + + "This can happen during early connection phases such as login. " + + "Override handleEarlyServerboundPacket(...) if your listener should handle packets before a Player exists." + ); } } } diff --git a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/nms/SurfBukkitNmsBridgeImpl.kt b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/nms/SurfBukkitNmsBridgeImpl.kt index a7459a64c..18dfdeb7a 100644 --- a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/nms/SurfBukkitNmsBridgeImpl.kt +++ b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/nms/SurfBukkitNmsBridgeImpl.kt @@ -84,7 +84,15 @@ class SurfBukkitNmsBridgeImpl : SurfBukkitNmsBridge { var cancel = false for (listener in listener) { listener as NmsServerboundPacketListener - val result = listener.handleEarlyServerboundPacket(packet, player) + val result = try { + listener.handleEarlyServerboundPacket(packet, player) + } catch (e: Throwable) { + log.atSevere() + .withCause(e) + .log("Failed to handle serverbound packet") + PacketListenerResult.CONTINUE + } + if (result == PacketListenerResult.CANCEL) { cancel = true } @@ -96,7 +104,7 @@ class SurfBukkitNmsBridgeImpl : SurfBukkitNmsBridge { @Suppress("UNCHECKED_CAST") fun handleClientboundPacket( packet: Packet, - player: Player, + player: Player?, ): Packet? { val listeners = clientboundPacketListeners[packet.packetClass] ?: return packet @@ -105,7 +113,15 @@ class SurfBukkitNmsBridgeImpl : SurfBukkitNmsBridge { var cancel = false for (listener in listeners) { listener as NmsClientboundPacketListener - val result = listener.handleClientboundPacket(packet, player) + val result = try { + listener.handleEarlyClientboundPacket(packet, player) + } catch (e: Throwable) { + log.atSevere() + .withCause(e) + .log("Failed to handle clientbound packet") + PacketListenerResult.CONTINUE + } + if (result == PacketListenerResult.CANCEL) { cancel = true } diff --git a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/packet/listener/SurfBukkitPacketListenerApiImpl.kt b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/packet/listener/SurfBukkitPacketListenerApiImpl.kt index 8f845598b..3160966da 100644 --- a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/packet/listener/SurfBukkitPacketListenerApiImpl.kt +++ b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/impl/packet/listener/SurfBukkitPacketListenerApiImpl.kt @@ -1,49 +1,68 @@ package dev.slne.surf.surfapi.bukkit.server.impl.packet.listener import com.google.auto.service.AutoService -import com.google.common.flogger.StackSize import dev.slne.surf.surfapi.bukkit.api.nms.NmsUseWithCaution import dev.slne.surf.surfapi.bukkit.api.packet.listener.SurfBukkitPacketListenerApi import dev.slne.surf.surfapi.bukkit.api.packet.listener.listener.PacketListener import dev.slne.surf.surfapi.bukkit.api.packet.listener.listener.PacketListenerResult import dev.slne.surf.surfapi.bukkit.api.packet.listener.listener.annotation.ClientboundListener import dev.slne.surf.surfapi.bukkit.api.packet.listener.listener.annotation.ServerboundListener -import dev.slne.surf.surfapi.core.api.util.* -import it.unimi.dsi.fastutil.objects.Object2ObjectMap -import it.unimi.dsi.fastutil.objects.ObjectSet +import dev.slne.surf.surfapi.core.api.util.checkInstantiationByServiceLoader +import dev.slne.surf.surfapi.core.api.util.logger import net.minecraft.network.protocol.Packet import net.minecraft.server.level.ServerPlayer +import org.bukkit.entity.Player import java.lang.invoke.MethodHandle import java.lang.invoke.MethodHandles +import java.lang.invoke.MethodType import java.lang.reflect.Method +import java.lang.reflect.Modifier +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ConcurrentMap +import java.util.concurrent.CopyOnWriteArraySet @NmsUseWithCaution @AutoService(SurfBukkitPacketListenerApi::class) class SurfBukkitPacketListenerApiImpl : SurfBukkitPacketListenerApi { private val log = logger() - private val clientboundListenerMethods = - mutableObject2ObjectMapOf, ObjectSet>().synchronize() - private val serverboundListenerMethods = - mutableObject2ObjectMapOf, ObjectSet>().synchronize() + private val clientboundListenerMethods = ConcurrentHashMap, CopyOnWriteArraySet>() + private val serverboundListenerMethods = ConcurrentHashMap, CopyOnWriteArraySet>() private val lookup = MethodHandles.lookup() + private val normalizedListenerType = MethodType.methodType( + Any::class.java, + Packet::class.java, + ServerPlayer::class.java + ) + + private val toBukkitPlayerHandle: MethodHandle = lookup.findStatic( + SurfBukkitPacketListenerApiImpl::class.java, + "toBukkitPlayer", + MethodType.methodType(Player::class.java, ServerPlayer::class.java) + ) + + companion object { + @Suppress("unused") + @JvmStatic + fun toBukkitPlayer(serverPlayer: ServerPlayer?): Player? = serverPlayer?.bukkitEntity + } + init { checkInstantiationByServiceLoader() } - override fun registerListeners(listener: PacketListener) { - for (method in listener.javaClass.getMethods()) { + for (method in listener.javaClass.declaredMethods) { try { if (method.isAnnotationPresent(ClientboundListener::class.java)) { registerListenerMethod(listener, method, clientboundListenerMethods) } else if (method.isAnnotationPresent(ServerboundListener::class.java)) { registerListenerMethod(listener, method, serverboundListenerMethods) } - } catch (_: IllegalAccessException) { + } catch (e: IllegalAccessException) { log.atSevere() - .withStackTrace(StackSize.MEDIUM) + .withCause(e) .log("Failed to register listener method '${method.declaringClass.name}#${method.name}' due to illegal access") } } @@ -52,26 +71,94 @@ class SurfBukkitPacketListenerApiImpl : SurfBukkitPacketListenerApi { private fun registerListenerMethod( listener: PacketListener, method: Method, - clientboundListenerMethods: Object2ObjectMap, ObjectSet> + listenerMethods: ConcurrentMap, CopyOnWriteArraySet> ) { - val methodHandle = lookup.unreflect(method) - val hasPlayerParameter = method.parameterCount == 2 + require(!Modifier.isStatic(method.modifiers)) { "Listener method must not be static: ${method.declaringClass.name}#${method.name}" } + require(method.parameterCount in 1..2) { "Listener method must have 1 or 2 parameters (Packet and optional ServerPlayer/Player): ${method.declaringClass.name}#${method.name}" } + + val packetParameterType = method.parameterTypes[0] + require(Packet::class.java.isAssignableFrom(packetParameterType)) { "Packet parameter must be a subclass of Packet: ${method.declaringClass.name}#${method.name}" } + + val playerParameterType = method.parameterTypes.getOrNull(1) val hasServerPlayerParameter = - hasPlayerParameter && method.parameterTypes[1].isAssignableFrom(ServerPlayer::class.java) - clientboundListenerMethods - .computeIfAbsent( - method.parameterTypes[0] - ) { mutableObjectSetOf() } - .add( - ListenerMethod( - listener, - methodHandle, - hasPlayerParameter, - hasServerPlayerParameter - ) - ) + playerParameterType != null && ServerPlayer::class.java.isAssignableFrom(playerParameterType) + + if (playerParameterType != null) { + val isBukkitPlayer = Player::class.java.isAssignableFrom(playerParameterType) + require(isBukkitPlayer || hasServerPlayerParameter) { "Second parameter must be either ServerPlayer or a Bukkit Player: ${method.declaringClass.name}#${method.name}" } + } + + val privateLookupIn = try { + MethodHandles.privateLookupIn(method.declaringClass, lookup) + } catch (_: IllegalAccessException) { + method.trySetAccessible() + lookup + } + + val methodHandle = createNormalizedInvokerHandle(listener, method, privateLookupIn) + val listeners = listenerMethods.computeIfAbsent(packetParameterType) { CopyOnWriteArraySet() } + val invoker = createInvoker(methodHandle, method.returnType) + + listeners.add(ListenerMethod(listener, invoker)) } + private fun createNormalizedInvokerHandle( + listener: PacketListener, + method: Method, + lookup: MethodHandles.Lookup + ): MethodHandle { + val bound = lookup.unreflect(method).bindTo(listener) + val parameterTypes = method.parameterTypes + + val adapted = when (parameterTypes.size) { + 1 -> { + MethodHandles.dropArguments(bound, 1, ServerPlayer::class.java) + } + + 2 -> { + val second = parameterTypes[1] + + when { + ServerPlayer::class.java.isAssignableFrom(second) -> bound + Player::class.java.isAssignableFrom(second) -> + MethodHandles.filterArguments(bound, 1, toBukkitPlayerHandle) + + else -> error("Unsupported second parameter: $second") + } + } + + else -> error("Unsupported parameter count: ${parameterTypes.size}") + } + + return adapted.asType(normalizedListenerType) + } + + @Suppress("UNCHECKED_CAST") + private fun createInvoker( + mh: MethodHandle, + returnType: Class<*> + ): ListenerInvoker { + val resultConverter = createResultConverter(returnType) as ListenerResultConverter + return { packet, player -> + @Suppress("USELESS_CAST") + val result: Any? = mh.invokeExact(packet as Packet<*>, player as ServerPlayer?) + resultConverter.convert(result, packet) + } + } + + private fun createResultConverter(returnType: Class<*>): ListenerResultConverter<*> = when { + returnType == Void.TYPE -> ListenerResultConverter { _, p -> p } + returnType == PacketListenerResult::class.java -> ListenerResultConverter { result, packet -> + if (result == PacketListenerResult.CANCEL) null else packet + } + + Packet::class.java.isAssignableFrom(returnType) -> ListenerResultConverter?> { result, original -> + result + } + else -> error("Unsupported return type for packet listener method: $returnType! Only void, PacketListenerResult, or a subclass of Packet is supported.") + } + + override fun unregisterListeners(listener: PacketListener) { for (methods in clientboundListenerMethods.values) { methods.removeIf { it.listener == listener } @@ -83,18 +170,15 @@ class SurfBukkitPacketListenerApiImpl : SurfBukkitPacketListenerApi { fun handleClientboundPacket( packet: Packet<*>, - serverPlayer: ServerPlayer + serverPlayer: ServerPlayer? ): Packet<*>? { val methods = clientboundListenerMethods[packet.javaClass] ?: return packet var result: Packet<*>? = packet try { for (listenerMethod in methods) { - result = callListener(listenerMethod, serverPlayer, packet) - - if (result == null) { - break - } + result = listenerMethod.invoker.invoke(result ?: break, serverPlayer) + if (result == null) break } } catch (t: Throwable) { log.atSevere() @@ -114,11 +198,8 @@ class SurfBukkitPacketListenerApiImpl : SurfBukkitPacketListenerApi { try { for (listenerMethod in methods) { - result = callListener(listenerMethod, serverPlayer, packet) - - if (result == null) { - break - } + result = listenerMethod.invoker.invoke(result ?: break, serverPlayer) + if (result == null) break } } catch (t: Throwable) { log.atSevere() @@ -129,40 +210,16 @@ class SurfBukkitPacketListenerApiImpl : SurfBukkitPacketListenerApi { return result } - private fun callListener( - listenerMethod: ListenerMethod, - serverPlayer: ServerPlayer?, - packet: Packet<*> - ): Packet<*>? { - if (listenerMethod.hasPlayerParameter) { - val player = - if (listenerMethod.hasServerPlayerParameter) serverPlayer else serverPlayer?.bukkitEntity - val result = listenerMethod.methodHandle(listenerMethod.listener, packet, player) - if (result is PacketListenerResult && result == PacketListenerResult.CANCEL) { - return null - } else if (result is Packet<*>) { - return result - } - } else { - val result = listenerMethod.methodHandle(listenerMethod.listener, packet) - if (result is PacketListenerResult && result == PacketListenerResult.CANCEL) { - return null - } else if (result is Packet<*>) { - return result - } - } - - return packet + fun interface ListenerResultConverter { + fun convert(result: T, packet: Packet<*>?): Packet<*>? } - private fun reduceResults(results: ObjectSet) = - if (PacketListenerResult.CANCEL in results) PacketListenerResult.CANCEL else PacketListenerResult.CONTINUE - + fun interface ListenerInvoker { + fun invoke(packet: Packet<*>, serverPlayer: ServerPlayer?): Packet<*>? + } private data class ListenerMethod( val listener: PacketListener, - val methodHandle: MethodHandle, - val hasPlayerParameter: Boolean, - val hasServerPlayerParameter: Boolean, + val invoker: ListenerInvoker, ) } diff --git a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/PacketApiLoader.kt b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/PacketApiLoader.kt index 171a4e472..4e790eefe 100644 --- a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/PacketApiLoader.kt +++ b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/PacketApiLoader.kt @@ -34,6 +34,7 @@ object PacketApiLoader { packetEvents.terminate() packetListenerApi.unregisterListeners(PacketLoreListener) PluginDisablePacketLoreListener.unregister() + PlayerChannelInjector.unregister() } private fun setupPacketEvents() { diff --git a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/listener/PlayerChannelInjector.kt b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/listener/PlayerChannelInjector.kt index 061184628..4d932d5d8 100644 --- a/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/listener/PlayerChannelInjector.kt +++ b/surf-api-bukkit/surf-api-bukkit-server/src/main/kotlin/dev/slne/surf/surfapi/bukkit/server/packet/listener/PlayerChannelInjector.kt @@ -1,15 +1,17 @@ package dev.slne.surf.surfapi.bukkit.server.packet.listener -import com.github.benmanes.caffeine.cache.Caffeine -import com.sksamuel.aedile.core.expireAfterAccess import dev.slne.surf.surfapi.bukkit.api.nms.NmsUseWithCaution -import dev.slne.surf.surfapi.bukkit.api.nms.nmsBridge +import dev.slne.surf.surfapi.bukkit.api.nms.SurfBukkitNmsBridge +import dev.slne.surf.surfapi.bukkit.api.packet.listener.SurfBukkitPacketListenerApi import dev.slne.surf.surfapi.bukkit.server.impl.nms.SurfBukkitNmsBridgeImpl import dev.slne.surf.surfapi.bukkit.server.impl.nms.listener.packets.NmsPacketImpl import dev.slne.surf.surfapi.bukkit.server.impl.nms.listener.packets.PacketRegistry import dev.slne.surf.surfapi.bukkit.server.impl.packet.listener.SurfBukkitPacketListenerApiImpl import dev.slne.surf.surfapi.bukkit.server.nms.toNms import dev.slne.surf.surfapi.bukkit.server.plugin +import dev.slne.surf.surfapi.core.api.messages.Colors +import dev.slne.surf.surfapi.core.api.messages.CommonComponents +import dev.slne.surf.surfapi.core.api.messages.adventure.text import dev.slne.surf.surfapi.core.api.reflection.Field import dev.slne.surf.surfapi.core.api.reflection.SurfProxy import dev.slne.surf.surfapi.core.api.reflection.createProxy @@ -19,6 +21,7 @@ import io.netty.channel.Channel import io.netty.channel.ChannelDuplexHandler import io.netty.channel.ChannelHandlerContext import io.netty.channel.ChannelPromise +import io.netty.util.AttributeKey import io.papermc.paper.connection.PaperPlayerLoginConnection import io.papermc.paper.connection.ReadablePlayerCookieConnectionImpl import io.papermc.paper.event.connection.PlayerConnectionValidateLoginEvent @@ -27,71 +30,84 @@ import net.kyori.adventure.key.Key import net.minecraft.network.Connection import net.minecraft.network.HandlerNames import net.minecraft.network.protocol.Packet -import net.minecraft.network.protocol.login.ClientboundLoginFinishedPacket +import net.minecraft.server.level.ServerPlayer import org.bukkit.event.EventHandler import org.bukkit.event.EventPriority import org.bukkit.event.Listener import org.bukkit.event.player.PlayerJoinEvent -import java.util.* -import kotlin.time.Duration.Companion.minutes import dev.slne.surf.surfapi.bukkit.api.event.register as registerListener import dev.slne.surf.surfapi.bukkit.api.event.unregister as unregisterListener +@Suppress("UnstableApiUsage") object PlayerChannelInjector : Listener { private val log = logger() private val CHANNEL_KEY = Key.key("surf-api", "packet-listener") private const val CHANNEL_NAME = "surf_api_packet_listener" - private val playerInjectorCache = Caffeine.newBuilder() - .weakValues() - .expireAfterAccess(1.minutes) - .build() - - private val injectedChannels = Caffeine.newBuilder() - .weakKeys() - .build() + private val packetHandlerKey = AttributeKey.newInstance("surf_api_packet_handler") fun register() { - ChannelInitializeListenerHolder.addListener(CHANNEL_KEY) { this.injectChannel(it) } + ChannelInitializeListenerHolder.addListener(CHANNEL_KEY) { this.getOrInjectPacketHandler(it) } registerListener(plugin) } fun unregister() { - ChannelInitializeListenerHolder.removeListener(CHANNEL_KEY) unregisterListener() + ChannelInitializeListenerHolder.removeListener(CHANNEL_KEY) } - private fun injectChannel(channel: Channel): PacketHandler { - val channelHandler = PacketHandler() - - channel.eventLoop().submit { - if (injectedChannels.asMap().putIfAbsent(channel, true) == null) { - val pipeline = channel.pipeline() + private fun getOrInjectPacketHandler(channel: Channel): PacketHandler { + val handler = PacketHandler() + val attr = channel.attr(packetHandlerKey) - if (pipeline.get(CHANNEL_NAME) != null) { - return@submit - } + if (!attr.compareAndSet(null, handler)) { + return attr.get() + } - pipeline.addBefore(HandlerNames.PACKET_HANDLER, CHANNEL_NAME, channelHandler) + val command = Runnable { + if (channel.pipeline().get(CHANNEL_NAME) == null) { + channel.pipeline().addBefore(HandlerNames.PACKET_HANDLER, CHANNEL_NAME, handler) } } - return channelHandler + if (channel.eventLoop().inEventLoop()) { + command.run() + } else { + channel.eventLoop().execute(command) + } + + return handler } @EventHandler fun onPlayerLogin(event: PlayerConnectionValidateLoginEvent) { val paperConnection = event.connection if (paperConnection is PaperPlayerLoginConnection) { - val profile = - paperConnection.authenticatedProfile ?: error("Authenticated profile is null") - val connection = - ReadablePlayerCookieConnectionImplProxy.instance.getConnection(paperConnection) - playerInjectorCache.put( - profile.id ?: error("PlayerProfile does not provide a uuid"), - connection - ) + val profile = paperConnection.authenticatedProfile + + if (profile == null) { + event.kickMessage( + CommonComponents.renderDisconnectMessage( + "FAILED TO INJECT PACKET LISTENER", + { + text( + "Dein Profil ist nicht authentifiziert, daher können wir den Paket-Listener nicht einbinden. Dies ist wahrscheinlich ein Problem mit deiner Verbindung zu den Authentifizierungsservern von Mojang.", + Colors.ERROR + ) + }, + issue = true + ) + ) + + log.atWarning() + .log("Failed to inject packet listener for player (${paperConnection.unsafeProfile}) with unauthenticated profile: ${paperConnection.address} - ${paperConnection.clientAddress}") + return + } + + val connection = ReadablePlayerCookieConnectionImplProxy.instance.getConnection(paperConnection) + val channel = connection.channel + getOrInjectPacketHandler(channel).connection = connection } } @@ -99,132 +115,78 @@ object PlayerChannelInjector : Listener { fun onPlayerJoin(event: PlayerJoinEvent) { val player = event.player.toNms() val connection = player.connection.connection - val channelHandler = connection.channel.pipeline().get(CHANNEL_NAME) - - if (channelHandler != null) { - if (channelHandler is PacketHandler) { - channelHandler.connection = connection - playerInjectorCache.invalidate(player.uuid) - } - return - } + val channel = connection.channel - injectChannel(connection.channel).connection = connection + getOrInjectPacketHandler(channel).connection = connection } + @OptIn(NmsUseWithCaution::class) private class PacketHandler : ChannelDuplexHandler() { + private val bridge = SurfBukkitNmsBridge.instance as SurfBukkitNmsBridgeImpl + private val packetListenerApi = SurfBukkitPacketListenerApi.instance as SurfBukkitPacketListenerApiImpl + @Volatile var connection: Connection? = null - override fun channelUnregistered(ctx: ChannelHandlerContext) { - injectedChannels.invalidate(ctx.channel()) - super.channelUnregistered(ctx) - } - @OptIn(NmsUseWithCaution::class) override fun write( - ctx: ChannelHandlerContext?, - msg: Any?, - promise: ChannelPromise?, + ctx: ChannelHandlerContext, + msg: Any, + promise: ChannelPromise, ) { // server -> client - - var msg = msg - if (msg !is Packet<*>) { // not our packet - super.write(ctx, msg, promise) - return - } - - if (connection == null && msg is ClientboundLoginFinishedPacket) { - val uuid = msg.gameProfile().id - val cachedConnection = playerInjectorCache.getIfPresent(uuid) - if (cachedConnection != null) { - connection = cachedConnection - } - } - - val connection = connection - val player = connection?.player - if (connection == null || player == null) { - super.write(ctx, msg, promise) - return - } - - var cancelled = false - - try { - // first, we try to handle the packet with the nms packet listener - msg = this.packetListenerApi.handleClientboundPacket(msg, connection.player) - - if (msg == null) { - // no need to handle the packet further - cancelled = true - } else { - // then we try to handle the packet with the api packet listener - msg = handleClientboundPacketFromBridge(connection, msg) - cancelled = (msg == null) - } - } catch (error: OutOfMemoryError) { - throw error - } catch (t: Throwable) { - log.atSevere() - .withCause(t) - .log("Failed to handle clientbound packet") - super.write(ctx, msg, promise) - } - - if (!cancelled) { - super.write(ctx, msg, promise) - } + handlePacket( + originalMsg = msg, + passthrough = { super.write(ctx, it, promise) }, + nmsHandler = packetListenerApi::handleClientboundPacket, + bridgeHandler = ::handleClientboundPacketFromBridge + ) } - @Suppress("UNNECESSARY_SAFE_CALL") @OptIn(NmsUseWithCaution::class) - override fun channelRead(ctx: ChannelHandlerContext?, msg: Any?) { // client -> server - var msg = msg - if (msg !is Packet<*>) { // not our packet - super.channelRead(ctx, msg) + override fun channelRead(ctx: ChannelHandlerContext, msg: Any) { // client -> server + handlePacket( + originalMsg = msg, + passthrough = { super.channelRead(ctx, it) }, + nmsHandler = packetListenerApi::handleServerboundPacket, + bridgeHandler = ::handleServerboundPacketFromBridge + ) + } + + private inline fun handlePacket( + originalMsg: Any, + passthrough: (Any) -> Unit, + nmsHandler: (Packet<*>, ServerPlayer?) -> Packet<*>?, + bridgeHandler: (ServerPlayer?, Packet<*>) -> Packet<*>? + ) { + var msg = originalMsg + if (msg !is Packet<*>) { + passthrough(msg) return } val connection = connection if (connection == null) { - super.channelRead(ctx, msg) + passthrough(msg) return } - val player = connection?.player - var cancelled = false + val player = connection.player as ServerPlayer? try { - // first, we try to handle the packet with the nms packet listener - msg = this.packetListenerApi.handleServerboundPacket(msg, player) - - if (msg == null) { - // no need to handle the packet further - cancelled = true - } else { - // then we try to handle the packet with the api packet listener - msg = handleServerboundPacketFromBridge(connection, msg) - cancelled = (msg == null) - } - } catch (error: OutOfMemoryError) { - throw error + msg = nmsHandler(msg, player) ?: return + msg = bridgeHandler(player, msg) ?: return + passthrough(msg) + } catch (outOfMemoryError: OutOfMemoryError) { + throw outOfMemoryError } catch (t: Throwable) { - log.atSevere() - .withCause(t) - .log("Failed to handle serverbound packet") - super.channelRead(ctx, msg) - } - - if (!cancelled) { - super.channelRead(ctx, msg) + log.atSevere().withCause(t).log("Failed to handle packet") + passthrough(msg) } } @OptIn(NmsUseWithCaution::class) - @Suppress("UNNECESSARY_SAFE_CALL") fun handleServerboundPacketFromBridge( - connection: Connection, + serverPlayer: ServerPlayer?, packet: Packet<*>, ): Packet<*>? { val apiPacket = PacketRegistry.createServerboundPacketOrNull(packet) @@ -232,7 +194,7 @@ object PlayerChannelInjector : Listener { if (apiPacket != null) { // we have an api packet wrapper for this packet val resultApi = this.bridge.handleServerboundPacket( apiPacket, - connection.player?.bukkitEntity + serverPlayer?.bukkitEntity ) if (resultApi != null) { // we may have a modified packet @@ -245,9 +207,8 @@ object PlayerChannelInjector : Listener { return null // the packet was canceled } - @OptIn(NmsUseWithCaution::class) fun handleClientboundPacketFromBridge( - connection: Connection, + serverPlayer: ServerPlayer?, packet: Packet<*>, ): Packet<*>? { val apiPacket = PacketRegistry.createClientboundPacketOrNull(packet) @@ -255,7 +216,7 @@ object PlayerChannelInjector : Listener { if (apiPacket != null) { val resultApi = this.bridge.handleClientboundPacket( apiPacket, - connection.player.bukkitEntity + serverPlayer?.bukkitEntity ) if (resultApi != null) { @@ -267,12 +228,6 @@ object PlayerChannelInjector : Listener { return null } - - @OptIn(NmsUseWithCaution::class) - val bridge get() = nmsBridge as SurfBukkitNmsBridgeImpl - - @OptIn(NmsUseWithCaution::class) - val packetListenerApi get() = dev.slne.surf.surfapi.bukkit.api.packet.listener.packetListenerApi as SurfBukkitPacketListenerApiImpl } @SurfProxy(ReadablePlayerCookieConnectionImpl::class) From cddaee49968430b7647680a8840ddafa54ba369b Mon Sep 17 00:00:00 2001 From: twisti Date: Tue, 17 Mar 2026 16:29:51 +0100 Subject: [PATCH 2/2] chore: bump version to 1.21.11-2.69.0 in gradle.properties --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 140ff6cb4..4858161dd 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,6 +7,6 @@ org.jetbrains.dokka.experimental.gradle.pluginMode=V2Enabled javaVersion=25 mcVersion=1.21.11 group=dev.slne.surf -version=1.21.11-2.68.0 +version=1.21.11-2.69.0 relocationPrefix=dev.slne.surf.surfapi.libs snapshot=false