From 9a31285ad60457b3884f36f5d5b424d3b2c8b58a Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 28 Jul 2026 02:08:04 +0200 Subject: [PATCH 1/5] fix(spigot): name the drifted accessor when server-internals lookup fails ClassNames resolves every Spigot/CraftBukkit internal in one static initializer, so a single renamed accessor on a new Minecraft release takes the whole plugin down. What the operator got back could not be acted on: a bare NullPointerException(" cannot be null") on the first touch, and "NoClassDefFoundError: Could not initialize class ...ClassNames" on every touch after that. Neither named the server software or the Minecraft version, and only the first named the accessor at all. Failures now report the exact accessor (plus every class name that was tried), the server software, the server/Bukkit/Minecraft versions and the Java version, and state that this is a Minecraft mapping change rather than a Java runtime incompatibility - a misdiagnosis this class of report keeps attracting. NmsDiagnostics deliberately lives outside ClassNames so it still initializes once ClassNames is in the erroneous state; that is what lets SpigotPlatform.enable() replay the original reason as a plain top-level log line instead of requiring a walk down a cause chain that only sometimes carries it. The ClassNames diff is dominated by re-indenting the initializer body into the latching try/catch; `git diff -w` shows the actual change. This is diagnosability only. It does not change which accessors resolve, so it does not by itself fix any server that is currently failing - it makes the next report say what to fix. --- AGENTS.md | 25 + .../com/minekube/connect/SpigotPlatform.java | 22 + .../com/minekube/connect/util/ClassNames.java | 625 +++++++++--------- .../minekube/connect/util/NmsDiagnostics.java | 231 +++++++ .../connect/util/NmsDiagnosticsTest.java | 246 +++++++ 5 files changed, 854 insertions(+), 295 deletions(-) create mode 100644 spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java create mode 100644 spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java diff --git a/AGENTS.md b/AGENTS.md index 7ea2bdd46..56d1d1b64 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,31 @@ curl -I -L --fail https://github.com/minekube/connect-java/releases/download/` plus the JUnit platform launcher + on a 26 JDK against the module's test runtime classpath. ## Velocity Join Bugs diff --git a/spigot/src/main/java/com/minekube/connect/SpigotPlatform.java b/spigot/src/main/java/com/minekube/connect/SpigotPlatform.java index 5cc901f33..02eed8b01 100644 --- a/spigot/src/main/java/com/minekube/connect/SpigotPlatform.java +++ b/spigot/src/main/java/com/minekube/connect/SpigotPlatform.java @@ -32,15 +32,19 @@ import com.minekube.connect.api.inject.PlatformInjector; import com.minekube.connect.api.logger.ConnectLogger; import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; +import com.minekube.connect.util.NmsDiagnostics; import org.bukkit.Bukkit; import org.bukkit.plugin.java.JavaPlugin; public final class SpigotPlatform extends ConnectPlatform { @Inject private JavaPlugin plugin; + private final ConnectLogger logger; + public SpigotPlatform(ConnectApi api, PlatformInjector platformInjector, ConnectLogger logger, Injector injector) { super(api, platformInjector, logger, injector); + this.logger = logger; } @Inject @@ -48,15 +52,33 @@ public SpigotPlatform(ConnectApi api, PlatformInjector platformInjector, ConnectLogger logger, Injector injector, BedrockAdmissionCoordinator admissionCoordinator) { super(api, platformInjector, logger, injector, admissionCoordinator); + this.logger = logger; } @Override public boolean enable(Module... postInitializeModules) { boolean success = super.enable(postInitializeModules); if (!success) { + reportNmsInitializationFailure(); Bukkit.getPluginManager().disablePlugin(plugin); return false; } return true; } + + /** + * Re-states why Connect's server-internals lookup failed, if that is what went wrong. + * + *

{@code ClassNames} resolves those internals from a static initializer, so only the very + * first touch carries the reason directly; every later one surfaces as + * {@code NoClassDefFoundError: Could not initialize class ...ClassNames}, which names the + * class but not the accessor. Whichever of the two the enable path happened to hit, this puts + * the accessor that actually drifted back into the log as a plain top-level line. + */ + private void reportNmsInitializationFailure() { + String failure = NmsDiagnostics.initializationFailure(); + if (failure != null) { + logger.error(failure); + } + } } diff --git a/spigot/src/main/java/com/minekube/connect/util/ClassNames.java b/spigot/src/main/java/com/minekube/connect/util/ClassNames.java index 5d397db1a..6fab2542c 100644 --- a/spigot/src/main/java/com/minekube/connect/util/ClassNames.java +++ b/spigot/src/main/java/com/minekube/connect/util/ClassNames.java @@ -36,7 +36,6 @@ import static com.minekube.connect.util.ReflectionUtils.invoke; import static com.minekube.connect.util.ReflectionUtils.makeAccessible; -import com.google.common.base.Preconditions; import com.mojang.authlib.GameProfile; import io.netty.channel.ChannelHandlerContext; import java.lang.reflect.Constructor; @@ -91,330 +90,357 @@ public class ClassNames { public static final boolean IS_POST_LOGIN_HANDLER; static { - // ahhhhhhh, this class should really be reworked at this point - - String[] versionSplit = Bukkit.getServer().getClass().getPackage().getName().split("\\."); - // Paper, since 1.20.5, no longer relocates CraftBukkit classes - // and NMS classes aren't relocated for a few versions now (both Spigot & Paper) - if (versionSplit.length <= 3 && getClassSilently("net.minecraft.server.MinecraftServer") == null) { - throw new IllegalStateException( - "Was unable to find net.minecraft.server.MinecraftServer. " + - "We don't support Mojmap yet" + // Every failure below is latched into NmsDiagnostics before it propagates. Once a static + // initializer throws, the JVM answers each later touch of this class with a + // NoClassDefFoundError that names only the class, so without the latch the message that + // names the drifted accessor is reachable only from the very first touch. + try { + // ahhhhhhh, this class should really be reworked at this point + + String[] versionSplit = Bukkit.getServer().getClass().getPackage().getName().split("\\."); + // Paper, since 1.20.5, no longer relocates CraftBukkit classes + // and NMS classes aren't relocated for a few versions now (both Spigot & Paper) + if (versionSplit.length <= 3 && getClassSilently("net.minecraft.server.MinecraftServer") == null) { + throw NmsDiagnostics.missingClass( + "MinecraftServer", + "net.minecraft.server.MinecraftServer"); + } + // Makes it that we don't have to lookup both the new and the old + // 'org.bukkit.craftbukkit. + version + CraftPlayer' will be .CraftPlayer on new + // versions and .v1_8R3.CraftPlayer on older versions + String version = versionSplit.length > 3 ? versionSplit[3] + '.' : ""; + String nmsPackage = "net.minecraft.server." + version; + + // SpigotSkinApplier + Class craftPlayerClass = ReflectionUtils.getClass( + "org.bukkit.craftbukkit." + version + "entity.CraftPlayer"); + GET_PROFILE_METHOD = getMethod(craftPlayerClass, "getProfile"); + checkNotNull(GET_PROFILE_METHOD, "Get profile method"); + + // SpigotInjector + MINECRAFT_SERVER = getClassOrFallback( + "net.minecraft.server.MinecraftServer", + nmsPackage + "MinecraftServer" ); - } - // Makes it that we don't have to lookup both the new and the old - // 'org.bukkit.craftbukkit. + version + CraftPlayer' will be .CraftPlayer on new - // versions and .v1_8R3.CraftPlayer on older versions - String version = versionSplit.length > 3 ? versionSplit[3] + '.' : ""; - String nmsPackage = "net.minecraft.server." + version; - - // SpigotSkinApplier - Class craftPlayerClass = ReflectionUtils.getClass( - "org.bukkit.craftbukkit." + version + "entity.CraftPlayer"); - GET_PROFILE_METHOD = getMethod(craftPlayerClass, "getProfile"); - checkNotNull(GET_PROFILE_METHOD, "Get profile method"); - - // SpigotInjector - MINECRAFT_SERVER = getClassOrFallback( - "net.minecraft.server.MinecraftServer", - nmsPackage + "MinecraftServer" - ); - - // Paper 1.20.5+ uses Mojang mappings: ServerConnection -> ServerConnectionListener - Class serverConnectionClass = getClassSilently( - "net.minecraft.server.network.ServerConnectionListener"); - if (serverConnectionClass == null) { - serverConnectionClass = getClassSilently( - "net.minecraft.server.network.ServerConnection"); - } - if (serverConnectionClass == null) { - serverConnectionClass = getClassSilently(nmsPackage + "ServerConnection"); - } - if (serverConnectionClass == null) { - throw new IllegalStateException( - "Could not find ServerConnection/ServerConnectionListener class."); - } - SERVER_CONNECTION = serverConnectionClass; - - // WhitelistUtils - Class craftServerClass = ReflectionUtils.getClass( - "org.bukkit.craftbukkit." + version + "CraftServer"); - Class craftOfflinePlayerClass = ReflectionUtils.getCastedClass( - "org.bukkit.craftbukkit." + version + "CraftOfflinePlayer"); - - CRAFT_OFFLINE_PLAYER_CONSTRUCTOR = getConstructor( - craftOfflinePlayerClass, true, craftServerClass, GameProfile.class); - - // SpigotDataHandler - // Paper 1.20.5+ uses Mojang mappings: NetworkManager -> Connection - Class networkManager = getClassSilently("net.minecraft.network.Connection"); - if (networkManager == null) { - networkManager = getClassSilently("net.minecraft.network.NetworkManager"); - } - if (networkManager == null) { - networkManager = getClassSilently(nmsPackage + "NetworkManager"); - } - if (networkManager == null) { - throw new IllegalStateException("Could not find NetworkManager/Connection class."); - } - SOCKET_ADDRESS = getFieldOfType(networkManager, SocketAddress.class, false); - - // Paper 1.20.5+ uses Mojang mappings: PacketHandshakingInSetProtocol -> ClientIntentionPacket - Class handshakePacket = getClassSilently( - "net.minecraft.network.protocol.handshake.ClientIntentionPacket"); - if (handshakePacket == null) { - handshakePacket = getClassSilently( - "net.minecraft.network.protocol.handshake.PacketHandshakingInSetProtocol"); - } - if (handshakePacket == null) { - handshakePacket = getClassSilently(nmsPackage + "PacketHandshakingInSetProtocol"); - } - if (handshakePacket == null) { - throw new IllegalStateException("Could not find HandshakePacket class."); - } - HANDSHAKE_PACKET = handshakePacket; - - HANDSHAKE_HOST = getFieldOfType(HANDSHAKE_PACKET, String.class); - checkNotNull(HANDSHAKE_HOST, "Handshake host"); + // Paper 1.20.5+ uses Mojang mappings: ServerConnection -> ServerConnectionListener + Class serverConnectionClass = getClassSilently( + "net.minecraft.server.network.ServerConnectionListener"); + if (serverConnectionClass == null) { + serverConnectionClass = getClassSilently( + "net.minecraft.server.network.ServerConnection"); + } + if (serverConnectionClass == null) { + serverConnectionClass = getClassSilently(nmsPackage + "ServerConnection"); + } + if (serverConnectionClass == null) { + throw NmsDiagnostics.missingClass( + "ServerConnection/ServerConnectionListener", + "net.minecraft.server.network.ServerConnectionListener", + "net.minecraft.server.network.ServerConnection", + nmsPackage + "ServerConnection"); + } + SERVER_CONNECTION = serverConnectionClass; + + // WhitelistUtils + Class craftServerClass = ReflectionUtils.getClass( + "org.bukkit.craftbukkit." + version + "CraftServer"); + Class craftOfflinePlayerClass = ReflectionUtils.getCastedClass( + "org.bukkit.craftbukkit." + version + "CraftOfflinePlayer"); + + CRAFT_OFFLINE_PLAYER_CONSTRUCTOR = getConstructor( + craftOfflinePlayerClass, true, craftServerClass, GameProfile.class); + + // SpigotDataHandler + // Paper 1.20.5+ uses Mojang mappings: NetworkManager -> Connection + Class networkManager = getClassSilently("net.minecraft.network.Connection"); + if (networkManager == null) { + networkManager = getClassSilently("net.minecraft.network.NetworkManager"); + } + if (networkManager == null) { + networkManager = getClassSilently(nmsPackage + "NetworkManager"); + } + if (networkManager == null) { + throw NmsDiagnostics.missingClass( + "NetworkManager/Connection", + "net.minecraft.network.Connection", + "net.minecraft.network.NetworkManager", + nmsPackage + "NetworkManager"); + } - // Paper 1.20.5+ uses Mojang mappings: PacketLoginInStart -> ServerboundHelloPacket - Class loginStartPacket = getClassSilently( - "net.minecraft.network.protocol.login.ServerboundHelloPacket"); - if (loginStartPacket == null) { - loginStartPacket = getClassSilently( - "net.minecraft.network.protocol.login.PacketLoginInStart"); - } - if (loginStartPacket == null) { - loginStartPacket = getClassSilently(nmsPackage + "PacketLoginInStart"); - } - if (loginStartPacket == null) { - throw new IllegalStateException("Could not find LoginStartPacket class."); - } - LOGIN_START_PACKET = loginStartPacket; + SOCKET_ADDRESS = getFieldOfType(networkManager, SocketAddress.class, false); - // Paper 1.20.5+ uses Mojang mappings: LoginListener -> ServerLoginPacketListenerImpl - Class loginListener = getClassSilently( - "net.minecraft.server.network.ServerLoginPacketListenerImpl"); - if (loginListener == null) { - loginListener = getClassSilently("net.minecraft.server.network.LoginListener"); - } - if (loginListener == null) { - loginListener = getClassSilently(nmsPackage + "LoginListener"); - } - if (loginListener == null) { - throw new IllegalStateException("Could not find LoginListener class."); - } - LOGIN_LISTENER = loginListener; - - LOGIN_PROFILE = getFieldOfType(LOGIN_LISTENER, GameProfile.class); - checkNotNull(LOGIN_PROFILE, "Profile from LoginListener"); - - LOGIN_DISCONNECT = getMethod(LOGIN_LISTENER, "disconnect", String.class); - checkNotNull(LOGIN_DISCONNECT, "LoginListener's disconnect method"); - - NETWORK_EXCEPTION_CAUGHT = getMethod( - networkManager, - "exceptionCaught", - ChannelHandlerContext.class, Throwable.class - ); - - VELOCITY_LOGIN_MESSAGE_ID = getField(LOGIN_LISTENER, "velocityLoginMessageId"); - - // there are multiple no-arg void methods - // Pre 1.20.2 uses initUUID so if it's null, we're on 1.20.2 or later - INIT_UUID = getMethod(LOGIN_LISTENER, "initUUID"); - IS_PRE_1_20_2 = INIT_UUID != null; - - // somewhere during 1.20.4 md_5 moved PreLogin logic to CraftBukkit - CALL_PLAYER_PRE_LOGIN_EVENTS = getMethod( - LOGIN_LISTENER, - "callPlayerPreLoginEvents", - GameProfile.class - ); - IS_POST_LOGIN_HANDLER = CALL_PLAYER_PRE_LOGIN_EVENTS != null; - - - if (IS_PRE_1_20_2) { - Class packetListenerClass = getClassSilently( - "net.minecraft.network.PacketListener"); - if (packetListenerClass == null) { - packetListenerClass = ReflectionUtils.getClassOrThrow( - nmsPackage + "PacketListener"); + // Paper 1.20.5+ uses Mojang mappings: PacketHandshakingInSetProtocol -> ClientIntentionPacket + Class handshakePacket = getClassSilently( + "net.minecraft.network.protocol.handshake.ClientIntentionPacket"); + if (handshakePacket == null) { + handshakePacket = getClassSilently( + "net.minecraft.network.protocol.handshake.PacketHandshakingInSetProtocol"); + } + if (handshakePacket == null) { + handshakePacket = getClassSilently(nmsPackage + "PacketHandshakingInSetProtocol"); } + if (handshakePacket == null) { + throw NmsDiagnostics.missingClass( + "HandshakePacket", + "net.minecraft.network.protocol.handshake.ClientIntentionPacket", + "net.minecraft.network.protocol.handshake.PacketHandshakingInSetProtocol", + nmsPackage + "PacketHandshakingInSetProtocol"); + } + HANDSHAKE_PACKET = handshakePacket; - PACKET_LISTENER = getFieldOfType(networkManager, packetListenerClass); - } else { - // We get the field by name on 1.20.2+ as there are now multiple fields of this type in network manager + HANDSHAKE_HOST = getFieldOfType(HANDSHAKE_PACKET, String.class); + checkNotNull(HANDSHAKE_HOST, "Handshake host"); - // PacketListener packetListener of NetworkManager - // Try Mojang mappings first (Paper 1.20.5+), then fall back to obfuscated name - Field packetListenerField = getField(networkManager, "packetListener"); - if (packetListenerField == null) { - packetListenerField = getField(networkManager, "q"); + // Paper 1.20.5+ uses Mojang mappings: PacketLoginInStart -> ServerboundHelloPacket + Class loginStartPacket = getClassSilently( + "net.minecraft.network.protocol.login.ServerboundHelloPacket"); + if (loginStartPacket == null) { + loginStartPacket = getClassSilently( + "net.minecraft.network.protocol.login.PacketLoginInStart"); } - PACKET_LISTENER = packetListenerField; - makeAccessible(PACKET_LISTENER); - } - checkNotNull(PACKET_LISTENER, "Packet listener"); - - if (IS_POST_LOGIN_HANDLER) { - makeAccessible(CALL_PLAYER_PRE_LOGIN_EVENTS); + if (loginStartPacket == null) { + loginStartPacket = getClassSilently(nmsPackage + "PacketLoginInStart"); + } + if (loginStartPacket == null) { + throw NmsDiagnostics.missingClass( + "LoginStartPacket", + "net.minecraft.network.protocol.login.ServerboundHelloPacket", + "net.minecraft.network.protocol.login.PacketLoginInStart", + nmsPackage + "PacketLoginInStart"); + } + LOGIN_START_PACKET = loginStartPacket; - // Try Mojang mappings first (Paper 1.20.5+), then fall back to obfuscated name - Method startClientVerificationMethod = getMethod(LOGIN_LISTENER, "startClientVerification", GameProfile.class); - if (startClientVerificationMethod == null) { - startClientVerificationMethod = getMethod(LOGIN_LISTENER, "b", GameProfile.class); + // Paper 1.20.5+ uses Mojang mappings: LoginListener -> ServerLoginPacketListenerImpl + Class loginListener = getClassSilently( + "net.minecraft.server.network.ServerLoginPacketListenerImpl"); + if (loginListener == null) { + loginListener = getClassSilently("net.minecraft.server.network.LoginListener"); } - START_CLIENT_VERIFICATION = startClientVerificationMethod; - checkNotNull(START_CLIENT_VERIFICATION, "startClientVerification"); - makeAccessible(START_CLIENT_VERIFICATION); - - LOGIN_HANDLER_CONSTRUCTOR = null; - FIRE_LOGIN_EVENTS = null; - FIRE_LOGIN_EVENTS_GAME_PROFILE = null; - } else { - // Paper 1.20.5+ uses Mojang mappings: LoginListener$LoginHandler -> ServerLoginPacketListenerImpl$LoginHandler - Class loginHandler = getClassSilently( - "net.minecraft.server.network.ServerLoginPacketListenerImpl$LoginHandler"); - if (loginHandler == null) { - loginHandler = getClassSilently( - "net.minecraft.server.network.LoginListener$LoginHandler"); + if (loginListener == null) { + loginListener = getClassSilently(nmsPackage + "LoginListener"); } - if (loginHandler == null) { - loginHandler = ReflectionUtils.getClassOrThrow( - nmsPackage + "LoginListener$LoginHandler"); + if (loginListener == null) { + throw NmsDiagnostics.missingClass( + "LoginListener/ServerLoginPacketListenerImpl", + "net.minecraft.server.network.ServerLoginPacketListenerImpl", + "net.minecraft.server.network.LoginListener", + nmsPackage + "LoginListener"); } - LOGIN_HANDLER_CONSTRUCTOR = - getConstructor(loginHandler, true, LOGIN_LISTENER); - checkNotNull(LOGIN_HANDLER_CONSTRUCTOR, "LoginHandler constructor"); + LOGIN_LISTENER = loginListener; - FIRE_LOGIN_EVENTS = getMethod(loginHandler, "fireEvents"); + LOGIN_PROFILE = getFieldOfType(LOGIN_LISTENER, GameProfile.class); + checkNotNull(LOGIN_PROFILE, "Profile from LoginListener"); - // LoginHandler().fireEvents(GameProfile) - FIRE_LOGIN_EVENTS_GAME_PROFILE = getMethod(loginHandler, "fireEvents", - GameProfile.class); - checkNotNull(FIRE_LOGIN_EVENTS, FIRE_LOGIN_EVENTS_GAME_PROFILE, - "fireEvents from LoginHandler", "fireEvents(GameProfile) from LoginHandler"); + LOGIN_DISCONNECT = getMethod(LOGIN_LISTENER, "disconnect", String.class); + checkNotNull(LOGIN_DISCONNECT, "LoginListener's disconnect method"); - START_CLIENT_VERIFICATION = null; - } + NETWORK_EXCEPTION_CAUGHT = getMethod( + networkManager, + "exceptionCaught", + ChannelHandlerContext.class, Throwable.class + ); - PAPER_DISABLE_USERNAME_VALIDATION = getField(LOGIN_LISTENER, - "iKnowThisMayNotBeTheBestIdeaButPleaseDisableUsernameValidation"); + VELOCITY_LOGIN_MESSAGE_ID = getField(LOGIN_LISTENER, "velocityLoginMessageId"); - if (Constants.DEBUG_MODE) { - System.out.println("Paper disable username validation field exists? " + - (PAPER_DISABLE_USERNAME_VALIDATION != null)); - } + // there are multiple no-arg void methods + // Pre 1.20.2 uses initUUID so if it's null, we're on 1.20.2 or later + INIT_UUID = getMethod(LOGIN_LISTENER, "initUUID"); + IS_PRE_1_20_2 = INIT_UUID != null; + + // somewhere during 1.20.4 md_5 moved PreLogin logic to CraftBukkit + CALL_PLAYER_PRE_LOGIN_EVENTS = getMethod( + LOGIN_LISTENER, + "callPlayerPreLoginEvents", + GameProfile.class + ); + IS_POST_LOGIN_HANDLER = CALL_PLAYER_PRE_LOGIN_EVENTS != null; + + + if (IS_PRE_1_20_2) { + Class packetListenerClass = getClassSilently( + "net.minecraft.network.PacketListener"); + if (packetListenerClass == null) { + packetListenerClass = ReflectionUtils.getClassOrThrow( + nmsPackage + "PacketListener"); + } - // ProxyUtils - Class spigotConfig = ReflectionUtils.getClass("org.spigotmc.SpigotConfig"); - checkNotNull(spigotConfig, "Spigot config"); - - BUNGEE = getField(spigotConfig, "bungee"); - checkNotNull(BUNGEE, "Bungee field"); - - Class paperConfigNew = getClassSilently( - "io.papermc.paper.configuration.GlobalConfiguration"); - if (paperConfigNew != null) { - // 1.19 and later - Method paperConfigGet = checkNotNull(getMethod(paperConfigNew, "get"), - "GlobalConfiguration get"); - Field paperConfigProxies = checkNotNull(getField(paperConfigNew, "proxies"), - "Proxies field"); - Field paperConfigVelocity = checkNotNull( - getField(paperConfigProxies.getType(), "velocity"), - "velocity field"); - Field paperVelocityEnabled = checkNotNull( - getField(paperConfigVelocity.getType(), "enabled"), - "Velocity enabled field"); - PAPER_VELOCITY_SUPPORT = () -> { - Object paperConfigInstance = invoke(null, paperConfigGet); - Object proxiesInstance = getValue(paperConfigInstance, paperConfigProxies); - Object velocityInstance = getValue(proxiesInstance, paperConfigVelocity); - return getBooleanValue(velocityInstance, paperVelocityEnabled); - }; - } else { - // Pre-1.19 - Class paperConfig = getClassSilently( - "com.destroystokyo.paper.PaperConfig"); - - if (paperConfig != null) { - Field velocitySupport = getField(paperConfig, "velocitySupport"); - // velocitySupport field is null pre-1.13 - PAPER_VELOCITY_SUPPORT = velocitySupport != null ? - () -> castedStaticBooleanValue(velocitySupport) : null; + PACKET_LISTENER = getFieldOfType(networkManager, packetListenerClass); } else { - PAPER_VELOCITY_SUPPORT = null; + // We get the field by name on 1.20.2+ as there are now multiple fields of this type in network manager + + // PacketListener packetListener of NetworkManager + // Try Mojang mappings first (Paper 1.20.5+), then fall back to obfuscated name + Field packetListenerField = getField(networkManager, "packetListener"); + if (packetListenerField == null) { + packetListenerField = getField(networkManager, "q"); + } + PACKET_LISTENER = packetListenerField; + makeAccessible(PACKET_LISTENER); } - } + checkNotNull(PACKET_LISTENER, "Packet listener"); - IS_FOLIA = ReflectionUtils.getClassSilently( - "io.papermc.paper.threadedregions.RegionizedServer" - ) != null; + if (IS_POST_LOGIN_HANDLER) { + makeAccessible(CALL_PLAYER_PRE_LOGIN_EVENTS); + // Try Mojang mappings first (Paper 1.20.5+), then fall back to obfuscated name + Method startClientVerificationMethod = getMethod(LOGIN_LISTENER, "startClientVerification", GameProfile.class); + if (startClientVerificationMethod == null) { + startClientVerificationMethod = getMethod(LOGIN_LISTENER, "b", GameProfile.class); + } + START_CLIENT_VERIFICATION = startClientVerificationMethod; + checkNotNull(START_CLIENT_VERIFICATION, "startClientVerification"); + makeAccessible(START_CLIENT_VERIFICATION); - if (!IS_PRE_1_20_2) { - // PacketHandshakingInSetProtocol is now a record - // This means its fields are now private and final - // We therefore must use reflection to obtain the constructor - CLIENT_INTENT = getClassOrFallback( - "net.minecraft.network.protocol.handshake.ClientIntent", - nmsPackage + "ClientIntent" - ); - checkNotNull(CLIENT_INTENT, "Client intent enum"); + LOGIN_HANDLER_CONSTRUCTOR = null; + FIRE_LOGIN_EVENTS = null; + FIRE_LOGIN_EVENTS_GAME_PROFILE = null; + } else { + // Paper 1.20.5+ uses Mojang mappings: LoginListener$LoginHandler -> ServerLoginPacketListenerImpl$LoginHandler + Class loginHandler = getClassSilently( + "net.minecraft.server.network.ServerLoginPacketListenerImpl$LoginHandler"); + if (loginHandler == null) { + loginHandler = getClassSilently( + "net.minecraft.server.network.LoginListener$LoginHandler"); + } + if (loginHandler == null) { + loginHandler = ReflectionUtils.getClassOrThrow( + nmsPackage + "LoginListener$LoginHandler"); + } + LOGIN_HANDLER_CONSTRUCTOR = + getConstructor(loginHandler, true, LOGIN_LISTENER); + checkNotNull(LOGIN_HANDLER_CONSTRUCTOR, "LoginHandler constructor"); + + FIRE_LOGIN_EVENTS = getMethod(loginHandler, "fireEvents"); - HANDSHAKE_PACKET_CONSTRUCTOR = getConstructor(HANDSHAKE_PACKET, false, int.class, - String.class, int.class, CLIENT_INTENT); - checkNotNull(HANDSHAKE_PACKET_CONSTRUCTOR, "Handshake packet constructor"); + // LoginHandler().fireEvents(GameProfile) + FIRE_LOGIN_EVENTS_GAME_PROFILE = getMethod(loginHandler, "fireEvents", + GameProfile.class); + checkNotNull(FIRE_LOGIN_EVENTS, FIRE_LOGIN_EVENTS_GAME_PROFILE, + "fireEvents from LoginHandler", "fireEvents(GameProfile) from LoginHandler"); - // Paper 1.20.5+ Mojang mappings expose real field names; older Spigot uses obfuscated a/b/c/d. - // Try Mojang name first, then obfuscated. - Field protocolField = getField(HANDSHAKE_PACKET, "protocolVersion"); - if (protocolField == null) { - protocolField = getField(HANDSHAKE_PACKET, "a"); + START_CLIENT_VERIFICATION = null; } - checkNotNull(protocolField, "Handshake \"a\" field (protocol version, or stream codec)"); - - if (protocolField.getType().isPrimitive()) { - // Mojang on 1.20.5+ OR obfuscated 1.20.2-1.20.4: int field is the protocol version - HANDSHAKE_PROTOCOL = protocolField; - Field portField = getField(HANDSHAKE_PACKET, "port"); - if (portField == null) { - portField = getField(HANDSHAKE_PACKET, "c"); - } - HANDSHAKE_PORT = portField; + + PAPER_DISABLE_USERNAME_VALIDATION = getField(LOGIN_LISTENER, + "iKnowThisMayNotBeTheBestIdeaButPleaseDisableUsernameValidation"); + + if (Constants.DEBUG_MODE) { + System.out.println("Paper disable username validation field exists? " + + (PAPER_DISABLE_USERNAME_VALIDATION != null)); + } + + // ProxyUtils + Class spigotConfig = ReflectionUtils.getClass("org.spigotmc.SpigotConfig"); + checkNotNull(spigotConfig, "Spigot config"); + + BUNGEE = getField(spigotConfig, "bungee"); + checkNotNull(BUNGEE, "Bungee field"); + + Class paperConfigNew = getClassSilently( + "io.papermc.paper.configuration.GlobalConfiguration"); + if (paperConfigNew != null) { + // 1.19 and later + Method paperConfigGet = checkNotNull(getMethod(paperConfigNew, "get"), + "GlobalConfiguration get"); + Field paperConfigProxies = checkNotNull(getField(paperConfigNew, "proxies"), + "Proxies field"); + Field paperConfigVelocity = checkNotNull( + getField(paperConfigProxies.getType(), "velocity"), + "velocity field"); + Field paperVelocityEnabled = checkNotNull( + getField(paperConfigVelocity.getType(), "enabled"), + "Velocity enabled field"); + PAPER_VELOCITY_SUPPORT = () -> { + Object paperConfigInstance = invoke(null, paperConfigGet); + Object proxiesInstance = getValue(paperConfigInstance, paperConfigProxies); + Object velocityInstance = getValue(proxiesInstance, paperConfigVelocity); + return getBooleanValue(velocityInstance, paperVelocityEnabled); + }; } else { - // Obfuscated 1.20.5: a is the stream_codec, everything is shifted - HANDSHAKE_PROTOCOL = getField(HANDSHAKE_PACKET, "b"); - Field portField = getField(HANDSHAKE_PACKET, "port"); - if (portField == null) { - portField = getField(HANDSHAKE_PACKET, "d"); + // Pre-1.19 + Class paperConfig = getClassSilently( + "com.destroystokyo.paper.PaperConfig"); + + if (paperConfig != null) { + Field velocitySupport = getField(paperConfig, "velocitySupport"); + // velocitySupport field is null pre-1.13 + PAPER_VELOCITY_SUPPORT = velocitySupport != null ? + () -> castedStaticBooleanValue(velocitySupport) : null; + } else { + PAPER_VELOCITY_SUPPORT = null; } - HANDSHAKE_PORT = portField; } - checkNotNull(HANDSHAKE_PROTOCOL, "Handshake protocol"); - makeAccessible(HANDSHAKE_PROTOCOL); + IS_FOLIA = ReflectionUtils.getClassSilently( + "io.papermc.paper.threadedregions.RegionizedServer" + ) != null; + + + if (!IS_PRE_1_20_2) { + // PacketHandshakingInSetProtocol is now a record + // This means its fields are now private and final + // We therefore must use reflection to obtain the constructor + CLIENT_INTENT = getClassOrFallback( + "net.minecraft.network.protocol.handshake.ClientIntent", + nmsPackage + "ClientIntent" + ); + checkNotNull(CLIENT_INTENT, "Client intent enum"); + + HANDSHAKE_PACKET_CONSTRUCTOR = getConstructor(HANDSHAKE_PACKET, false, int.class, + String.class, int.class, CLIENT_INTENT); + checkNotNull(HANDSHAKE_PACKET_CONSTRUCTOR, "Handshake packet constructor"); + + // Paper 1.20.5+ Mojang mappings expose real field names; older Spigot uses obfuscated a/b/c/d. + // Try Mojang name first, then obfuscated. + Field protocolField = getField(HANDSHAKE_PACKET, "protocolVersion"); + if (protocolField == null) { + protocolField = getField(HANDSHAKE_PACKET, "a"); + } + checkNotNull(protocolField, "Handshake \"a\" field (protocol version, or stream codec)"); + + if (protocolField.getType().isPrimitive()) { + // Mojang on 1.20.5+ OR obfuscated 1.20.2-1.20.4: int field is the protocol version + HANDSHAKE_PROTOCOL = protocolField; + Field portField = getField(HANDSHAKE_PACKET, "port"); + if (portField == null) { + portField = getField(HANDSHAKE_PACKET, "c"); + } + HANDSHAKE_PORT = portField; + } else { + // Obfuscated 1.20.5: a is the stream_codec, everything is shifted + HANDSHAKE_PROTOCOL = getField(HANDSHAKE_PACKET, "b"); + Field portField = getField(HANDSHAKE_PACKET, "port"); + if (portField == null) { + portField = getField(HANDSHAKE_PACKET, "d"); + } + HANDSHAKE_PORT = portField; + } - checkNotNull(HANDSHAKE_PORT, "Handshake port"); - makeAccessible(HANDSHAKE_PORT); + checkNotNull(HANDSHAKE_PROTOCOL, "Handshake protocol"); + makeAccessible(HANDSHAKE_PROTOCOL); - // Try Mojang field name first, then fall back to type-based lookup (obfuscated) - Field intentionField = getField(HANDSHAKE_PACKET, "intention"); - if (intentionField == null) { - intentionField = getFieldOfType(HANDSHAKE_PACKET, CLIENT_INTENT); + checkNotNull(HANDSHAKE_PORT, "Handshake port"); + makeAccessible(HANDSHAKE_PORT); + + // Try Mojang field name first, then fall back to type-based lookup (obfuscated) + Field intentionField = getField(HANDSHAKE_PACKET, "intention"); + if (intentionField == null) { + intentionField = getFieldOfType(HANDSHAKE_PACKET, CLIENT_INTENT); + } + HANDSHAKE_INTENTION = intentionField; + checkNotNull(HANDSHAKE_INTENTION, "Handshake intention"); + makeAccessible(HANDSHAKE_INTENTION); + } else { + CLIENT_INTENT = null; + HANDSHAKE_PACKET_CONSTRUCTOR = null; + HANDSHAKE_PORT = null; + HANDSHAKE_PROTOCOL = null; + HANDSHAKE_INTENTION = null; } - HANDSHAKE_INTENTION = intentionField; - checkNotNull(HANDSHAKE_INTENTION, "Handshake intention"); - makeAccessible(HANDSHAKE_INTENTION); - } else { - CLIENT_INTENT = null; - HANDSHAKE_PACKET_CONSTRUCTOR = null; - HANDSHAKE_PORT = null; - HANDSHAKE_PROTOCOL = null; - HANDSHAKE_INTENTION = null; + } catch (RuntimeException | Error failure) { + NmsDiagnostics.recordInitializationFailure(failure); + throw failure; } } @@ -438,7 +464,12 @@ private static Class getClassOrFallback(String className, String fallbackName } private static T checkNotNull(@CheckForNull T toCheck, @CheckForNull String objectName) { - return Preconditions.checkNotNull(toCheck, objectName + " cannot be null"); + if (toCheck == null) { + // Names the accessor and the server it was looked up on, so a mapping drift is + // actionable from the log line alone instead of a bare "... cannot be null" NPE. + throw NmsDiagnostics.missingAccessor(objectName, null); + } + return toCheck; } // Ensure one of two is not null @@ -448,7 +479,11 @@ private static T checkNotNull( @CheckForNull String objectName, @CheckForNull String objectName2 ) { - return Preconditions.checkNotNull(toCheck != null ? toCheck : toCheck2, - objectName2 + " cannot be null if " + objectName + " is null"); + T resolved = toCheck != null ? toCheck : toCheck2; + if (resolved == null) { + throw NmsDiagnostics.missingAccessor(objectName2, + "Required because '" + objectName + "' is also missing."); + } + return resolved; } } diff --git a/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java b/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java new file mode 100644 index 000000000..5d27c028c --- /dev/null +++ b/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java @@ -0,0 +1,231 @@ +/* + * Copyright (c) 2021-2022 Minekube. https://minekube.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @author Minekube + * @link https://github.com/minekube/connect-java + */ + +package com.minekube.connect.util; + +import java.lang.reflect.Method; +import java.util.Arrays; +import javax.annotation.Nullable; +import org.bukkit.Bukkit; + +/** + * Describes the running server and latches {@link ClassNames} initialization failures. + * + *

{@code ClassNames} resolves the server's internals reflectively from a static initializer. + * When a Minecraft release renames or removes one of those accessors the initializer throws, and + * from then on the JVM answers every later touch of {@code ClassNames} with + * {@code NoClassDefFoundError: Could not initialize class ...ClassNames}, whose own message names + * only the class — the reason that names the drifted accessor is, at best, buried in a cause + * chain, and the enable path is not guaranteed to hit the informative first touch. + * + *

This class deliberately lives outside {@code ClassNames} so it still initializes fine once + * {@code ClassNames} is in the erroneous state. That is what lets + * {@link #initializationFailure()} hand the original, named reason straight back to the enable + * path, with no cause-chain digging. + * + *

Nothing here may throw: it only ever runs while Connect is already reporting a failure. + */ +public final class NmsDiagnostics { + private static final String UNKNOWN = "unknown"; + + private static volatile String initializationFailure; + + private NmsDiagnostics() { + } + + /** + * Builds the failure for a server internal Connect knows about but could not resolve. + * + * @param accessor the exact accessor that is missing, e.g. + * {@code ServerLoginPacketListenerImpl#startClientVerification(GameProfile)} + * @param detail optional extra context, e.g. the names that were tried + */ + public static IllegalStateException missingAccessor( + @Nullable String accessor, + @Nullable String detail) { + StringBuilder message = new StringBuilder() + .append("Connect could not resolve the server internal '") + .append(accessor == null ? UNKNOWN : accessor) + .append("' on this server."); + if (detail != null && !detail.isEmpty()) { + message.append(' ').append(detail); + } + message.append(" Connect binds to server internals reflectively, so this means the") + .append(" server's mappings differ from every name Connect knows — a Minecraft") + .append(" version/mapping change, not a Java runtime incompatibility.") + .append(" Environment: ").append(environment()).append('.') + .append(" Please report this exact line at") + .append(" https://github.com/minekube/connect-java/issues"); + return new IllegalStateException(message.toString()); + } + + /** + * Builds the failure for a server internal class Connect could not find under any known name. + */ + public static IllegalStateException missingClass( + @Nullable String what, + String... candidates) { + String detail = candidates.length == 0 + ? null + : "Tried: " + String.join(", ", Arrays.asList(candidates)) + "."; + return missingAccessor(what, detail); + } + + /** + * Records why {@code ClassNames} failed to initialize, so the reason survives the + * causeless {@code NoClassDefFoundError}s the JVM raises on every subsequent touch. + */ + public static void recordInitializationFailure(@Nullable Throwable failure) { + if (failure == null) { + return; + } + String message = failure.getMessage(); + if (message == null || message.isEmpty()) { + message = failure.getClass().getName(); + } + // Keep the first failure: it is the one that names the accessor that actually drifted. + if (initializationFailure == null) { + initializationFailure = message; + } + } + + /** + * The recorded {@code ClassNames} initialization failure, or {@code null} if it initialized + * fine. Callers use this to report something actionable instead of a causeless + * {@code NoClassDefFoundError}. + */ + @Nullable + public static String initializationFailure() { + return initializationFailure; + } + + /** Test seam: resets the latch so a simulated failure does not leak between tests. */ + static void resetInitializationFailure() { + initializationFailure = null; + } + + /** + * A single-line description of the server software and runtime, safe to log. Every lookup + * degrades to {@code unknown} rather than throwing, because this runs on servers whose API + * shape is by definition not what Connect expected. + */ + public static String environment() { + return "server=" + serverName() + + ", serverVersion=" + serverVersion() + + ", bukkitVersion=" + bukkitVersion() + + ", minecraftVersion=" + minecraftVersion() + + ", craftbukkitPackage=" + craftBukkitPackage() + + ", java=" + javaVersion(); + } + + private static String serverName() { + try { + return orUnknown(Bukkit.getName()); + } catch (Throwable ignored) { + return UNKNOWN; + } + } + + private static String serverVersion() { + try { + return orUnknown(Bukkit.getVersion()); + } catch (Throwable ignored) { + return UNKNOWN; + } + } + + private static String bukkitVersion() { + try { + return orUnknown(Bukkit.getBukkitVersion()); + } catch (Throwable ignored) { + return UNKNOWN; + } + } + + /** + * The Minecraft version. Prefers {@code Server#getMinecraftVersion()} (Paper) and otherwise + * derives it from the Bukkit version, which is shaped like {@code 1.21.4-R0.1-SNAPSHOT}. + */ + private static String minecraftVersion() { + try { + Object server = Bukkit.getServer(); + if (server != null) { + // Paper-only accessor, resolved reflectively so this compiles and runs on Spigot. + Method getMinecraftVersion = ReflectionUtils.getMethod( + server.getClass(), "getMinecraftVersion"); + if (getMinecraftVersion != null) { + Object version = ReflectionUtils.invoke(server, getMinecraftVersion); + if (version instanceof String && !((String) version).isEmpty()) { + return (String) version; + } + } + } + } catch (Throwable ignored) { + // fall through to the Bukkit version + } + try { + String bukkitVersion = Bukkit.getBukkitVersion(); + if (bukkitVersion != null && !bukkitVersion.isEmpty()) { + int dash = bukkitVersion.indexOf('-'); + return dash > 0 ? bukkitVersion.substring(0, dash) : bukkitVersion; + } + } catch (Throwable ignored) { + // fall through + } + return UNKNOWN; + } + + /** + * The CraftBukkit package, which reveals the mapping era: a relocated + * {@code org.bukkit.craftbukkit.v1_8_R3} on older servers versus a bare + * {@code org.bukkit.craftbukkit} on Paper 1.20.5+. + */ + private static String craftBukkitPackage() { + try { + Object server = Bukkit.getServer(); + if (server == null) { + return UNKNOWN; + } + Package pkg = server.getClass().getPackage(); + return pkg == null ? UNKNOWN : orUnknown(pkg.getName()); + } catch (Throwable ignored) { + return UNKNOWN; + } + } + + private static String javaVersion() { + try { + String version = orUnknown(System.getProperty("java.version")); + String vendor = System.getProperty("java.vendor"); + return vendor == null || vendor.isEmpty() ? version : version + " (" + vendor + ")"; + } catch (Throwable ignored) { + return UNKNOWN; + } + } + + private static String orUnknown(@Nullable String value) { + return value == null || value.isEmpty() ? UNKNOWN : value; + } +} diff --git a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java new file mode 100644 index 000000000..e41236476 --- /dev/null +++ b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java @@ -0,0 +1,246 @@ +/* + * Copyright (c) 2021-2022 Minekube. https://minekube.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @author Minekube + * @link https://github.com/minekube/connect-java + */ + +package com.minekube.connect.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; + +import java.io.File; +import java.lang.reflect.Method; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Logger; +import org.bukkit.Bukkit; +import org.bukkit.Server; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Pins the diagnosability of Connect's server-internals ({@code ClassNames}) reflection. + * + *

Connect binds to server internals reflectively from a static initializer. When a Minecraft + * release renames or removes one of those accessors, the initializer throws — and before this was + * hardened the operator got a bare {@code NullPointerException("... cannot be null")} on the first + * touch and {@code NoClassDefFoundError: Could not initialize class ...ClassNames} on every touch + * after that. Neither named the server software or the Minecraft version, and only the first named + * the accessor at all, which is what made a real report impossible to act on. + * + *

These tests fail against that pre-fix behaviour. + */ +class NmsDiagnosticsTest { + private static final String SERVER_NAME = "Paper"; + private static final String MINECRAFT_VERSION = "1.21.4"; + private static final String BUKKIT_VERSION = MINECRAFT_VERSION + "-R0.1-SNAPSHOT"; + private static final String SERVER_VERSION = "git-Paper-196 (MC: " + MINECRAFT_VERSION + ")"; + + @BeforeAll + static void installServer() { + if (Bukkit.getServer() != null) { + return; + } + Server server = mock(Server.class); + lenient().when(server.getName()).thenReturn(SERVER_NAME); + lenient().when(server.getVersion()).thenReturn(SERVER_VERSION); + lenient().when(server.getBukkitVersion()).thenReturn(BUKKIT_VERSION); + // Bukkit.setServer logs its banner through the server's logger. + lenient().when(server.getLogger()).thenReturn(Logger.getLogger(SERVER_NAME)); + Bukkit.setServer(server); + } + + @BeforeEach + void resetLatch() { + NmsDiagnostics.resetInitializationFailure(); + } + + @Test + void missingAccessorNamesTheAccessorAndTheServerItWasLookedUpOn() { + IllegalStateException failure = NmsDiagnostics.missingAccessor( + "ServerLoginPacketListenerImpl#startClientVerification(GameProfile)", null); + + String message = failure.getMessage(); + assertNotNull(message); + // The exact accessor that drifted - the whole point of the report. + assertTrue(message.contains("startClientVerification"), + "must name the missing accessor: " + message); + // Server software and Minecraft version, so the drift can be pinned to a release. + assertTrue(message.contains(SERVER_NAME), "must name the server software: " + message); + assertTrue(message.contains(MINECRAFT_VERSION), + "must name the Minecraft version: " + message); + assertTrue(message.contains(BUKKIT_VERSION), + "must name the Bukkit version: " + message); + // The Java version is reported too, so a Java-runtime theory can be ruled out on sight + // instead of being guessed at. + assertTrue(message.contains("java="), "must report the Java version: " + message); + assertTrue(message.contains("not a Java runtime incompatibility"), + "must steer away from a Java-runtime misdiagnosis: " + message); + } + + @Test + void missingClassListsEveryNameThatWasTried() { + IllegalStateException failure = NmsDiagnostics.missingClass( + "LoginListener/ServerLoginPacketListenerImpl", + "net.minecraft.server.network.ServerLoginPacketListenerImpl", + "net.minecraft.server.network.LoginListener"); + + String message = failure.getMessage(); + assertTrue(message.contains("ServerLoginPacketListenerImpl"), + "must list the Mojang-mapped candidate: " + message); + assertTrue(message.contains("net.minecraft.server.network.LoginListener"), + "must list the fallback candidate: " + message); + } + + @Test + void environmentDegradesInsteadOfThrowingWhenLookupsFail() { + // Runs on servers whose API shape is by definition not what Connect expected, so it must + // never throw on top of the failure it is describing. + String environment = NmsDiagnostics.environment(); + assertNotNull(environment); + assertTrue(environment.contains("server="), environment); + assertTrue(environment.contains("java="), environment); + } + + @Test + void latchKeepsTheFirstFailureRatherThanALaterLessInformativeOne() { + NmsDiagnostics.recordInitializationFailure( + new IllegalStateException("names the drifted accessor")); + NmsDiagnostics.recordInitializationFailure( + new NoClassDefFoundError("com/minekube/connect/util/ClassNames")); + + assertEquals("names the drifted accessor", NmsDiagnostics.initializationFailure()); + } + + @Test + void noFailureIsReportedWhenNothingWentWrong() { + assertNull(NmsDiagnostics.initializationFailure()); + } + + /** + * The end-to-end shape of the bug: a real {@code ClassNames} static-init failure must stay + * readable after the JVM has started answering with causeless {@code NoClassDefFoundError}s. + * + *

{@code ClassNames} is loaded in a throwaway child-first loader so its initializer runs + * for real. No {@code net.minecraft.*} exists on the test classpath, so it fails exactly the + * way a drifted mapping fails on a live server. + */ + @Test + void classNamesInitFailureStaysReadableAfterTheCauselessNoClassDefFoundError() + throws Exception { + try (ChildFirstLoader loader = new ChildFirstLoader()) { + // First touch: the JVM still carries the reason. + ExceptionInInitializerError first = assertThrows( + ExceptionInInitializerError.class, + () -> Class.forName(ClassNames.class.getName(), true, loader)); + assertNotNull(first.getCause(), "first touch should still carry a cause"); + + // Every later touch degrades to "Could not initialize class ...ClassNames": the + // top-level message names only the class, never the accessor that actually drifted. + // That is the burial being pinned - the reason is at best buried in a cause chain. + NoClassDefFoundError later = assertThrows( + NoClassDefFoundError.class, + () -> Class.forName(ClassNames.class.getName(), true, loader)); + assertNotNull(later.getMessage()); + assertTrue(!later.getMessage().contains("MinecraftServer"), + "the re-initialization error alone never names the drifted accessor: " + + later.getMessage()); + + // The latch must still hand back the original, actionable reason. + Class diagnostics = Class.forName( + NmsDiagnostics.class.getName(), true, loader); + Method initializationFailure = + diagnostics.getDeclaredMethod("initializationFailure"); + Object recorded = initializationFailure.invoke(null); + + assertNotNull(recorded, + "the reason ClassNames failed to initialize must survive the burial"); + String message = (String) recorded; + assertTrue(message.contains("MinecraftServer"), + "must name the server internal that could not be resolved: " + message); + assertTrue(message.contains(SERVER_NAME), + "must name the server software: " + message); + assertTrue(message.contains(MINECRAFT_VERSION), + "must name the Minecraft version: " + message); + } + } + + /** + * Loads Connect's util classes child-first so {@code ClassNames} gets a fresh, uninitialized + * copy per test, while {@code org.bukkit.*} stays parent-loaded so the mock server installed + * above is the one it sees. + */ + private static final class ChildFirstLoader extends URLClassLoader { + private static final String CHILD_FIRST_PREFIX = "com.minekube.connect.util."; + + ChildFirstLoader() { + super(classPathUrls(), NmsDiagnosticsTest.class.getClassLoader()); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + synchronized (getClassLoadingLock(name)) { + Class loaded = findLoadedClass(name); + if (loaded == null && name.startsWith(CHILD_FIRST_PREFIX)) { + try { + loaded = findClass(name); + } catch (ClassNotFoundException ignored) { + loaded = null; + } + } + if (loaded == null) { + loaded = super.loadClass(name, false); + } + if (resolve) { + resolveClass(loaded); + } + return loaded; + } + } + + private static URL[] classPathUrls() { + List urls = new ArrayList<>(); + for (String entry : System.getProperty("java.class.path", "") + .split(File.pathSeparator)) { + if (entry.isEmpty()) { + continue; + } + try { + urls.add(new File(entry).toURI().toURL()); + } catch (MalformedURLException ignored) { + // Skip unusable entries; the parent loader remains the fallback. + } + } + return urls.toArray(new URL[0]); + } + } +} From d921a849d75d178dbcd977274547c0039219499b Mon Sep 17 00:00:00 2001 From: "no-mistakes[bot]" Date: Tue, 28 Jul 2026 02:28:10 +0200 Subject: [PATCH 2/5] no-mistakes(review): Hardened ClassNames drift diagnostics across all lookup paths --- .../com/minekube/connect/util/ClassNames.java | 226 ++++++++---------- .../connect/util/NmsDiagnosticsTest.java | 36 +++ .../craftbukkit/v1_21_R1/VersionedServer.java | 6 + .../v1_21_R1/entity/CraftPlayer.java | 7 + 4 files changed, 148 insertions(+), 127 deletions(-) create mode 100644 spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/VersionedServer.java create mode 100644 spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/entity/CraftPlayer.java diff --git a/spigot/src/main/java/com/minekube/connect/util/ClassNames.java b/spigot/src/main/java/com/minekube/connect/util/ClassNames.java index 6fab2542c..075ef2e63 100644 --- a/spigot/src/main/java/com/minekube/connect/util/ClassNames.java +++ b/spigot/src/main/java/com/minekube/connect/util/ClassNames.java @@ -112,10 +112,11 @@ public class ClassNames { String nmsPackage = "net.minecraft.server." + version; // SpigotSkinApplier - Class craftPlayerClass = ReflectionUtils.getClass( + Class craftPlayerClass = getRequiredClass( + "CraftPlayer", "org.bukkit.craftbukkit." + version + "entity.CraftPlayer"); GET_PROFILE_METHOD = getMethod(craftPlayerClass, "getProfile"); - checkNotNull(GET_PROFILE_METHOD, "Get profile method"); + checkNotNull(GET_PROFILE_METHOD, "CraftPlayer#getProfile()"); // SpigotInjector MINECRAFT_SERVER = getClassOrFallback( @@ -124,28 +125,18 @@ public class ClassNames { ); // Paper 1.20.5+ uses Mojang mappings: ServerConnection -> ServerConnectionListener - Class serverConnectionClass = getClassSilently( - "net.minecraft.server.network.ServerConnectionListener"); - if (serverConnectionClass == null) { - serverConnectionClass = getClassSilently( - "net.minecraft.server.network.ServerConnection"); - } - if (serverConnectionClass == null) { - serverConnectionClass = getClassSilently(nmsPackage + "ServerConnection"); - } - if (serverConnectionClass == null) { - throw NmsDiagnostics.missingClass( - "ServerConnection/ServerConnectionListener", - "net.minecraft.server.network.ServerConnectionListener", - "net.minecraft.server.network.ServerConnection", - nmsPackage + "ServerConnection"); - } - SERVER_CONNECTION = serverConnectionClass; + SERVER_CONNECTION = getRequiredClass( + "ServerConnection/ServerConnectionListener", + "net.minecraft.server.network.ServerConnectionListener", + "net.minecraft.server.network.ServerConnection", + nmsPackage + "ServerConnection"); // WhitelistUtils - Class craftServerClass = ReflectionUtils.getClass( + Class craftServerClass = getRequiredClass( + "CraftServer", "org.bukkit.craftbukkit." + version + "CraftServer"); - Class craftOfflinePlayerClass = ReflectionUtils.getCastedClass( + Class craftOfflinePlayerClass = getRequiredClass( + "CraftOfflinePlayer", "org.bukkit.craftbukkit." + version + "CraftOfflinePlayer"); CRAFT_OFFLINE_PLAYER_CONSTRUCTOR = getConstructor( @@ -153,87 +144,46 @@ public class ClassNames { // SpigotDataHandler // Paper 1.20.5+ uses Mojang mappings: NetworkManager -> Connection - Class networkManager = getClassSilently("net.minecraft.network.Connection"); - if (networkManager == null) { - networkManager = getClassSilently("net.minecraft.network.NetworkManager"); - } - if (networkManager == null) { - networkManager = getClassSilently(nmsPackage + "NetworkManager"); - } - if (networkManager == null) { - throw NmsDiagnostics.missingClass( - "NetworkManager/Connection", - "net.minecraft.network.Connection", - "net.minecraft.network.NetworkManager", - nmsPackage + "NetworkManager"); - } + Class networkManager = getRequiredClass( + "NetworkManager/Connection", + "net.minecraft.network.Connection", + "net.minecraft.network.NetworkManager", + nmsPackage + "NetworkManager"); SOCKET_ADDRESS = getFieldOfType(networkManager, SocketAddress.class, false); // Paper 1.20.5+ uses Mojang mappings: PacketHandshakingInSetProtocol -> ClientIntentionPacket - Class handshakePacket = getClassSilently( - "net.minecraft.network.protocol.handshake.ClientIntentionPacket"); - if (handshakePacket == null) { - handshakePacket = getClassSilently( - "net.minecraft.network.protocol.handshake.PacketHandshakingInSetProtocol"); - } - if (handshakePacket == null) { - handshakePacket = getClassSilently(nmsPackage + "PacketHandshakingInSetProtocol"); - } - if (handshakePacket == null) { - throw NmsDiagnostics.missingClass( - "HandshakePacket", - "net.minecraft.network.protocol.handshake.ClientIntentionPacket", - "net.minecraft.network.protocol.handshake.PacketHandshakingInSetProtocol", - nmsPackage + "PacketHandshakingInSetProtocol"); - } + Class handshakePacket = getRequiredClass( + "HandshakePacket", + "net.minecraft.network.protocol.handshake.ClientIntentionPacket", + "net.minecraft.network.protocol.handshake.PacketHandshakingInSetProtocol", + nmsPackage + "PacketHandshakingInSetProtocol"); HANDSHAKE_PACKET = handshakePacket; HANDSHAKE_HOST = getFieldOfType(HANDSHAKE_PACKET, String.class); - checkNotNull(HANDSHAKE_HOST, "Handshake host"); + checkNotNull(HANDSHAKE_HOST, "HandshakePacket#"); // Paper 1.20.5+ uses Mojang mappings: PacketLoginInStart -> ServerboundHelloPacket - Class loginStartPacket = getClassSilently( - "net.minecraft.network.protocol.login.ServerboundHelloPacket"); - if (loginStartPacket == null) { - loginStartPacket = getClassSilently( - "net.minecraft.network.protocol.login.PacketLoginInStart"); - } - if (loginStartPacket == null) { - loginStartPacket = getClassSilently(nmsPackage + "PacketLoginInStart"); - } - if (loginStartPacket == null) { - throw NmsDiagnostics.missingClass( - "LoginStartPacket", - "net.minecraft.network.protocol.login.ServerboundHelloPacket", - "net.minecraft.network.protocol.login.PacketLoginInStart", - nmsPackage + "PacketLoginInStart"); - } + Class loginStartPacket = getRequiredClass( + "LoginStartPacket", + "net.minecraft.network.protocol.login.ServerboundHelloPacket", + "net.minecraft.network.protocol.login.PacketLoginInStart", + nmsPackage + "PacketLoginInStart"); LOGIN_START_PACKET = loginStartPacket; // Paper 1.20.5+ uses Mojang mappings: LoginListener -> ServerLoginPacketListenerImpl - Class loginListener = getClassSilently( - "net.minecraft.server.network.ServerLoginPacketListenerImpl"); - if (loginListener == null) { - loginListener = getClassSilently("net.minecraft.server.network.LoginListener"); - } - if (loginListener == null) { - loginListener = getClassSilently(nmsPackage + "LoginListener"); - } - if (loginListener == null) { - throw NmsDiagnostics.missingClass( - "LoginListener/ServerLoginPacketListenerImpl", - "net.minecraft.server.network.ServerLoginPacketListenerImpl", - "net.minecraft.server.network.LoginListener", - nmsPackage + "LoginListener"); - } + Class loginListener = getRequiredClass( + "LoginListener/ServerLoginPacketListenerImpl", + "net.minecraft.server.network.ServerLoginPacketListenerImpl", + "net.minecraft.server.network.LoginListener", + nmsPackage + "LoginListener"); LOGIN_LISTENER = loginListener; LOGIN_PROFILE = getFieldOfType(LOGIN_LISTENER, GameProfile.class); - checkNotNull(LOGIN_PROFILE, "Profile from LoginListener"); + checkNotNull(LOGIN_PROFILE, "LoginListener#"); LOGIN_DISCONNECT = getMethod(LOGIN_LISTENER, "disconnect", String.class); - checkNotNull(LOGIN_DISCONNECT, "LoginListener's disconnect method"); + checkNotNull(LOGIN_DISCONNECT, "LoginListener#disconnect(String)"); NETWORK_EXCEPTION_CAUGHT = getMethod( networkManager, @@ -258,14 +208,13 @@ public class ClassNames { if (IS_PRE_1_20_2) { - Class packetListenerClass = getClassSilently( - "net.minecraft.network.PacketListener"); - if (packetListenerClass == null) { - packetListenerClass = ReflectionUtils.getClassOrThrow( - nmsPackage + "PacketListener"); - } + Class packetListenerClass = getRequiredClass( + "PacketListener", + "net.minecraft.network.PacketListener", + nmsPackage + "PacketListener"); PACKET_LISTENER = getFieldOfType(networkManager, packetListenerClass); + checkNotNull(PACKET_LISTENER, "NetworkManager#"); } else { // We get the field by name on 1.20.2+ as there are now multiple fields of this type in network manager @@ -276,9 +225,10 @@ public class ClassNames { packetListenerField = getField(networkManager, "q"); } PACKET_LISTENER = packetListenerField; + checkNotNull(PACKET_LISTENER, + "NetworkManager#packetListener", "NetworkManager#q"); makeAccessible(PACKET_LISTENER); } - checkNotNull(PACKET_LISTENER, "Packet listener"); if (IS_POST_LOGIN_HANDLER) { makeAccessible(CALL_PLAYER_PRE_LOGIN_EVENTS); @@ -289,7 +239,9 @@ public class ClassNames { startClientVerificationMethod = getMethod(LOGIN_LISTENER, "b", GameProfile.class); } START_CLIENT_VERIFICATION = startClientVerificationMethod; - checkNotNull(START_CLIENT_VERIFICATION, "startClientVerification"); + checkNotNull(START_CLIENT_VERIFICATION, + "ServerLoginPacketListenerImpl#startClientVerification(GameProfile)", + "LoginListener#b(GameProfile)"); makeAccessible(START_CLIENT_VERIFICATION); LOGIN_HANDLER_CONSTRUCTOR = null; @@ -297,19 +249,15 @@ public class ClassNames { FIRE_LOGIN_EVENTS_GAME_PROFILE = null; } else { // Paper 1.20.5+ uses Mojang mappings: LoginListener$LoginHandler -> ServerLoginPacketListenerImpl$LoginHandler - Class loginHandler = getClassSilently( - "net.minecraft.server.network.ServerLoginPacketListenerImpl$LoginHandler"); - if (loginHandler == null) { - loginHandler = getClassSilently( - "net.minecraft.server.network.LoginListener$LoginHandler"); - } - if (loginHandler == null) { - loginHandler = ReflectionUtils.getClassOrThrow( - nmsPackage + "LoginListener$LoginHandler"); - } + Class loginHandler = getRequiredClass( + "LoginHandler", + "net.minecraft.server.network.ServerLoginPacketListenerImpl$LoginHandler", + "net.minecraft.server.network.LoginListener$LoginHandler", + nmsPackage + "LoginListener$LoginHandler"); LOGIN_HANDLER_CONSTRUCTOR = getConstructor(loginHandler, true, LOGIN_LISTENER); - checkNotNull(LOGIN_HANDLER_CONSTRUCTOR, "LoginHandler constructor"); + checkNotNull(LOGIN_HANDLER_CONSTRUCTOR, + "LoginHandler#(LoginListener)"); FIRE_LOGIN_EVENTS = getMethod(loginHandler, "fireEvents"); @@ -317,7 +265,7 @@ public class ClassNames { FIRE_LOGIN_EVENTS_GAME_PROFILE = getMethod(loginHandler, "fireEvents", GameProfile.class); checkNotNull(FIRE_LOGIN_EVENTS, FIRE_LOGIN_EVENTS_GAME_PROFILE, - "fireEvents from LoginHandler", "fireEvents(GameProfile) from LoginHandler"); + "LoginHandler#fireEvents()", "LoginHandler#fireEvents(GameProfile)"); START_CLIENT_VERIFICATION = null; } @@ -331,26 +279,25 @@ public class ClassNames { } // ProxyUtils - Class spigotConfig = ReflectionUtils.getClass("org.spigotmc.SpigotConfig"); - checkNotNull(spigotConfig, "Spigot config"); + Class spigotConfig = getRequiredClass("SpigotConfig", "org.spigotmc.SpigotConfig"); BUNGEE = getField(spigotConfig, "bungee"); - checkNotNull(BUNGEE, "Bungee field"); + checkNotNull(BUNGEE, "SpigotConfig#bungee"); Class paperConfigNew = getClassSilently( "io.papermc.paper.configuration.GlobalConfiguration"); if (paperConfigNew != null) { // 1.19 and later Method paperConfigGet = checkNotNull(getMethod(paperConfigNew, "get"), - "GlobalConfiguration get"); + "GlobalConfiguration#get()"); Field paperConfigProxies = checkNotNull(getField(paperConfigNew, "proxies"), - "Proxies field"); + "GlobalConfiguration#proxies"); Field paperConfigVelocity = checkNotNull( getField(paperConfigProxies.getType(), "velocity"), - "velocity field"); + "Proxies#velocity"); Field paperVelocityEnabled = checkNotNull( getField(paperConfigVelocity.getType(), "enabled"), - "Velocity enabled field"); + "Velocity#enabled"); PAPER_VELOCITY_SUPPORT = () -> { Object paperConfigInstance = invoke(null, paperConfigGet); Object proxiesInstance = getValue(paperConfigInstance, paperConfigProxies); @@ -385,11 +332,12 @@ public class ClassNames { "net.minecraft.network.protocol.handshake.ClientIntent", nmsPackage + "ClientIntent" ); - checkNotNull(CLIENT_INTENT, "Client intent enum"); + checkNotNull(CLIENT_INTENT, "ClientIntent"); HANDSHAKE_PACKET_CONSTRUCTOR = getConstructor(HANDSHAKE_PACKET, false, int.class, String.class, int.class, CLIENT_INTENT); - checkNotNull(HANDSHAKE_PACKET_CONSTRUCTOR, "Handshake packet constructor"); + checkNotNull(HANDSHAKE_PACKET_CONSTRUCTOR, + "HandshakePacket#(int, String, int, ClientIntent)"); // Paper 1.20.5+ Mojang mappings expose real field names; older Spigot uses obfuscated a/b/c/d. // Try Mojang name first, then obfuscated. @@ -397,7 +345,7 @@ public class ClassNames { if (protocolField == null) { protocolField = getField(HANDSHAKE_PACKET, "a"); } - checkNotNull(protocolField, "Handshake \"a\" field (protocol version, or stream codec)"); + checkNotNull(protocolField, "HandshakePacket#protocolVersion", "HandshakePacket#a"); if (protocolField.getType().isPrimitive()) { // Mojang on 1.20.5+ OR obfuscated 1.20.2-1.20.4: int field is the protocol version @@ -407,20 +355,21 @@ public class ClassNames { portField = getField(HANDSHAKE_PACKET, "c"); } HANDSHAKE_PORT = portField; + checkNotNull(HANDSHAKE_PORT, "HandshakePacket#port", "HandshakePacket#c"); } else { // Obfuscated 1.20.5: a is the stream_codec, everything is shifted HANDSHAKE_PROTOCOL = getField(HANDSHAKE_PACKET, "b"); + checkNotNull(HANDSHAKE_PROTOCOL, "HandshakePacket#b"); Field portField = getField(HANDSHAKE_PACKET, "port"); if (portField == null) { portField = getField(HANDSHAKE_PACKET, "d"); } HANDSHAKE_PORT = portField; + checkNotNull(HANDSHAKE_PORT, "HandshakePacket#port", "HandshakePacket#d"); } - checkNotNull(HANDSHAKE_PROTOCOL, "Handshake protocol"); makeAccessible(HANDSHAKE_PROTOCOL); - checkNotNull(HANDSHAKE_PORT, "Handshake port"); makeAccessible(HANDSHAKE_PORT); // Try Mojang field name first, then fall back to type-based lookup (obfuscated) @@ -429,7 +378,8 @@ public class ClassNames { intentionField = getFieldOfType(HANDSHAKE_PACKET, CLIENT_INTENT); } HANDSHAKE_INTENTION = intentionField; - checkNotNull(HANDSHAKE_INTENTION, "Handshake intention"); + checkNotNull(HANDSHAKE_INTENTION, "HandshakePacket#intention", + "HandshakePacket#"); makeAccessible(HANDSHAKE_INTENTION); } else { CLIENT_INTENT = null; @@ -444,6 +394,20 @@ public class ClassNames { } } + @SuppressWarnings("unchecked") + private static Class getRequiredClass(String accessor, String... candidates) { + for (String candidate : candidates) { + Class clazz = getClassSilently(candidate); + if (clazz != null) { + if (Constants.DEBUG_MODE) { + System.out.println("Found class: " + clazz.getName()); + } + return (Class) clazz; + } + } + throw NmsDiagnostics.missingClass(accessor, candidates); + } + private static Class getClassOrFallback(String className, String fallbackName) { Class clazz = getClassSilently(className); @@ -454,8 +418,10 @@ private static Class getClassOrFallback(String className, String fallbackName return clazz; } - // do throw an exception when both classes couldn't be found - clazz = ReflectionUtils.getClassOrThrow(fallbackName); + clazz = getClassSilently(fallbackName); + if (clazz == null) { + throw NmsDiagnostics.missingClass(className, className, fallbackName); + } if (Constants.DEBUG_MODE) { System.out.println("Found class (fallback): " + clazz.getName()); } @@ -463,11 +429,15 @@ private static Class getClassOrFallback(String className, String fallbackName return clazz; } - private static T checkNotNull(@CheckForNull T toCheck, @CheckForNull String objectName) { + private static T checkNotNull( + @CheckForNull T toCheck, + @CheckForNull String objectName, + String... candidates) { if (toCheck == null) { - // Names the accessor and the server it was looked up on, so a mapping drift is - // actionable from the log line alone instead of a bare "... cannot be null" NPE. - throw NmsDiagnostics.missingAccessor(objectName, null); + String tried = candidates.length == 0 + ? objectName + : objectName + ", " + String.join(", ", candidates); + throw NmsDiagnostics.missingAccessor(objectName, "Tried: " + tried + "."); } return toCheck; } @@ -477,12 +447,14 @@ private static T checkNotNull( @CheckForNull T toCheck, @CheckForNull T toCheck2, @CheckForNull String objectName, - @CheckForNull String objectName2 + String... candidates ) { T resolved = toCheck != null ? toCheck : toCheck2; if (resolved == null) { - throw NmsDiagnostics.missingAccessor(objectName2, - "Required because '" + objectName + "' is also missing."); + String tried = candidates.length == 0 + ? objectName + : objectName + ", " + String.join(", ", candidates); + throw NmsDiagnostics.missingAccessor(objectName, "Tried: " + tried + "."); } return resolved; } diff --git a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java index e41236476..494cc78b1 100644 --- a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java +++ b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java @@ -32,8 +32,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.File; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; @@ -43,6 +45,7 @@ import java.util.logging.Logger; import org.bukkit.Bukkit; import org.bukkit.Server; +import org.bukkit.craftbukkit.v1_21_R1.VersionedServer; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -194,6 +197,39 @@ void classNamesInitFailureStaysReadableAfterTheCauselessNoClassDefFoundError() } } + @Test + void fallbackClassFailureNamesEveryCandidateAndTheEnvironment() throws Exception { + Server previousServer = Bukkit.getServer(); + VersionedServer versionedServer = mock(VersionedServer.class); + when(versionedServer.getName()).thenReturn(SERVER_NAME); + when(versionedServer.getVersion()).thenReturn(SERVER_VERSION); + when(versionedServer.getBukkitVersion()).thenReturn(BUKKIT_VERSION); + when(versionedServer.getLogger()).thenReturn(Logger.getLogger(SERVER_NAME)); + Field serverField = Bukkit.class.getDeclaredField("server"); + serverField.setAccessible(true); + serverField.set(null, versionedServer); + try (ChildFirstLoader loader = new ChildFirstLoader()) { + assertThrows( + ExceptionInInitializerError.class, + () -> Class.forName(ClassNames.class.getName(), true, loader)); + + Class diagnostics = Class.forName( + NmsDiagnostics.class.getName(), true, loader); + Method initializationFailure = diagnostics.getDeclaredMethod("initializationFailure"); + String message = (String) initializationFailure.invoke(null); + + assertNotNull(message); + assertTrue(message.contains("net.minecraft.server.MinecraftServer"), message); + assertTrue(message.contains("net.minecraft.server.v1_21_R1.MinecraftServer"), message); + assertTrue(message.contains(SERVER_NAME), message); + assertTrue(message.contains(BUKKIT_VERSION), message); + assertTrue(message.contains(MINECRAFT_VERSION), message); + assertTrue(message.contains("java="), message); + } finally { + serverField.set(null, previousServer); + } + } + /** * Loads Connect's util classes child-first so {@code ClassNames} gets a fresh, uninitialized * copy per test, while {@code org.bukkit.*} stays parent-loaded so the mock server installed diff --git a/spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/VersionedServer.java b/spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/VersionedServer.java new file mode 100644 index 000000000..4d83c2757 --- /dev/null +++ b/spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/VersionedServer.java @@ -0,0 +1,6 @@ +package org.bukkit.craftbukkit.v1_21_R1; + +import org.bukkit.Server; + +public abstract class VersionedServer implements Server { +} diff --git a/spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/entity/CraftPlayer.java b/spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/entity/CraftPlayer.java new file mode 100644 index 000000000..f6592dd34 --- /dev/null +++ b/spigot/src/test/java/org/bukkit/craftbukkit/v1_21_R1/entity/CraftPlayer.java @@ -0,0 +1,7 @@ +package org.bukkit.craftbukkit.v1_21_R1.entity; + +public final class CraftPlayer { + public Object getProfile() { + return null; + } +} From 8c1f782de3866283cd44d30cd8d9ad31c8daaa22 Mon Sep 17 00:00:00 2001 From: "no-mistakes[bot]" Date: Tue, 28 Jul 2026 02:40:01 +0200 Subject: [PATCH 3/5] no-mistakes(review): Guarded optional velocity field drift at point of use --- .../connect/addon/data/SpigotDataHandler.java | 34 +++++++++--- .../com/minekube/connect/util/ClassNames.java | 2 +- .../addon/data/SpigotDataHandlerTest.java | 52 +++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/spigot/src/main/java/com/minekube/connect/addon/data/SpigotDataHandler.java b/spigot/src/main/java/com/minekube/connect/addon/data/SpigotDataHandler.java index ca29e0a2d..40392754f 100644 --- a/spigot/src/main/java/com/minekube/connect/addon/data/SpigotDataHandler.java +++ b/spigot/src/main/java/com/minekube/connect/addon/data/SpigotDataHandler.java @@ -37,12 +37,15 @@ import com.minekube.connect.config.ConnectConfig; import com.minekube.connect.network.netty.LocalSession.Context; import com.minekube.connect.util.ClassNames; +import com.minekube.connect.util.NmsDiagnostics; import com.minekube.connect.util.ProxyUtils; import com.minekube.connect.util.SpigotGameProfiles; import com.mojang.authlib.GameProfile; +import java.lang.reflect.Field; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.function.UnaryOperator; +import javax.annotation.Nullable; public final class SpigotDataHandler extends CommonDataHandler { private final Context sessionCtx; @@ -185,13 +188,15 @@ public boolean channelRead(Object packet) throws Exception { return false; } - if (ProxyUtils.isVelocitySupport()) { - // We securely skip the velocity plugin message login method entirely. - // Setting this field to a value other than -1 skips the velocity check in paper and - // prevents the player getting kicked due to "This server requires you to connect with Velocity." - // when invoking LoginHandler#fireEvents() below. - setValue(packetListener, ClassNames.VELOCITY_LOGIN_MESSAGE_ID, 0); - } + // We securely skip the velocity plugin message login method entirely. + // Setting this field to a value other than -1 skips the velocity check in paper and + // prevents the player getting kicked due to "This server requires you to connect with Velocity." + // when invoking LoginHandler#fireEvents() below. + setVelocityLoginMessageIdIfSupported( + packetListener, + ProxyUtils.isVelocitySupport(), + ClassNames.VELOCITY_LOGIN_MESSAGE_ID, + ClassNames.LOGIN_LISTENER); // Set the player's correct GameProfile, including signed texture properties for skins. GameProfile gameProfile = SpigotGameProfiles.fromConnectProfile(sessionCtx.getPlayer().getGameProfile()); @@ -252,6 +257,21 @@ boolean enforceBedrockIdentity() { return false; } + static void setVelocityLoginMessageIdIfSupported( + Object packetListener, + boolean velocitySupport, + @Nullable Field velocityLoginMessageId, + Class loginListener) { + if (!velocitySupport) { + return; + } + if (velocityLoginMessageId == null) { + String accessor = loginListener.getName() + "#velocityLoginMessageId"; + throw NmsDiagnostics.missingAccessor(accessor, "Tried: " + accessor + "."); + } + setValue(packetListener, velocityLoginMessageId, 0); + } + private static final Gson GSON = new Gson(); // source https://github.com/PaperMC/Velocity/blob/2586210ca67f2510eb4f91bf7567643f8a26ee7b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java#L126 diff --git a/spigot/src/main/java/com/minekube/connect/util/ClassNames.java b/spigot/src/main/java/com/minekube/connect/util/ClassNames.java index 075ef2e63..d64ae0c82 100644 --- a/spigot/src/main/java/com/minekube/connect/util/ClassNames.java +++ b/spigot/src/main/java/com/minekube/connect/util/ClassNames.java @@ -63,7 +63,7 @@ public class ClassNames { public static final Field SOCKET_ADDRESS; public static final Field HANDSHAKE_HOST; - public static final Field VELOCITY_LOGIN_MESSAGE_ID; + @Nullable public static final Field VELOCITY_LOGIN_MESSAGE_ID; public static final Field LOGIN_PROFILE; public static final Field PACKET_LISTENER; diff --git a/spigot/src/test/java/com/minekube/connect/addon/data/SpigotDataHandlerTest.java b/spigot/src/test/java/com/minekube/connect/addon/data/SpigotDataHandlerTest.java index 938f8b915..2b7a1bb3f 100644 --- a/spigot/src/test/java/com/minekube/connect/addon/data/SpigotDataHandlerTest.java +++ b/spigot/src/test/java/com/minekube/connect/addon/data/SpigotDataHandlerTest.java @@ -3,17 +3,44 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import com.minekube.connect.api.player.GameProfile; +import com.minekube.connect.util.NmsDiagnostics; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.channel.local.LocalAddress; import java.lang.reflect.Method; import java.net.InetSocketAddress; import java.util.Arrays; import java.util.UUID; +import java.util.logging.Logger; +import org.bukkit.Bukkit; +import org.bukkit.Server; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; class SpigotDataHandlerTest { + private static final String SERVER_NAME = "Paper"; + private static final String MINECRAFT_VERSION = "1.21.4"; + private static final String BUKKIT_VERSION = MINECRAFT_VERSION + "-R0.1-SNAPSHOT"; + + @BeforeAll + static void installServer() { + if (Bukkit.getServer() != null) { + return; + } + Server server = mock(Server.class); + lenient().when(server.getName()).thenReturn(SERVER_NAME); + lenient().when(server.getVersion()).thenReturn("git-Paper-196 (MC: " + MINECRAFT_VERSION + ")"); + lenient().when(server.getBukkitVersion()).thenReturn(BUKKIT_VERSION); + lenient().when(server.getLogger()).thenReturn(Logger.getLogger(SERVER_NAME)); + Bukkit.setServer(server); + } + @Test void usesSpoofedPlayerAddressWhenLocalChannelHasNoInetRemoteAddress() { String address = SpigotDataHandler.playerRemoteAddressAsString( @@ -59,4 +86,29 @@ void bungeeForwardedPropertiesExcludeIdentityEnvelopeAndNonce() { assertFalse(properties.contains("replay-nonce-a")); assertFalse(properties.contains("private-endpoint-id")); } + + @Test + void missingVelocityFieldIsIgnoredWhenVelocitySupportIsDisabled() { + assertDoesNotThrow(() -> SpigotDataHandler.setVelocityLoginMessageIdIfSupported( + new Object(), false, null, TestLoginListener.class)); + } + + @Test + void missingVelocityFieldNamesTheAccessorWhenVelocitySupportIsEnabled() { + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> SpigotDataHandler.setVelocityLoginMessageIdIfSupported( + new Object(), true, null, TestLoginListener.class)); + + String message = failure.getMessage(); + assertNotNull(message); + assertTrue(message.contains(TestLoginListener.class.getName() + "#velocityLoginMessageId"), message); + assertTrue(message.contains(SERVER_NAME), message); + assertTrue(message.contains(BUKKIT_VERSION), message); + assertTrue(message.contains(MINECRAFT_VERSION), message); + assertTrue(message.contains("java="), message); + assertTrue(message.contains("not a Java runtime incompatibility"), message); + } + + private static final class TestLoginListener { + } } From 0586f415b8fedb157e535501f0c31610f6c42dc9 Mon Sep 17 00:00:00 2001 From: "no-mistakes[bot]" Date: Tue, 28 Jul 2026 02:58:43 +0200 Subject: [PATCH 4/5] no-mistakes(document): Tighten NMS diagnostic documentation --- AGENTS.md | 40 +++++++------------ .../minekube/connect/util/NmsDiagnostics.java | 8 ++-- .../connect/util/NmsDiagnosticsTest.java | 3 +- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 56d1d1b64..1c1273fe7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,31 +129,21 @@ curl -I -L --fail https://github.com/minekube/connect-java/releases/download/` plus the JUnit platform launcher - on a 26 JDK against the module's test runtime classpath. +- Spigot NMS drift is expected when a Minecraft release renames an internal accessor: + `spigot/.../util/ClassNames.java` resolves server internals in one static initializer, + and failures are latched in the separate `NmsDiagnostics` class so they remain + available after `ClassNames` becomes erroneous. `SpigotPlatform.enable()` logs the + accessor and environment from that latch. Route new lookups through the + `NmsDiagnostics` helpers so they stay reportable; `spigot/.../util/NmsDiagnosticsTest` + guards this contract. + +## Java runtime compatibility + +- Java 26 is not a libp2p classloader incompatibility; the isolation has been verified on + Java 26. Treat a Java 26 report as an NMS compatibility signal until the + `NmsDiagnostics` line identifies the server and Minecraft version. +- CI currently stops at JDK 21 because the repository uses Gradle 8.5; the authoritative + matrix is `.github/workflows/pullrequest.yml`. ## Velocity Join Bugs diff --git a/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java b/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java index 5d27c028c..4be8d92a0 100644 --- a/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java +++ b/spigot/src/main/java/com/minekube/connect/util/NmsDiagnostics.java @@ -95,7 +95,9 @@ public static IllegalStateException missingClass( /** * Records why {@code ClassNames} failed to initialize, so the reason survives the - * causeless {@code NoClassDefFoundError}s the JVM raises on every subsequent touch. + * uninformative top-level {@code NoClassDefFoundError} the JVM raises on every subsequent + * touch. The original failure may remain in the cause chain, but callers should not need to + * inspect it. */ public static void recordInitializationFailure(@Nullable Throwable failure) { if (failure == null) { @@ -113,8 +115,8 @@ public static void recordInitializationFailure(@Nullable Throwable failure) { /** * The recorded {@code ClassNames} initialization failure, or {@code null} if it initialized - * fine. Callers use this to report something actionable instead of a causeless - * {@code NoClassDefFoundError}. + * fine. Callers use this to report something actionable instead of relying on the + * uninformative top-level {@code NoClassDefFoundError}. */ @Nullable public static String initializationFailure() { diff --git a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java index 494cc78b1..1af499a69 100644 --- a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java +++ b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java @@ -151,7 +151,8 @@ void noFailureIsReportedWhenNothingWentWrong() { /** * The end-to-end shape of the bug: a real {@code ClassNames} static-init failure must stay - * readable after the JVM has started answering with causeless {@code NoClassDefFoundError}s. + * readable after the JVM has started answering with uninformative top-level + * {@code NoClassDefFoundError}s. * *

{@code ClassNames} is loaded in a throwaway child-first loader so its initializer runs * for real. No {@code net.minecraft.*} exists on the test classpath, so it fails exactly the From 3b5ff80d0bfa3b74b02724f21ee98057e086623f Mon Sep 17 00:00:00 2001 From: "no-mistakes[bot]" Date: Tue, 28 Jul 2026 03:04:07 +0200 Subject: [PATCH 5/5] no-mistakes(document): Correct diagnostic test naming --- .../test/java/com/minekube/connect/util/NmsDiagnosticsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java index 1af499a69..b3053cc8b 100644 --- a/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java +++ b/spigot/src/test/java/com/minekube/connect/util/NmsDiagnosticsTest.java @@ -159,7 +159,7 @@ void noFailureIsReportedWhenNothingWentWrong() { * way a drifted mapping fails on a live server. */ @Test - void classNamesInitFailureStaysReadableAfterTheCauselessNoClassDefFoundError() + void classNamesInitFailureStaysReadableAfterTheUninformativeNoClassDefFoundError() throws Exception { try (ChildFirstLoader loader = new ChildFirstLoader()) { // First touch: the JVM still carries the reason.