From 39012776e3cc4c3a2bc338680be707c24875637e Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 10:42:28 +0200 Subject: [PATCH 01/10] feat(protocol): add CreatePlayer.joinMode + locale, bump RuntimeProtocol.VERSION to 10 Goal: give the wire protocol the vocabulary the S6 FULL_LOGIN driver (LK-18) needs before the nms/agent/framework layers can route by join mode. How this slots into S6/LK-18: - New top-level JoinMode enum (FULL_LOGIN | LEGACY_SPAWN), serialized by name. - CreatePlayer.Command gains joinMode + @Nullable locale (LK-12); uuid becomes @Nullable and is validated null under FULL_LOGIN (server derives the offline UUID from the name) and required under LEGACY_SPAWN. locale must be non-blank when present. - RuntimeProtocol.VERSION 9->10 so framework/agent handshake enforces the matched pair. - New AgentErrorCode.PLAYER_JOIN_DENIED / PLAYER_JOIN_TIMEOUT for the agent to surface login denials and timeouts as typed errors. - Serialization + validation tests extended for the joinMode/locale/uuid-null rules; the reflective everyCommand_shouldRejectBlankRequestId guard picks up the new fields. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../lightkeeper/protocol/AgentErrorCode.java | 2 + .../lightkeeper/protocol/CreatePlayer.java | 32 +++++++-- .../lightkeeper/protocol/JoinMode.java | 30 +++++++++ .../AgentCommandSerializationTest.java | 50 ++++++++++++-- .../protocol/AgentCommandValidationTest.java | 65 +++++++++++++++++++ .../lightkeeper/runtime/RuntimeProtocol.java | 2 +- 6 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/JoinMode.java diff --git a/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/AgentErrorCode.java b/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/AgentErrorCode.java index aaf8adce..55663f8c 100644 --- a/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/AgentErrorCode.java +++ b/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/AgentErrorCode.java @@ -22,6 +22,8 @@ public enum AgentErrorCode INVALID_ARGUMENT, TIMEOUT, INTERRUPTED, + PLAYER_JOIN_DENIED, + PLAYER_JOIN_TIMEOUT, UNKNOWN; private static final Map BY_WIRE_CODE = Arrays.stream(values()) diff --git a/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/CreatePlayer.java b/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/CreatePlayer.java index b0637fcd..3e465f97 100644 --- a/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/CreatePlayer.java +++ b/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/CreatePlayer.java @@ -21,7 +21,8 @@ private CreatePlayer() * @param name * Display name of the synthetic player. * @param uuid - * Unique identifier assigned to the player. + * Unique identifier assigned to the player under {@link JoinMode#LEGACY_SPAWN}; must be {@code null} + * under {@link JoinMode#FULL_LOGIN}, where the server derives the offline UUID from {@code name}. * @param worldName * Name of the world in which the player is spawned. * @param x @@ -34,27 +35,50 @@ private CreatePlayer() * Starting health value, or {@code null} to use the server default. * @param permissionsCsv * Comma-separated list of permission nodes to grant, or {@code null} for none. + * @param joinMode + * How the player joins the server: the full login pipeline or the internal legacy spawn. + * @param locale + * Client locale (e.g. {@code "en_us"}) sent during the configuration phase under + * {@link JoinMode#FULL_LOGIN}, or {@code null} for the server default. Ignored under + * {@link JoinMode#LEGACY_SPAWN}. */ public record Command( String requestId, String name, - UUID uuid, + @Nullable UUID uuid, String worldName, @Nullable Double x, @Nullable Double y, @Nullable Double z, @Nullable Double health, - @Nullable String permissionsCsv + @Nullable String permissionsCsv, + JoinMode joinMode, + @Nullable String locale ) implements IAgentCommand { public Command { ProtocolPreconditions.requireNonBlank(requestId, "requestId"); - ProtocolPreconditions.requireNonNull(uuid, "uuid"); + ProtocolPreconditions.requireNonNull(joinMode, "joinMode"); if (name == null || name.isBlank()) throw new IllegalArgumentException("'name' must not be blank."); if (worldName == null || worldName.isBlank()) throw new IllegalArgumentException("'worldName' must not be blank."); + + switch (joinMode) + { + case FULL_LOGIN -> + { + if (uuid != null) + throw new IllegalArgumentException( + "'uuid' must be null under FULL_LOGIN: the server derives the offline UUID from the " + + "player name. Use LEGACY_SPAWN to supply an explicit UUID."); + } + case LEGACY_SPAWN -> ProtocolPreconditions.requireNonNull(uuid, "uuid"); + } + + if (locale != null && locale.isBlank()) + throw new IllegalArgumentException("'locale' must not be blank when present."); } @Override diff --git a/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/JoinMode.java b/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/JoinMode.java new file mode 100644 index 00000000..f35af83f --- /dev/null +++ b/lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/JoinMode.java @@ -0,0 +1,30 @@ +package nl.pim16aap2.lightkeeper.protocol; + +/** + * Selects how a synthetic player is placed onto the server by {@link CreatePlayer.Command}. + * + *

Serialized on the wire by {@linkplain Enum#name() name}, so the constant names are part of the protocol + * contract and must not be renamed without a {@code RuntimeProtocol.VERSION} bump. + */ +public enum JoinMode +{ + /** + * Drive the full vanilla login pipeline over a real loopback TCP connection. + * + *

The join fires the real {@code AsyncPlayerPreLoginEvent}, {@code PlayerLoginEvent}, and + * {@code PlayerJoinEvent}, traverses the 1.20.2+ configuration phase, and lets the server derive the + * offline UUID from the player name. Because it is a genuine login, it can be denied by whitelist, ban, + * or a full server; such denials are surfaced as typed errors. The {@code uuid} field of + * {@link CreatePlayer.Command} must be {@code null} under this mode. + */ + FULL_LOGIN, + + /** + * Spawn the synthetic player directly through the internal, reflective spawn path. + * + *

This is the historical fast path: it fires {@code PlayerJoinEvent} but skips the pre-login, login, + * and configuration phases. The caller-supplied {@code uuid} is honored. Kept as an uncontracted internal + * shortcut for LightKeeper's own fast tests. + */ + LEGACY_SPAWN +} diff --git a/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandSerializationTest.java b/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandSerializationTest.java index 35f5d1ce..14baee73 100644 --- a/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandSerializationTest.java +++ b/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandSerializationTest.java @@ -74,11 +74,11 @@ void serialize_teleportPlayerCommand_roundTrips() throws Exception } // ----------------------------------------------------------------------- - // Round-trip: CreatePlayer.Command with null optional fields + // Round-trip: CreatePlayer.Command (LEGACY_SPAWN) with null optional fields // ----------------------------------------------------------------------- @Test - void serialize_createPlayerCommand_withNullOptionals_roundTrips() throws Exception + void serialize_createPlayerCommand_legacySpawnWithNullOptionals_roundTrips() throws Exception { // setup final ObjectMapper mapper = AgentProtocolMapper.create(); @@ -86,7 +86,8 @@ void serialize_createPlayerCommand_withNullOptionals_roundTrips() throws Excepti final CreatePlayer.Command original = new CreatePlayer.Command( "req-3", "Alice", uuid, "world", null, null, null, - null, null + null, null, + JoinMode.LEGACY_SPAWN, null ); // execute @@ -106,14 +107,16 @@ void serialize_createPlayerCommand_withNullOptionals_roundTrips() throws Excepti assertThat(result.z()).isNull(); assertThat(result.health()).isNull(); assertThat(result.permissionsCsv()).isNull(); + assertThat(result.joinMode()).isEqualTo(JoinMode.LEGACY_SPAWN); + assertThat(result.locale()).isNull(); } // ----------------------------------------------------------------------- - // Round-trip: CreatePlayer.Command with all fields present + // Round-trip: CreatePlayer.Command (LEGACY_SPAWN) with all fields present // ----------------------------------------------------------------------- @Test - void serialize_createPlayerCommand_withAllFields_roundTrips() throws Exception + void serialize_createPlayerCommand_legacySpawnWithAllFields_roundTrips() throws Exception { // setup final ObjectMapper mapper = AgentProtocolMapper.create(); @@ -121,7 +124,8 @@ void serialize_createPlayerCommand_withAllFields_roundTrips() throws Exception final CreatePlayer.Command original = new CreatePlayer.Command( "req-4", "Bob", uuid, "nether", 10.0, 64.0, -5.0, - 20.0, "minecraft.command.tp,some.other.node" + 20.0, "minecraft.command.tp,some.other.node", + JoinMode.LEGACY_SPAWN, null ); // execute @@ -141,6 +145,40 @@ void serialize_createPlayerCommand_withAllFields_roundTrips() throws Exception assertThat(result.z()).isEqualTo(-5.0); assertThat(result.health()).isEqualTo(20.0); assertThat(result.permissionsCsv()).isEqualTo("minecraft.command.tp,some.other.node"); + assertThat(result.joinMode()).isEqualTo(JoinMode.LEGACY_SPAWN); + assertThat(result.locale()).isNull(); + } + + // ----------------------------------------------------------------------- + // Round-trip: CreatePlayer.Command (FULL_LOGIN) with null uuid and a locale + // ----------------------------------------------------------------------- + + @Test + void serialize_createPlayerCommand_fullLoginWithLocale_roundTrips() throws Exception + { + // setup + final ObjectMapper mapper = AgentProtocolMapper.create(); + final CreatePlayer.Command original = new CreatePlayer.Command( + "req-4b", "Carol", null, "world", + null, null, null, + null, null, + JoinMode.FULL_LOGIN, "en_us" + ); + + // execute + final String json = mapper.writeValueAsString(original); + @SuppressWarnings("rawtypes") + final IAgentCommand deserialized = mapper.readValue(json, IAgentCommand.class); + + // verify + assertThat(deserialized).isInstanceOf(CreatePlayer.Command.class); + final CreatePlayer.Command result = (CreatePlayer.Command) deserialized; + assertThat(result.requestId()).isEqualTo("req-4b"); + assertThat(result.name()).isEqualTo("Carol"); + assertThat(result.uuid()).isNull(); + assertThat(result.worldName()).isEqualTo("world"); + assertThat(result.joinMode()).isEqualTo(JoinMode.FULL_LOGIN); + assertThat(result.locale()).isEqualTo("en_us"); } // ----------------------------------------------------------------------- diff --git a/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandValidationTest.java b/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandValidationTest.java index 4461e172..e0d4faf0 100644 --- a/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandValidationTest.java +++ b/lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/AgentCommandValidationTest.java @@ -329,6 +329,71 @@ void entityDataConstructor_shouldRejectBlankTypeKey() .hasMessageContaining("typeKey"); } + // ----------------------------------------------------------------------- + // CreatePlayer.Command validation + // ----------------------------------------------------------------------- + + @Test + void createPlayerCommand_shouldRejectNonNullUuidUnderFullLogin() + { + // execute + verify + assertThatThrownBy(() -> new CreatePlayer.Command( + "request-1", "Alice", UUID.randomUUID(), "world", + null, null, null, null, null, JoinMode.FULL_LOGIN, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("uuid") + .hasMessageContaining("FULL_LOGIN"); + } + + @Test + @SuppressWarnings("NullAway") // Intentionally crosses the non-null API boundary to verify fail-fast validation. + void createPlayerCommand_shouldRejectNullUuidUnderLegacySpawn() + { + // execute + verify + assertThatThrownBy(() -> new CreatePlayer.Command( + "request-1", "Alice", null, "world", + null, null, null, null, null, JoinMode.LEGACY_SPAWN, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("uuid"); + } + + @Test + void createPlayerCommand_shouldAllowNullUuidUnderFullLogin() + { + // setup + execute + final CreatePlayer.Command command = new CreatePlayer.Command( + "request-1", "Alice", null, "world", + null, null, null, null, null, JoinMode.FULL_LOGIN, "en_us"); + + // verify + assertThat(command.uuid()).isNull(); + assertThat(command.joinMode()).isEqualTo(JoinMode.FULL_LOGIN); + assertThat(command.locale()).isEqualTo("en_us"); + } + + @Test + @SuppressWarnings("NullAway") // Intentionally crosses the non-null API boundary to verify fail-fast validation. + void createPlayerCommand_shouldRejectNullJoinMode() + { + // execute + verify + assertThatThrownBy(() -> new CreatePlayer.Command( + "request-1", "Alice", UUID.randomUUID(), "world", + null, null, null, null, null, null, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("joinMode"); + } + + @Test + void createPlayerCommand_shouldRejectBlankLocaleWhenPresent() + { + // execute + verify + assertThatThrownBy(() -> new CreatePlayer.Command( + "request-1", "Alice", null, "world", + null, null, null, null, null, JoinMode.FULL_LOGIN, " ")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("locale"); + } + // ----------------------------------------------------------------------- // IProtocolValue leaf validation // ----------------------------------------------------------------------- diff --git a/lightkeeper-runtime-core/src/main/java/nl/pim16aap2/lightkeeper/runtime/RuntimeProtocol.java b/lightkeeper-runtime-core/src/main/java/nl/pim16aap2/lightkeeper/runtime/RuntimeProtocol.java index 87436dc0..542e48eb 100644 --- a/lightkeeper-runtime-core/src/main/java/nl/pim16aap2/lightkeeper/runtime/RuntimeProtocol.java +++ b/lightkeeper-runtime-core/src/main/java/nl/pim16aap2/lightkeeper/runtime/RuntimeProtocol.java @@ -20,7 +20,7 @@ public final class RuntimeProtocol * in a backward-incompatible way. Both the framework and the agent must agree on this value; * a mismatch causes an {@code HANDSHAKE} failure. */ - public static final int VERSION = 9; + public static final int VERSION = 10; /** * Minecraft server version supported by this LightKeeper build. From 5d4ef426cdd48d99899bcb972633f8ae1a65f9e2 Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 11:11:47 +0200 Subject: [PATCH 02/10] feat(nms): add IBotLoginDriver + reflective BotLoginPipelineDriver (proxy over client listeners) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Goal: implement the S6/LK-18 FULL_LOGIN mechanism — drive the real vanilla login pipeline over loopback TCP so bots fire AsyncPreLogin/Login/Join and get a server-derived offline UUID, behind a swappable interface. How this slots into S6/LK-18: - nms-api: new IBotLoginDriver seam (Bukkit/JDK-typed only, so an MCProtocolLib transport could replace it) + BotLoginRequest / IBotLoginOutcome (Joined | Denied | TimedOut) / BotJoinPhase. IBotPlayerNmsAdapter gains loginDriver(). - nms-v1_21_R7: BotLoginReflection resolves the whole login surface fully reflectively with per-distribution fallbacks (Connection/NetworkManager, ConnectionProtocol/ EnumProtocol, obfuscated login+game packet classes vs shared common/configuration names, EventLoopGroupHolder, LoginProtocols/ConfigurationProtocols/GameProtocols resolved structurally because Spigot obfuscates the static field names, game ProtocolInfo bound via RegistryFriendlyByteBuf.decorator + a synthesized GameProtocols.Context). BotLoginSession is the JDK Proxy's InvocationHandler, dispatching on the packet class (never the method name, since Spigot obfuscates every method to 'a') and driving the login -> configuration -> play phase machine (compression, login-ack, known-packs echo, ClientInformation/locale LK-12, code-of-conduct, config-finish, keepalive/ping, teleport-ack). Fail-loud on unhandled login/config packets (naming the class); warn-once in play; kicks surfaced as Denied. - Legacy placeNewPlayer spawn path is untouched and remains the LEGACY_SPAWN route. - BotLoginPipelineDriver resolves the surface once and runs an independent session per login; wired lazily from the adapter so legacy-spawn-only usage never pays for it. - Unit tests cover the pure, server-decoupled helpers (structural method/enum resolution, offline-UUID scheme, request validation); the live pipeline is covered by the ITs. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../lightkeeper/nms/api/BotJoinPhase.java | 25 + .../lightkeeper/nms/api/BotLoginRequest.java | 43 + .../lightkeeper/nms/api/IBotLoginDriver.java | 26 + .../lightkeeper/nms/api/IBotLoginOutcome.java | 58 ++ .../nms/api/IBotPlayerNmsAdapter.java | 13 +- .../nms/api/BotLoginRequestTest.java | 63 ++ .../nms/v121r7/BotLoginPipelineDriver.java | 35 + .../nms/v121r7/BotLoginReflection.java | 837 ++++++++++++++++++ .../nms/v121r7/BotLoginSession.java | 256 ++++++ .../v121r7/BotPlayerNmsAdapterV1_21_R7.java | 31 + .../nms/v121r7/NmsReflectionUtils.java | 112 +++ .../v121r7/BotLoginReflectionHelpersTest.java | 106 +++ 12 files changed, 1604 insertions(+), 1 deletion(-) create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequest.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginDriver.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-api/src/test/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequestTest.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginPipelineDriver.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java create mode 100644 lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java new file mode 100644 index 00000000..02a4aefd --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java @@ -0,0 +1,25 @@ +package nl.pim16aap2.lightkeeper.nms.api; + +/** + * Protocol phase a {@link IBotLoginDriver} was in when a full-login join was denied. + * + *

Surfaced on {@link BotLoginOutcome.Denied} so callers can distinguish, for example, a whitelist/ban + * kick during login from a kick during the configuration or play phase. + */ +public enum BotJoinPhase +{ + /** + * The login phase (handshake, hello, compression, login success). + */ + LOGIN, + + /** + * The 1.20.2+ configuration phase (known packs, registries, configuration finish). + */ + CONFIGURATION, + + /** + * The play phase (after the player has been placed onto the server). + */ + PLAY +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequest.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequest.java new file mode 100644 index 00000000..ba31232f --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequest.java @@ -0,0 +1,43 @@ +package nl.pim16aap2.lightkeeper.nms.api; + +import org.jspecify.annotations.Nullable; + +import java.time.Duration; +import java.util.Objects; + +/** + * Immutable inputs for a single full-login join driven by {@link IBotLoginDriver#login(BotLoginRequest)}. + * + * @param name + * Player name to log in as. The server derives the offline UUID from this name. + * @param port + * TCP port of the loopback server to connect to. + * @param locale + * Client locale (e.g. {@code "en_us"}) to send during the configuration phase, or {@code null} for the + * server default. + * @param timeout + * Maximum time to wait for the login pipeline to reach the play phase before giving up. + */ +public record BotLoginRequest( + String name, + int port, + @Nullable String locale, + Duration timeout) +{ + /** + * Validates the request inputs. + */ + public BotLoginRequest + { + Objects.requireNonNull(name, "name"); + if (name.isBlank()) + throw new IllegalArgumentException("name must not be blank."); + if (port <= 0 || port > 65_535) + throw new IllegalArgumentException("port must be in (0, 65535] but was " + port + "."); + if (locale != null && locale.isBlank()) + throw new IllegalArgumentException("locale must not be blank when present."); + Objects.requireNonNull(timeout, "timeout"); + if (timeout.isNegative() || timeout.isZero()) + throw new IllegalArgumentException("timeout must be positive but was " + timeout + "."); + } +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginDriver.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginDriver.java new file mode 100644 index 00000000..594fdf74 --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginDriver.java @@ -0,0 +1,26 @@ +package nl.pim16aap2.lightkeeper.nms.api; + +/** + * Drives a synthetic player through the full vanilla login pipeline over a real loopback TCP connection. + * + *

This is the swappable transport seam (LK-18): the reflective in-agent implementation could be replaced + * by an out-of-process protocol client (e.g. MCProtocolLib) without changing the agent, because the interface + * is expressed purely in Bukkit/JDK types. Implementations open a TCP connection to the server's own port, + * traverse the handshake, login, and configuration phases, and block until the play phase is reached or the + * connection is denied/times out. + */ +public interface IBotLoginDriver +{ + /** + * Logs a synthetic player in through the full pipeline and blocks until a terminal outcome. + * + *

Must not be called on the server's main thread: the login runs its own I/O and the server fires the + * asynchronous pre-login and (main-thread) join events while this method waits. + * + * @param request + * Login inputs (name, port, locale, timeout). + * @return + * The terminal outcome: joined, denied (with kick reason), or timed out. + */ + IBotLoginOutcome login(BotLoginRequest request); +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java new file mode 100644 index 00000000..e94d9c5c --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java @@ -0,0 +1,58 @@ +package nl.pim16aap2.lightkeeper.nms.api; + +import java.util.Objects; + +/** + * Terminal result of a full-login join attempt. + * + *

Exactly one of the permitted subtypes is produced: {@link Joined} when the login pipeline reached the + * play phase, {@link Denied} when the server kicked the connection (whitelist/ban/full/plugin denial), or + * {@link TimedOut} when the pipeline did not complete in time. + */ +public sealed interface IBotLoginOutcome + permits IBotLoginOutcome.Joined, IBotLoginOutcome.Denied, IBotLoginOutcome.TimedOut +{ + /** + * The login pipeline reached the play phase; the server has accepted the player. + * + * @param name + * Name the player logged in with. + */ + record Joined(String name) implements IBotLoginOutcome + { + /** + * Validates the outcome inputs. + */ + public Joined + { + Objects.requireNonNull(name, "name"); + } + } + + /** + * The server refused the connection with a kick reason. + * + * @param phase + * Protocol phase the connection was in when it was denied. + * @param reason + * Human-readable kick reason text extracted from the server's disconnect packet. + */ + record Denied(BotJoinPhase phase, String reason) implements IBotLoginOutcome + { + /** + * Validates the outcome inputs. + */ + public Denied + { + Objects.requireNonNull(phase, "phase"); + Objects.requireNonNull(reason, "reason"); + } + } + + /** + * The login pipeline did not reach the play phase within the requested timeout. + */ + record TimedOut() implements IBotLoginOutcome + { + } +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotPlayerNmsAdapter.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotPlayerNmsAdapter.java index d2faa91c..eaa89374 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotPlayerNmsAdapter.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotPlayerNmsAdapter.java @@ -13,7 +13,7 @@ public interface IBotPlayerNmsAdapter { /** - * Spawns and registers a synthetic player. + * Spawns and registers a synthetic player through the internal legacy spawn path. * * @param uuid * The player UUID. @@ -27,6 +27,17 @@ public interface IBotPlayerNmsAdapter */ Player spawnPlayer(UUID uuid, String name, World world, Location spawnLocation); + /** + * Returns the full-login driver for this adapter. + * + *

Used for {@code FULL_LOGIN} joins; the legacy spawn path above is used for {@code LEGACY_SPAWN}. + * Implementations may resolve the (larger) login reflection surface lazily, so callers that only ever use + * the legacy path never pay for it. + * + * @return The login driver. + */ + IBotLoginDriver loginDriver(); + /** * Removes a synthetic player from the server. * diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/test/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequestTest.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/test/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequestTest.java new file mode 100644 index 00000000..34375537 --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/test/java/nl/pim16aap2/lightkeeper/nms/api/BotLoginRequestTest.java @@ -0,0 +1,63 @@ +package nl.pim16aap2.lightkeeper.nms.api; + +import org.junit.jupiter.api.Test; + +import java.time.Duration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Validation tests for {@link BotLoginRequest}. + */ +class BotLoginRequestTest +{ + @Test + void constructor_shouldAcceptValidInputsWithNullLocale() + { + // execute + final BotLoginRequest request = new BotLoginRequest("Bot", 25_565, null, Duration.ofSeconds(30)); + + // verify + assertThat(request.name()).isEqualTo("Bot"); + assertThat(request.port()).isEqualTo(25_565); + assertThat(request.locale()).isNull(); + assertThat(request.timeout()).isEqualTo(Duration.ofSeconds(30)); + } + + @Test + void constructor_shouldRejectBlankName() + { + // execute + verify + assertThatThrownBy(() -> new BotLoginRequest(" ", 25_565, null, Duration.ofSeconds(30))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("name"); + } + + @Test + void constructor_shouldRejectPortOutOfRange() + { + // execute + verify + assertThatThrownBy(() -> new BotLoginRequest("Bot", 70_000, null, Duration.ofSeconds(30))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("port"); + } + + @Test + void constructor_shouldRejectBlankLocaleWhenPresent() + { + // execute + verify + assertThatThrownBy(() -> new BotLoginRequest("Bot", 25_565, " ", Duration.ofSeconds(30))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("locale"); + } + + @Test + void constructor_shouldRejectNonPositiveTimeout() + { + // execute + verify + assertThatThrownBy(() -> new BotLoginRequest("Bot", 25_565, null, Duration.ZERO)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("timeout"); + } +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginPipelineDriver.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginPipelineDriver.java new file mode 100644 index 00000000..b439afc8 --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginPipelineDriver.java @@ -0,0 +1,35 @@ +package nl.pim16aap2.lightkeeper.nms.v121r7; + +import nl.pim16aap2.lightkeeper.nms.api.BotLoginRequest; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginDriver; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginOutcome; + +import java.util.Objects; + +/** + * Paper/Spigot 1.21.11 full-login driver: drives the vanilla login pipeline over a real loopback TCP + * connection using a fully reflective, per-distribution surface (see {@link BotLoginReflection}). + * + *

The (larger) login reflection surface is resolved once in the constructor; each {@link #login} runs an + * independent {@link BotLoginSession}. Because it references zero server classes at compile time, one compiled + * artifact drives both the Mojang-mapped Paper jar and the obfuscated Spigot jar. + */ +final class BotLoginPipelineDriver implements IBotLoginDriver +{ + private final BotLoginReflection reflection; + + /** + * Resolves the full-login reflection surface for the running server. + */ + BotLoginPipelineDriver() + { + this.reflection = new BotLoginReflection(); + } + + @Override + public IBotLoginOutcome login(BotLoginRequest request) + { + Objects.requireNonNull(request, "request"); + return new BotLoginSession(reflection).run(request); + } +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java new file mode 100644 index 00000000..0e3d8115 --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java @@ -0,0 +1,837 @@ +package nl.pim16aap2.lightkeeper.nms.v121r7; + +import nl.pim16aap2.lightkeeper.nms.api.BotJoinPhase; +import org.bukkit.Bukkit; +import org.jspecify.annotations.Nullable; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Proxy; +import java.lang.reflect.RecordComponent; +import java.lang.reflect.Type; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; + +/** + * Resolves and holds the (fully reflective) Paper/Spigot 1.21.11 surface used by {@link BotLoginPipelineDriver} + * to drive the vanilla login pipeline over a real loopback TCP connection. + * + *

Nothing here references a Minecraft/Netty type at compile time: every class, method, field, and enum + * constant is resolved through {@link NmsReflectionUtils} with per-distribution name fallbacks. Paper ships + * Mojang-mapped names; Spigot obfuscates the {@code Connection} class ({@code NetworkManager}), the + * {@code ConnectionProtocol} enum ({@code EnumProtocol}), all member names, and the login/game/handshake + * packet classes (legacy {@code PacketLoginOut*}/{@code PacketPlayOut*} names), while keeping Mojang names for + * the {@code common}/{@code configuration} packages and the protocol-holder classes. Members whose names are + * obfuscated are therefore resolved structurally (by signature or generic type), never by name. + */ +final class BotLoginReflection +{ + private static final System.Logger LOG = System.getLogger(BotLoginReflection.class.getName()); + + /** + * How the session should react to a received clientbound packet, resolved from its class via + * {@link #classify(Class)}. + */ + enum PacketKind + { + /** Login compression threshold. */ COMPRESSION, + /** Login success. */ LOGIN_FINISHED, + /** Configuration known-packs request. */ KNOWN_PACKS, + /** Configuration code-of-conduct. */ CODE_OF_CONDUCT, + /** Configuration finished. */ FINISH_CONFIGURATION, + /** Common keep-alive. */ KEEP_ALIVE, + /** Common ping. */ PING, + /** Play teleport/position. */ TELEPORT, + /** Play login (join confirmation). */ GAME_LOGIN, + /** Login-phase disconnect. */ LOGIN_DISCONNECT, + /** Common disconnect. */ DISCONNECT, + /** Known informational packet requiring no response. */ CONSUME, + /** Not recognized (fail-loud in login/config, warn-once in play). */ UNKNOWN + } + + private final ClassLoader serverClassLoader; + + // ---- Interfaces the client-listener Proxy implements + core protocol types. ---- + private final Class loginListenerInterface; + private final Class configurationListenerInterface; + private final Class gameListenerInterface; + private final Class packetClass; + private final Class packetFlowClass; + private final Class connectionProtocolClass; + + // ---- Enum constants the Proxy answers with for the non-packet listener methods. ---- + private final Object clientboundFlow; + private final Object protocolLogin; + private final Object protocolConfiguration; + private final Object protocolPlay; + + // ---- Clientbound packet classes the session dispatches on. ---- + private final Class loginCompressionClass; + private final Class loginFinishedClass; + private final Class loginDisconnectClass; + private final Class selectKnownPacksClass; + private final Class finishConfigurationClass; + private final Class codeOfConductClass; + private final Class keepAliveClass; + private final Class pingClass; + private final Class disconnectClass; + private final Class playerPositionClass; + private final Class gameLoginClass; + + /** + * Clientbound packets that are expected in the login/configuration phase and require no response; kept + * explicit so a genuinely new packet (protocol drift) is not in the set and triggers the fail-loud policy. + */ + private final Set> consumeClasses; + + // ---- Resolved handles used by the operation methods. ---- + private final Class connectionClass; + private final Object eventLoopGroupHolder; + private final Method connectToServerMethod; + private final Method initiateLoginConnectionMethod; + private final Method sendMethod; + private final Method setupInboundProtocolMethod; + private final Method setupOutboundProtocolMethod; + private final Method setupCompressionMethod; + private final Field channelField; + private final Method channelCloseMethod; + + private final Object configurationClientboundInfo; + private final Object configurationServerboundInfo; + private final Object gameClientboundInfo; + private final Object gameServerboundInfo; + + private final Constructor helloPacketConstructor; + private final Object loginAcknowledgedPacket; + private final Object finishConfigurationPacket; + private final Object acceptCodeOfConductPacket; + private final Constructor selectKnownPacksConstructor; + private final Constructor keepAlivePacketConstructor; + private final Constructor pongPacketConstructor; + private final Constructor acceptTeleportationConstructor; + private final Constructor clientInformationPacketConstructor; + private final Class clientInformationClass; + private final Method clientInformationCreateDefaultMethod; + + private final Map accessorCache = new ConcurrentHashMap<>(); + + BotLoginReflection() + { + try + { + serverClassLoader = Bukkit.getServer().getClass().getClassLoader(); + final Object minecraftServer = resolveMinecraftServer(); + + packetClass = NmsReflectionUtils.resolveClass("net.minecraft.network.protocol.Packet", serverClassLoader); + packetFlowClass = NmsReflectionUtils.resolveFirstClass( + serverClassLoader, + "net.minecraft.network.protocol.PacketFlow", + "net.minecraft.network.protocol.EnumProtocolDirection"); + connectionProtocolClass = NmsReflectionUtils.resolveFirstClass( + serverClassLoader, + "net.minecraft.network.ConnectionProtocol", + "net.minecraft.network.EnumProtocol"); + connectionClass = NmsReflectionUtils.resolveFirstClass( + serverClassLoader, + "net.minecraft.network.Connection", + "net.minecraft.network.NetworkManager"); + final Class packetListenerClass = + NmsReflectionUtils.resolveClass("net.minecraft.network.PacketListener", serverClassLoader); + + clientboundFlow = NmsReflectionUtils.resolveEnumConstant(packetFlowClass, "CLIENTBOUND"); + final Object serverboundFlow = NmsReflectionUtils.resolveEnumConstant(packetFlowClass, "SERVERBOUND"); + protocolLogin = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "LOGIN"); + protocolConfiguration = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "CONFIGURATION"); + protocolPlay = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "PLAY"); + + loginListenerInterface = NmsReflectionUtils.resolveFirstClass( + serverClassLoader, + "net.minecraft.network.protocol.login.ClientLoginPacketListener", + "net.minecraft.network.protocol.login.PacketLoginOutListener"); + configurationListenerInterface = NmsReflectionUtils.resolveClass( + "net.minecraft.network.protocol.configuration.ClientConfigurationPacketListener", serverClassLoader); + gameListenerInterface = NmsReflectionUtils.resolveFirstClass( + serverClassLoader, + "net.minecraft.network.protocol.game.ClientGamePacketListener", + "net.minecraft.network.protocol.game.PacketListenerPlayOut"); + + loginCompressionClass = resolveFirst( + "net.minecraft.network.protocol.login.ClientboundLoginCompressionPacket", + "net.minecraft.network.protocol.login.PacketLoginOutSetCompression"); + loginFinishedClass = resolveFirst( + "net.minecraft.network.protocol.login.ClientboundLoginFinishedPacket", + "net.minecraft.network.protocol.login.PacketLoginOutSuccess"); + loginDisconnectClass = resolveFirst( + "net.minecraft.network.protocol.login.ClientboundLoginDisconnectPacket", + "net.minecraft.network.protocol.login.PacketLoginOutDisconnect"); + selectKnownPacksClass = resolveFirst( + "net.minecraft.network.protocol.configuration.ClientboundSelectKnownPacks"); + finishConfigurationClass = resolveFirst( + "net.minecraft.network.protocol.configuration.ClientboundFinishConfigurationPacket"); + codeOfConductClass = resolveFirst( + "net.minecraft.network.protocol.configuration.ClientboundCodeOfConductPacket"); + keepAliveClass = resolveFirst("net.minecraft.network.protocol.common.ClientboundKeepAlivePacket"); + pingClass = resolveFirst("net.minecraft.network.protocol.common.ClientboundPingPacket"); + disconnectClass = resolveFirst("net.minecraft.network.protocol.common.ClientboundDisconnectPacket"); + playerPositionClass = resolveFirst( + "net.minecraft.network.protocol.game.ClientboundPlayerPositionPacket", + "net.minecraft.network.protocol.game.PacketPlayOutPosition"); + gameLoginClass = resolveFirst( + "net.minecraft.network.protocol.game.ClientboundLoginPacket", + "net.minecraft.network.protocol.game.PacketPlayOutLogin"); + + consumeClasses = resolveConsumeClasses(); + + // Event loop group + connection factory (real TCP loopback client). + final Class eventLoopGroupHolderClass = NmsReflectionUtils.resolveClass( + "net.minecraft.server.network.EventLoopGroupHolder", serverClassLoader); + final Method remoteMethod = NmsReflectionUtils.resolveStaticFactoryMethod( + eventLoopGroupHolderClass, eventLoopGroupHolderClass, boolean.class); + eventLoopGroupHolder = Objects.requireNonNull( + remoteMethod.invoke(null, false), "EventLoopGroupHolder.remote(false) returned null."); + connectToServerMethod = resolveConnectToServer(connectionClass, eventLoopGroupHolderClass); + initiateLoginConnectionMethod = NmsReflectionUtils.findPublicMethod( + connectionClass, void.class, String.class, int.class, loginListenerInterface); + sendMethod = NmsReflectionUtils.findPublicMethod(connectionClass, void.class, packetClass); + setupInboundProtocolMethod = NmsReflectionUtils.findPublicMethod( + connectionClass, void.class, + NmsReflectionUtils.resolveClass("net.minecraft.network.ProtocolInfo", serverClassLoader), + packetListenerClass); + setupOutboundProtocolMethod = NmsReflectionUtils.findPublicMethod( + connectionClass, void.class, + NmsReflectionUtils.resolveClass("net.minecraft.network.ProtocolInfo", serverClassLoader)); + setupCompressionMethod = NmsReflectionUtils.findPublicMethod( + connectionClass, void.class, int.class, boolean.class); + final Class channelClass = + NmsReflectionUtils.resolveClass("io.netty.channel.Channel", serverClassLoader); + channelField = NmsReflectionUtils.resolveFieldByNameOrAcceptedType( + connectionClass, "channel", channelClass); + channelCloseMethod = channelClass.getMethod("close"); + + // Bound protocol infos for the configuration and play phases. + final ProtocolInfos protocolInfos = resolveProtocolInfos(minecraftServer); + configurationClientboundInfo = protocolInfos.configurationClientbound(); + configurationServerboundInfo = protocolInfos.configurationServerbound(); + gameClientboundInfo = protocolInfos.gameClientbound(); + gameServerboundInfo = protocolInfos.gameServerbound(); + + // Serverbound packet factories. + final Class helloClass = resolveFirst( + "net.minecraft.network.protocol.login.ServerboundHelloPacket", + "net.minecraft.network.protocol.login.PacketLoginInStart"); + helloPacketConstructor = helloClass.getConstructor(String.class, UUID.class); + loginAcknowledgedPacket = newNoArgPacket( + "net.minecraft.network.protocol.login.ServerboundLoginAcknowledgedPacket"); + finishConfigurationPacket = newNoArgPacket( + "net.minecraft.network.protocol.configuration.ServerboundFinishConfigurationPacket"); + acceptCodeOfConductPacket = newNoArgPacket( + "net.minecraft.network.protocol.configuration.ServerboundAcceptCodeOfConductPacket"); + selectKnownPacksConstructor = resolveFirst( + "net.minecraft.network.protocol.configuration.ServerboundSelectKnownPacks") + .getConstructor(java.util.List.class); + keepAlivePacketConstructor = resolveFirst( + "net.minecraft.network.protocol.common.ServerboundKeepAlivePacket").getConstructor(long.class); + pongPacketConstructor = resolveFirst( + "net.minecraft.network.protocol.common.ServerboundPongPacket").getConstructor(int.class); + acceptTeleportationConstructor = resolveFirst( + "net.minecraft.network.protocol.game.ServerboundAcceptTeleportationPacket", + "net.minecraft.network.protocol.game.PacketPlayInTeleportAccept").getConstructor(int.class); + + clientInformationClass = NmsReflectionUtils.resolveClass( + "net.minecraft.server.level.ClientInformation", serverClassLoader); + clientInformationCreateDefaultMethod = + NmsReflectionUtils.resolveStaticNoArgFactoryMethod(clientInformationClass, clientInformationClass); + clientInformationPacketConstructor = resolveFirst( + "net.minecraft.network.protocol.common.ServerboundClientInformationPacket") + .getConstructor(clientInformationClass); + + // Keep the serverbound flow reachable so a future distro rename is caught at init, not at first send. + Objects.requireNonNull(serverboundFlow, "SERVERBOUND packet flow constant."); + } + catch (Exception exception) + { + throw new IllegalStateException( + "Failed to initialize the v1_21_R7 full-login driver reflection surface. Cause: %s: %s" + .formatted(exception.getClass().getName(), exception.getMessage()), + exception); + } + } + + // ------------------------------------------------------------------------------------------------------- + // Operations invoked by BotLoginSession (each runs on the Netty event-loop thread from the Proxy handler, + // except openLoginConnection which runs on the caller's thread). + // ------------------------------------------------------------------------------------------------------- + + /** + * Opens a loopback TCP connection and sends the login-intent handshake, transitioning the connection into + * the login phase with the given client-listener Proxy installed. + * + *

Does not yet send the login-start (hello); the caller must store the returned connection before calling + * {@link #sendHello(Object, String)} so the Proxy handler never observes a null connection while reacting to + * the server's login responses. + * + * @param port + * Server port to connect to. + * @param listenerProxy + * Client-listener Proxy that will receive clientbound packets. + * @return The connected {@code Connection}/{@code NetworkManager} instance. + * @throws ReflectiveOperationException + * When a reflective call fails. + */ + Object openConnection(int port, Object listenerProxy) + throws ReflectiveOperationException + { + final InetSocketAddress address = new InetSocketAddress(InetAddress.getLoopbackAddress(), port); + final Object connection = connectToServerMethod.invoke(null, address, eventLoopGroupHolder, null); + initiateLoginConnectionMethod.invoke(connection, "127.0.0.1", port, listenerProxy); + return connection; + } + + /** + * Sends the login-start (hello) packet that begins the login exchange. + * + * @param connection + * The connection returned by {@link #openConnection(int, Object)}. + * @param name + * Player name to log in as. + * @throws ReflectiveOperationException + * When a reflective call fails. + */ + void sendHello(Object connection, String name) + throws ReflectiveOperationException + { + sendMethod.invoke(connection, helloPacketConstructor.newInstance(name, offlineUuid(name))); + } + + /** + * Applies the compression threshold advertised by a {@code ClientboundLoginCompressionPacket}. + */ + void applyCompression(Object connection, Object compressionPacket) + throws ReflectiveOperationException + { + final int threshold = ((Number) readNoArg(compressionPacket, int.class)).intValue(); + setupCompressionMethod.invoke(connection, threshold, false); + } + + /** + * Acknowledges login and transitions the connection into the configuration phase, sending the client's + * locale/settings. + */ + void enterConfigurationPhase(Object connection, Object listenerProxy, @Nullable String locale) + throws ReflectiveOperationException + { + sendMethod.invoke(connection, loginAcknowledgedPacket); + setupInboundProtocolMethod.invoke(connection, configurationClientboundInfo, listenerProxy); + setupOutboundProtocolMethod.invoke(connection, configurationServerboundInfo); + final Object clientInformationPacket = + clientInformationPacketConstructor.newInstance(buildClientInformation(locale)); + sendMethod.invoke(connection, clientInformationPacket); + } + + /** + * Echoes the server's known-pack selection back to it. + */ + void echoKnownPacks(Object connection, Object selectKnownPacksPacket) + throws ReflectiveOperationException + { + final Object knownPacks = readNoArg(selectKnownPacksPacket, java.util.List.class); + sendMethod.invoke(connection, selectKnownPacksConstructor.newInstance(knownPacks)); + } + + /** + * Accepts a server code-of-conduct so the join is not stalled by it. + */ + void acceptCodeOfConduct(Object connection) + throws ReflectiveOperationException + { + sendMethod.invoke(connection, acceptCodeOfConductPacket); + } + + /** + * Finishes configuration and transitions the connection into the play phase. + */ + void enterPlayPhase(Object connection, Object listenerProxy) + throws ReflectiveOperationException + { + sendMethod.invoke(connection, finishConfigurationPacket); + setupInboundProtocolMethod.invoke(connection, gameClientboundInfo, listenerProxy); + setupOutboundProtocolMethod.invoke(connection, gameServerboundInfo); + } + + /** + * Answers a keep-alive so the bot is not timed out. + */ + void answerKeepAlive(Object connection, Object keepAlivePacket) + throws ReflectiveOperationException + { + final long id = ((Number) readNoArg(keepAlivePacket, long.class)).longValue(); + sendMethod.invoke(connection, keepAlivePacketConstructor.newInstance(id)); + } + + /** + * Answers a ping with a pong. + */ + void answerPing(Object connection, Object pingPacket) + throws ReflectiveOperationException + { + final int id = ((Number) readNoArg(pingPacket, int.class)).intValue(); + sendMethod.invoke(connection, pongPacketConstructor.newInstance(id)); + } + + /** + * Acknowledges a teleport so the server accepts the bot's position. + */ + void acceptTeleport(Object connection, Object positionPacket) + throws ReflectiveOperationException + { + final int id = ((Number) readNoArg(positionPacket, int.class)).intValue(); + sendMethod.invoke(connection, acceptTeleportationConstructor.newInstance(id)); + } + + /** + * Extracts a best-effort plain-text kick reason from a disconnect packet. + */ + String disconnectReason(Object disconnectPacket) + { + try + { + final Object reasonComponent = readComponent(disconnectPacket); + if (reasonComponent == null) + return ""; + final String text = NmsReflectionUtils.invokeStringMethod(reasonComponent, "getString"); + return text != null && !text.isBlank() ? text : String.valueOf(reasonComponent); + } + catch (Exception exception) + { + return ""; + } + } + + /** + * Closes the connection's channel, best-effort. + */ + void closeConnection(Object connection) + { + try + { + final Object channel = channelField.get(connection); + if (channel != null) + channelCloseMethod.invoke(channel); + } + catch (Exception exception) + { + LOG.log(System.Logger.Level.DEBUG, "Failed to close full-login bot connection cleanly.", exception); + } + } + + /** + * Creates the client-listener Proxy over the login/configuration/game listener interfaces. + */ + Object newListenerProxy(InvocationHandler handler) + { + return Proxy.newProxyInstance( + serverClassLoader, + new Class[]{loginListenerInterface, configurationListenerInterface, gameListenerInterface}, + handler); + } + + /** + * Classifies a received clientbound packet class so the session can react to it. + * + * @param clientboundPacketClass + * Runtime class of the received packet. + * @return The reaction kind. + */ + PacketKind classify(Class clientboundPacketClass) + { + if (clientboundPacketClass == loginCompressionClass) + return PacketKind.COMPRESSION; + if (clientboundPacketClass == loginFinishedClass) + return PacketKind.LOGIN_FINISHED; + if (clientboundPacketClass == selectKnownPacksClass) + return PacketKind.KNOWN_PACKS; + if (clientboundPacketClass == codeOfConductClass) + return PacketKind.CODE_OF_CONDUCT; + if (clientboundPacketClass == finishConfigurationClass) + return PacketKind.FINISH_CONFIGURATION; + if (clientboundPacketClass == keepAliveClass) + return PacketKind.KEEP_ALIVE; + if (clientboundPacketClass == pingClass) + return PacketKind.PING; + if (clientboundPacketClass == playerPositionClass) + return PacketKind.TELEPORT; + if (clientboundPacketClass == gameLoginClass) + return PacketKind.GAME_LOGIN; + if (clientboundPacketClass == loginDisconnectClass) + return PacketKind.LOGIN_DISCONNECT; + if (clientboundPacketClass == disconnectClass) + return PacketKind.DISCONNECT; + if (consumeClasses.contains(clientboundPacketClass)) + return PacketKind.CONSUME; + return PacketKind.UNKNOWN; + } + + /** + * @return The server's {@code Packet} base class (used by the session to recognize packet-handler methods). + */ + Class packetClass() + { + return packetClass; + } + + /** + * @return The server's {@code PacketFlow} enum class. + */ + Class packetFlowClass() + { + return packetFlowClass; + } + + /** + * @return The server's {@code ConnectionProtocol} enum class. + */ + Class connectionProtocolClass() + { + return connectionProtocolClass; + } + + /** + * @return The {@code CLIENTBOUND} packet-flow constant the Proxy reports as its flow. + */ + Object clientboundFlow() + { + return clientboundFlow; + } + + /** + * Returns the {@code ConnectionProtocol} constant matching a join phase, for the Proxy's {@code protocol()}. + * + * @param phase + * Current join phase. + * @return The matching {@code ConnectionProtocol} enum constant. + */ + Object protocolConstant(BotJoinPhase phase) + { + return switch (phase) + { + case LOGIN -> protocolLogin; + case CONFIGURATION -> protocolConfiguration; + case PLAY -> protocolPlay; + }; + } + + /** + * Computes the offline (cracked) UUID the server derives for a name, matching Bukkit's scheme. + */ + static UUID offlineUuid(String name) + { + return UUID.nameUUIDFromBytes(("OfflinePlayer:" + name).getBytes(StandardCharsets.UTF_8)); + } + + // ------------------------------------------------------------------------------------------------------- + // Resolution helpers. + // ------------------------------------------------------------------------------------------------------- + + private Class resolveFirst(String... classNames) + throws ClassNotFoundException + { + return NmsReflectionUtils.resolveFirstClass(serverClassLoader, classNames); + } + + private Object resolveMinecraftServer() + throws ReflectiveOperationException + { + final Object craftServer = Bukkit.getServer(); + return craftServer.getClass().getMethod("getServer").invoke(craftServer); + } + + private Set> resolveConsumeClasses() + { + final Set> classes = new HashSet<>(); + // Login-phase informational packets (offline mode never sends encryption/custom-query, but tolerate). + addOptional(classes, + "net.minecraft.network.protocol.login.ClientboundHelloPacket", + "net.minecraft.network.protocol.login.PacketLoginOutEncryptionBegin"); + addOptional(classes, + "net.minecraft.network.protocol.login.ClientboundCustomQueryPacket", + "net.minecraft.network.protocol.login.PacketLoginOutCustomPayload"); + // Configuration/common informational packets that require no response (Mojang names on both distros). + for (final String className : new String[]{ + "net.minecraft.network.protocol.configuration.ClientboundRegistryDataPacket", + "net.minecraft.network.protocol.configuration.ClientboundUpdateEnabledFeaturesPacket", + "net.minecraft.network.protocol.configuration.ClientboundResetChatPacket", + "net.minecraft.network.protocol.common.ClientboundCustomPayloadPacket", + "net.minecraft.network.protocol.common.ClientboundUpdateTagsPacket", + "net.minecraft.network.protocol.common.ClientboundResourcePackPushPacket", + "net.minecraft.network.protocol.common.ClientboundResourcePackPopPacket", + "net.minecraft.network.protocol.common.ClientboundStoreCookiePacket", + "net.minecraft.network.protocol.common.ClientboundTransferPacket", + "net.minecraft.network.protocol.common.ClientboundCustomReportDetailsPacket", + "net.minecraft.network.protocol.common.ClientboundServerLinksPacket", + "net.minecraft.network.protocol.common.ClientboundClearDialogPacket", + "net.minecraft.network.protocol.common.ClientboundShowDialogPacket", + "net.minecraft.network.protocol.cookie.ClientboundCookieRequestPacket"}) + { + addOptional(classes, className); + } + return Set.copyOf(classes); + } + + private void addOptional(Set> classes, String... classNames) + { + try + { + classes.add(NmsReflectionUtils.resolveFirstClass(serverClassLoader, classNames)); + } + catch (ClassNotFoundException ignored) + { + LOG.log( + System.Logger.Level.DEBUG, + () -> "Optional login/config consume packet not present on this server: " + classNames[0]); + } + } + + private static Method resolveConnectToServer(Class connectionClass, Class holderClass) + throws NoSuchMethodException + { + for (final Method method : connectionClass.getDeclaredMethods()) + { + if (!Modifier.isStatic(method.getModifiers())) + continue; + if (!connectionClass.isAssignableFrom(method.getReturnType())) + continue; + final Class[] parameters = method.getParameterTypes(); + if (parameters.length != 3 + || !parameters[0].equals(InetSocketAddress.class) + || !parameters[1].equals(holderClass)) + continue; + + method.setAccessible(true); + return method; + } + throw new NoSuchMethodException( + "Failed to resolve connectToServer(InetSocketAddress, EventLoopGroupHolder, ?) on " + + connectionClass.getName() + "."); + } + + private Object newNoArgPacket(String className) + throws ReflectiveOperationException + { + final Constructor constructor = + NmsReflectionUtils.resolveClass(className, serverClassLoader).getDeclaredConstructor(); + constructor.setAccessible(true); + return constructor.newInstance(); + } + + private ProtocolInfos resolveProtocolInfos(Object minecraftServer) + throws ReflectiveOperationException + { + final Class protocolInfoClass = + NmsReflectionUtils.resolveClass("net.minecraft.network.ProtocolInfo", serverClassLoader); + final Class simpleUnboundProtocolClass = + NmsReflectionUtils.resolveClass("net.minecraft.network.protocol.SimpleUnboundProtocol", serverClassLoader); + final Class unboundProtocolClass = + NmsReflectionUtils.resolveClass("net.minecraft.network.protocol.UnboundProtocol", serverClassLoader); + final Class registryFriendlyByteBufClass = + NmsReflectionUtils.resolveClass("net.minecraft.network.RegistryFriendlyByteBuf", serverClassLoader); + final Class registryAccessClass = NmsReflectionUtils.resolveFirstClass( + serverClassLoader, "net.minecraft.core.RegistryAccess", "net.minecraft.core.IRegistryCustom"); + + // Configuration protocol infos, distinguished by the (generic) listener type argument. + final Class configurationProtocolsClass = NmsReflectionUtils.resolveClass( + "net.minecraft.network.protocol.configuration.ConfigurationProtocols", serverClassLoader); + Object configurationClientbound = null; + Object configurationServerbound = null; + for (final Field field : configurationProtocolsClass.getDeclaredFields()) + { + if (!Modifier.isStatic(field.getModifiers()) || !protocolInfoClass.equals(field.getType())) + continue; + field.setAccessible(true); + if (genericArgument(field, 0) == configurationListenerInterface) + configurationClientbound = field.get(null); + else + configurationServerbound = field.get(null); + } + + // Game protocol templates: one SimpleUnboundProtocol (clientbound), one UnboundProtocol (serverbound). + final Class gameProtocolsClass = NmsReflectionUtils.resolveClass( + "net.minecraft.network.protocol.game.GameProtocols", serverClassLoader); + final Field gameClientboundTemplateField = + singleStaticField(gameProtocolsClass, simpleUnboundProtocolClass); + final Field gameServerboundTemplateField = + singleStaticField(gameProtocolsClass, unboundProtocolClass); + final Class gameContextClass = (Class) genericArgumentType(gameServerboundTemplateField, 2); + + final Object registryAccess = resolveRegistryAccess(minecraftServer, registryAccessClass); + final Method decoratorMethod = NmsReflectionUtils.resolveStaticFactoryMethod( + registryFriendlyByteBufClass, Function.class, registryAccessClass); + final Object decorator = decoratorMethod.invoke(null, registryAccess); + + final Method simpleBindMethod = + NmsReflectionUtils.findPublicMethod(simpleUnboundProtocolClass, protocolInfoClass, Function.class); + final Object gameClientbound = + simpleBindMethod.invoke(gameClientboundTemplateField.get(null), decorator); + + final Method unboundBindMethod = NmsReflectionUtils.findPublicMethod( + unboundProtocolClass, protocolInfoClass, Function.class, Object.class); + final Object gameContext = Proxy.newProxyInstance( + serverClassLoader, new Class[]{gameContextClass}, BotLoginReflection::gameContextInvoke); + final Object gameServerbound = + unboundBindMethod.invoke(gameServerboundTemplateField.get(null), decorator, gameContext); + + return new ProtocolInfos( + Objects.requireNonNull(configurationClientbound, "configuration clientbound ProtocolInfo"), + Objects.requireNonNull(configurationServerbound, "configuration serverbound ProtocolInfo"), + gameClientbound, + gameServerbound); + } + + @SuppressWarnings({"PMD.UseVarargs", "PMD.CompareObjectsWithEquals"}) // InvocationHandler contract; proxy identity. + private static Object gameContextInvoke(Object proxy, Method method, Object @Nullable [] args) + { + if (method.getReturnType() == boolean.class) + return Boolean.FALSE; + if ("toString".equals(method.getName())) + return "GameProtocols$Context(hasInfiniteMaterials=false)"; + if ("hashCode".equals(method.getName())) + return System.identityHashCode(proxy); + if ("equals".equals(method.getName())) + return args != null && args.length == 1 && proxy == args[0]; + return null; + } + + private Object resolveRegistryAccess(Object minecraftServer, Class registryAccessClass) + throws ReflectiveOperationException + { + final Method named = NmsReflectionUtils.findNamedNoArgMethod(minecraftServer.getClass(), "registryAccess"); + final Method accessor = named != null + ? named + : NmsReflectionUtils.findInstanceNoArgMethodByReturnType(minecraftServer.getClass(), registryAccessClass); + return Objects.requireNonNull(accessor.invoke(minecraftServer), "registryAccess() returned null."); + } + + private static Field singleStaticField(Class owner, Class fieldType) + throws NoSuchFieldException + { + Field found = null; + for (final Field field : owner.getDeclaredFields()) + { + if (!Modifier.isStatic(field.getModifiers()) || !fieldType.equals(field.getType())) + continue; + if (found != null) + throw new NoSuchFieldException("Multiple static " + fieldType.getName() + " fields on " + + owner.getName() + "; cannot disambiguate."); + found = field; + } + if (found == null) + throw new NoSuchFieldException("No static " + fieldType.getName() + " field on " + owner.getName() + "."); + found.setAccessible(true); + return found; + } + + private static @Nullable Class genericArgument(Field field, int index) + { + final Type argument = genericArgumentType(field, index); + return argument instanceof Class clazz ? clazz : null; + } + + private static Type genericArgumentType(Field field, int index) + { + return ((ParameterizedType) field.getGenericType()).getActualTypeArguments()[index]; + } + + private Object buildClientInformation(@Nullable String locale) + throws ReflectiveOperationException + { + final Object defaultInformation = clientInformationCreateDefaultMethod.invoke(null); + if (locale == null) + return defaultInformation; + + // Rebuild the record with only the language (first component) replaced, so the locale wire path is + // exercised without depending on the (obfuscated) accessor/constructor-parameter names. + final RecordComponent[] components = clientInformationClass.getRecordComponents(); + final Class[] parameterTypes = new Class[components.length]; + final Object[] values = new Object[components.length]; + for (int idx = 0; idx < components.length; ++idx) + { + parameterTypes[idx] = components[idx].getType(); + values[idx] = components[idx].getAccessor().invoke(defaultInformation); + } + values[0] = locale; + final Constructor canonical = clientInformationClass.getDeclaredConstructor(parameterTypes); + canonical.setAccessible(true); + return canonical.newInstance(values); + } + + private Object readNoArg(Object target, Class returnType) + throws ReflectiveOperationException + { + final Method accessor = accessorCache.computeIfAbsent( + new AccessorKey(target.getClass(), returnType), + key -> findNoArgAccessor(key.owner(), key.returnType())); + return accessor.invoke(target); + } + + private static Method findNoArgAccessor(Class owner, Class returnType) + { + for (Class cursor = owner; cursor != null; cursor = cursor.getSuperclass()) + { + for (final Method method : cursor.getDeclaredMethods()) + { + if (method.getParameterCount() != 0 || Modifier.isStatic(method.getModifiers())) + continue; + final String name = method.getName(); + if ("hashCode".equals(name) || "toString".equals(name)) + continue; + if (!returnType.equals(method.getReturnType())) + continue; + method.setAccessible(true); + return method; + } + } + throw new IllegalStateException( + "No no-arg accessor returning " + returnType.getName() + " on " + owner.getName() + "."); + } + + private @Nullable Object readComponent(Object packet) + throws ReflectiveOperationException + { + for (Class cursor = packet.getClass(); cursor != null; cursor = cursor.getSuperclass()) + { + for (final Method method : cursor.getDeclaredMethods()) + { + if (method.getParameterCount() != 0 || Modifier.isStatic(method.getModifiers())) + continue; + if (!method.getReturnType().getName().startsWith("net.minecraft.network.chat.")) + continue; + method.setAccessible(true); + return method.invoke(packet); + } + } + return null; + } + + private record AccessorKey(Class owner, Class returnType) + { + } + + private record ProtocolInfos( + Object configurationClientbound, + Object configurationServerbound, + Object gameClientbound, + Object gameServerbound) + { + } +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java new file mode 100644 index 00000000..19240fd1 --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java @@ -0,0 +1,256 @@ +package nl.pim16aap2.lightkeeper.nms.v121r7; + +import nl.pim16aap2.lightkeeper.nms.api.BotJoinPhase; +import nl.pim16aap2.lightkeeper.nms.api.BotLoginRequest; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginOutcome; +import org.jspecify.annotations.Nullable; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * A single full-login attempt: builds the client-listener Proxy, drives the login/configuration/play phase + * transitions in response to server packets, and reports the terminal outcome. + * + *

This class is the Proxy's {@link InvocationHandler}. The server-driven packet handlers run on the Netty + * event-loop thread (via {@code Connection.channelRead0 -> packet.handle(listener)}); {@link #run(BotLoginRequest)} + * runs on the caller's thread and blocks on a {@link CompletableFuture} until a handler completes it. + * + *

Dispatch keys on the packet class ({@code method.getParameterTypes()[0]}), never the method name, + * because Spigot obfuscates every listener method to {@code a}. Unhandled packets are fail-loud in the + * login/configuration phase (they would silently stall the join) and warn-once in the play phase. + */ +final class BotLoginSession implements InvocationHandler +{ + private static final System.Logger LOG = System.getLogger(BotLoginSession.class.getName()); + + private final BotLoginReflection reflection; + private final CompletableFuture outcome = new CompletableFuture<>(); + private final Map, Boolean> warnedPlayPackets = new ConcurrentHashMap<>(); + + private volatile BotJoinPhase phase = BotJoinPhase.LOGIN; + private volatile @Nullable Object connection; + private volatile @Nullable Object listenerProxy; + private volatile @Nullable String locale; + private volatile String playerName = ""; + + BotLoginSession(BotLoginReflection reflection) + { + this.reflection = reflection; + } + + /** + * Runs the login pipeline and blocks until it reaches the play phase, is denied, or times out. + * + * @param request + * Login inputs. + * @return The terminal outcome. + */ + IBotLoginOutcome run(BotLoginRequest request) + { + this.locale = request.locale(); + this.playerName = request.name(); + final Object proxy = reflection.newListenerProxy(this); + this.listenerProxy = proxy; + + final Object openedConnection; + try + { + openedConnection = reflection.openConnection(request.port(), proxy); + this.connection = openedConnection; + reflection.sendHello(openedConnection, request.name()); + } + catch (ReflectiveOperationException exception) + { + throw new IllegalStateException( + "Failed to open full-login connection for bot '%s'.".formatted(request.name()), exception); + } + + try + { + final IBotLoginOutcome result = outcome.get(request.timeout().toMillis(), TimeUnit.MILLISECONDS); + if (!(result instanceof IBotLoginOutcome.Joined)) + reflection.closeConnection(openedConnection); + return result; + } + catch (TimeoutException exception) + { + reflection.closeConnection(openedConnection); + return new IBotLoginOutcome.TimedOut(); + } + catch (ExecutionException exception) + { + reflection.closeConnection(openedConnection); + throw new IllegalStateException( + "Full-login pipeline failed for bot '%s'.".formatted(request.name()), exception); + } + catch (InterruptedException exception) + { + Thread.currentThread().interrupt(); + reflection.closeConnection(openedConnection); + throw new IllegalStateException( + "Interrupted while awaiting full-login of bot '%s'.".formatted(request.name()), exception); + } + } + + // ------------------------------------------------------------------------------------------------------- + // InvocationHandler: called by the server's packet dispatch on the Netty event-loop thread. + // ------------------------------------------------------------------------------------------------------- + + @Override + @SuppressWarnings("PMD.UseVarargs") // InvocationHandler.invoke's array parameter is fixed by the contract. + public @Nullable Object invoke(Object proxy, Method method, Object @Nullable [] args) + throws Throwable + { + if (method.getDeclaringClass() == Object.class) + return invokeObjectMethod(proxy, method, firstArg(args)); + + final Class returnType = method.getReturnType(); + final Class[] parameterTypes = method.getParameterTypes(); + + if (parameterTypes.length == 0) + { + if (returnType == reflection.packetFlowClass()) + return reflection.clientboundFlow(); + if (returnType == reflection.connectionProtocolClass()) + return reflection.protocolConstant(phase); + if (returnType == boolean.class) + return Boolean.TRUE; // isAcceptingMessages + } + else if (parameterTypes.length == 1) + { + // Dispatch keys on whether the single argument is a Packet, flattened to keep nesting shallow. + final boolean isPacket = reflection.packetClass().isAssignableFrom(parameterTypes[0]); + if (isPacket && returnType == void.class) + { + dispatchPacket(firstArg(args)); + return null; + } + if (isPacket && returnType == boolean.class) + return Boolean.TRUE; // shouldHandleMessage + if (!isPacket && returnType == void.class) + { + handleTransportDisconnect(); // onDisconnect(DisconnectionDetails) + return null; + } + } + + if (method.isDefault()) + return InvocationHandler.invokeDefault(proxy, method, args); + return returnType == boolean.class ? Boolean.FALSE : null; + } + + @SuppressWarnings("PMD.CompareObjectsWithEquals") // Proxy equality is identity by design. + private @Nullable Object invokeObjectMethod(Object proxy, Method method, @Nullable Object other) + { + return switch (method.getName()) + { + case "equals" -> proxy == other; + case "hashCode" -> System.identityHashCode(proxy); + case "toString" -> "BotLoginSession$Proxy(phase=" + phase + ")"; + default -> null; + }; + } + + private static @Nullable Object firstArg(Object @Nullable ... args) + { + return args == null || args.length == 0 ? null : args[0]; + } + + private void dispatchPacket(@Nullable Object packet) + { + if (packet == null || outcome.isDone()) + return; + + final Class packetClass = packet.getClass(); + try + { + switch (reflection.classify(packetClass)) + { + case COMPRESSION -> reflection.applyCompression(connection(), packet); + case LOGIN_FINISHED -> + { + reflection.enterConfigurationPhase(connection(), proxy(), locale); + phase = BotJoinPhase.CONFIGURATION; + } + case KNOWN_PACKS -> reflection.echoKnownPacks(connection(), packet); + case CODE_OF_CONDUCT -> reflection.acceptCodeOfConduct(connection()); + case FINISH_CONFIGURATION -> + { + reflection.enterPlayPhase(connection(), proxy()); + phase = BotJoinPhase.PLAY; + } + case KEEP_ALIVE -> reflection.answerKeepAlive(connection(), packet); + case PING -> reflection.answerPing(connection(), packet); + case TELEPORT -> reflection.acceptTeleport(connection(), packet); + case GAME_LOGIN -> completeJoined(); + case LOGIN_DISCONNECT, DISCONNECT -> completeDenied(packet); + case CONSUME -> + { + // Known informational packet; no response required. + } + case UNKNOWN -> handleUnknownPacket(packetClass); + } + } + catch (ReflectiveOperationException exception) + { + outcome.completeExceptionally(new IllegalStateException( + "Failed to handle %s packet %s.".formatted(phase, packetClass.getName()), exception)); + } + } + + private void handleUnknownPacket(Class packetClass) + { + if (phase == BotJoinPhase.PLAY) + { + if (warnedPlayPackets.putIfAbsent(packetClass, Boolean.TRUE) == null) + LOG.log(System.Logger.Level.DEBUG, () -> "Ignoring unhandled play packet " + packetClass.getName()); + return; + } + + // Fail-loud: an unhandled login/configuration packet would silently stall the join. + outcome.completeExceptionally(new IllegalStateException( + "Unhandled %s packet from server: %s. Add it to the login driver's packet table." + .formatted(phase, packetClass.getName()))); + } + + private void handleTransportDisconnect() + { + if (!outcome.isDone()) + outcome.complete(new IBotLoginOutcome.Denied(phase, "Connection closed during " + phase + ".")); + } + + private void completeJoined() + { + if (!outcome.isDone()) + outcome.complete(new IBotLoginOutcome.Joined(playerName)); + } + + private void completeDenied(Object disconnectPacket) + { + if (!outcome.isDone()) + outcome.complete(new IBotLoginOutcome.Denied(phase, reflection.disconnectReason(disconnectPacket))); + } + + private Object connection() + { + final Object current = connection; + if (current == null) + throw new IllegalStateException("Connection is not initialized."); + return current; + } + + private Object proxy() + { + final Object current = listenerProxy; + if (current == null) + throw new IllegalStateException("Listener proxy is not initialized."); + return current; + } +} diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7.java index 67048eb2..de6faa45 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7.java @@ -1,5 +1,6 @@ package nl.pim16aap2.lightkeeper.nms.v121r7; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginDriver; import nl.pim16aap2.lightkeeper.nms.api.IBotPlayerNmsAdapter; import org.bukkit.Bukkit; import org.bukkit.Location; @@ -64,6 +65,12 @@ public final class BotPlayerNmsAdapterV1_21_R7 implements IBotPlayerNmsAdapter private final Method dataResultErrorMethod; private final Object serverboundPacketFlow; + /** + * Full-login driver, resolved lazily so the (larger) login reflection surface is only touched when a + * {@code FULL_LOGIN} join is actually requested; legacy-spawn-only usage never pays for it. + */ + private volatile @Nullable IBotLoginDriver botLoginDriver; + /** * Initializes reflective handles for the Paper 1.21.11 server internals. */ @@ -400,6 +407,30 @@ public Player spawnPlayer(UUID uuid, String name, World world, Location spawnLoc } } + /** + * Returns the full-login driver, resolving its reflection surface on first use. + * + * @return The full-login driver for this server. + */ + @Override + public IBotLoginDriver loginDriver() + { + IBotLoginDriver driver = botLoginDriver; + if (driver == null) + { + synchronized (this) + { + driver = botLoginDriver; + if (driver == null) + { + driver = new BotLoginPipelineDriver(); + botLoginDriver = driver; + } + } + } + return driver; + } + /** * Removes a previously spawned synthetic player from the server and internal channel tracking. * diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java index 09dfbc68..b3919522 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java @@ -388,6 +388,118 @@ static Method findCompatibleMethod(Class type, Class... argumentTypes) ); } + /** + * Resolves an enum constant by its {@link Enum#name() name}, tolerating obfuscated field names. + * + * @param enumClass + * Enum class to search. + * @param name + * Unobfuscated constant name (e.g. {@code "CLIENTBOUND"}). + * @return + * Matching enum constant. + * @throws IllegalStateException + * When no constant with that name exists. + */ + static Object resolveEnumConstant(Class enumClass, String name) + { + final Object[] constants = Objects.requireNonNull( + enumClass.getEnumConstants(), "Expected enum constants for " + enumClass.getName()); + for (final Object constant : constants) + { + if (((Enum) constant).name().equals(name)) + return constant; + } + throw new IllegalStateException( + "No enum constant '" + name + "' found in " + enumClass.getName() + "."); + } + + /** + * Finds a public, non-static method by return type and parameter types, tolerating obfuscated method + * names. Parameters and the return type are matched by assignability. + * + * @param type + * Class to search (including superclasses). + * @param returnType + * Required return type (supertype match accepted; use {@code void.class} for void). + * @param parameterTypes + * Required parameter types (each matched by assignability from the declared parameter). + * @return + * Found method with accessibility set. + * @throws NoSuchMethodException + * When no matching public method exists. + */ + static Method findPublicMethod(Class type, Class returnType, Class... parameterTypes) + throws NoSuchMethodException + { + for (Class cursor = type; cursor != null; cursor = cursor.getSuperclass()) + { + for (final Method method : cursor.getDeclaredMethods()) + { + if (!Modifier.isPublic(method.getModifiers()) || Modifier.isStatic(method.getModifiers())) + continue; + if (!returnType.isAssignableFrom(method.getReturnType())) + continue; + if (!parametersAssignable(method, parameterTypes)) + continue; + + method.setAccessible(true); + return method; + } + } + throw new NoSuchMethodException( + "Failed to resolve public method on " + type.getName() + " returning " + returnType.getName() + + " with parameters " + List.of(parameterTypes) + "."); + } + + private static boolean parametersAssignable(Method method, Class... parameterTypes) + { + final Class[] declared = method.getParameterTypes(); + if (declared.length != parameterTypes.length) + return false; + for (int idx = 0; idx < declared.length; ++idx) + { + if (!declared[idx].isAssignableFrom(parameterTypes[idx])) + return false; + } + return true; + } + + /** + * Finds a non-static, no-argument method whose return type is assignable to the given type. + * + *

Unlike {@link #findNoArgMethodByReturnType}, static methods are skipped so an instance accessor is + * preferred over a static default/factory of the same return type. + * + * @param type + * Root class to search. + * @param returnType + * Expected return type (supertype match accepted). + * @return + * Found method with accessibility set. + * @throws NoSuchMethodException + * When no matching instance method exists. + */ + static Method findInstanceNoArgMethodByReturnType(Class type, Class returnType) + throws NoSuchMethodException + { + for (Class cursor = type; cursor != null; cursor = cursor.getSuperclass()) + { + for (final Method method : cursor.getDeclaredMethods()) + { + if (Modifier.isStatic(method.getModifiers()) || method.getParameterCount() != 0) + continue; + if (!returnType.isAssignableFrom(method.getReturnType())) + continue; + + method.setAccessible(true); + return method; + } + } + throw new NoSuchMethodException( + "Failed to resolve instance no-arg method on " + type.getName() + " returning " + + returnType.getName() + "."); + } + /** * Drains all elements from a queue into an immutable list. * diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java new file mode 100644 index 00000000..c3a09761 --- /dev/null +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java @@ -0,0 +1,106 @@ +package nl.pim16aap2.lightkeeper.nms.v121r7; + +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Unit tests for the pure, server-decoupled reflection helpers the full-login driver relies on to resolve the + * obfuscated Spigot surface structurally. The server-coupled pipeline itself is exercised by the integration + * tests (it cannot run without a live server). + */ +class BotLoginReflectionHelpersTest +{ + private enum SampleEnum + { + ALPHA, + BETA + } + + @SuppressWarnings("unused") + private static final class SampleTarget + { + public static String staticAccessor() + { + return "static"; + } + + public String instanceAccessor() + { + return "instance"; + } + + public void ambiguous(String first, int second) + { + } + + private void ambiguous(int flipped, String order) + { + } + } + + @Test + void resolveEnumConstant_shouldFindConstantByName() + { + // execute + final Object constant = NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, "BETA"); + + // verify + assertThat(constant).isEqualTo(SampleEnum.BETA); + } + + @Test + void resolveEnumConstant_shouldThrowWhenNameIsUnknown() + { + // execute + verify + assertThatThrownBy(() -> NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, "GAMMA")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("GAMMA"); + } + + @Test + void findPublicMethod_shouldPreferPublicOverPrivateWithSameArity() + throws Exception + { + // execute + final Method method = + NmsReflectionUtils.findPublicMethod(SampleTarget.class, void.class, String.class, int.class); + + // verify — the private (int, String) overload must not be selected. + assertThat(method.getName()).isEqualTo("ambiguous"); + assertThat(method.getParameterTypes()).containsExactly(String.class, int.class); + } + + @Test + void findInstanceNoArgMethodByReturnType_shouldSkipStaticMethods() + throws Exception + { + // execute + final Method method = + NmsReflectionUtils.findInstanceNoArgMethodByReturnType(SampleTarget.class, String.class); + + // verify + assertThat(method.getName()).isEqualTo("instanceAccessor"); + assertThat(method.invoke(new SampleTarget())).isEqualTo("instance"); + } + + @Test + void offlineUuid_shouldMatchBukkitOfflinePlayerScheme() + { + // setup + final String name = "lk_full_login"; + final UUID expected = + UUID.nameUUIDFromBytes(("OfflinePlayer:" + name).getBytes(StandardCharsets.UTF_8)); + + // execute + final UUID actual = BotLoginReflection.offlineUuid(name); + + // verify + assertThat(actual).isEqualTo(expected); + } +} From 5dbeaa580976ae3e31c6e08ba6e9c1c7059322aa Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 11:17:43 +0200 Subject: [PATCH 03/10] feat(agent): route CreatePlayer by joinMode; await PlayerJoinEvent with bounded timeout; kicks->typed errors Goal: turn the S6/LK-18 driver into a real join on the agent side, so FULL_LOGIN bots join through the pipeline and denials/timeouts become typed errors, while LEGACY_SPAWN keeps today's fast path. How this slots into S6/LK-18: - handleCreatePlayer branches on joinMode. LEGACY_SPAWN keeps the existing main-thread reflective spawn (now asserting the non-null UUID the protocol guarantees). - FULL_LOGIN orchestrates the join off the main thread (the handler already runs on a per-connection virtual thread): registers a one-shot PlayerJoinEvent listener filtered to the bot name, invokes botLoginDriver.login(name, serverPort, locale, timeout), and awaits the join latch. Denied -> AgentProtocolException(PLAYER_JOIN_DENIED, kick reason); TimedOut / no join event -> PLAYER_JOIN_TIMEOUT. On success it registers the joined Player, applies health/permissions, and returns the same CreatePlayer.Response shape carrying the server-derived offline UUID. - The bounded timeout reuses the agent's configured sync-operation timeout, now exposed on AgentMainThreadExecutor so one knob governs how long the agent blocks before the client watchdog fires. - FULL_LOGIN bots are real Players, so every other AgentPlayerActions handler works unchanged and stays pure Bukkit (swappability invariant preserved). - Tests: existing spawn tests pin LEGACY_SPAWN; new test asserts a login denial surfaces as AgentProtocolException(PLAYER_JOIN_DENIED). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../agent/spigot/AgentMainThreadExecutor.java | 14 ++ .../agent/spigot/AgentPlayerActions.java | 156 +++++++++++++++++- .../agent/spigot/AgentPlayerActionsTest.java | 38 ++++- .../spigot/AgentRequestDispatcherTest.java | 3 +- 4 files changed, 204 insertions(+), 7 deletions(-) diff --git a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java index 23dd9faa..f9991d1a 100644 --- a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java +++ b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java @@ -77,6 +77,20 @@ final class AgentMainThreadExecutor * {@link AgentProtocolException} with {@link AgentErrorCode#TIMEOUT} when the operation exceeds the * configured timeout, or {@link AgentErrorCode#INTERRUPTED} when the waiting thread is interrupted. */ + /** + * Returns the configured maximum wait, in seconds, for a scheduled synchronous server operation. + * + *

Reused by long-running orchestration (such as awaiting a full-login {@code PlayerJoinEvent}) so a + * single, configurable bound governs how long the agent blocks before reporting a timeout to the client. + * + * @return + * Positive timeout in seconds. + */ + long syncOperationTimeoutSeconds() + { + return syncOperationTimeoutSeconds; + } + T callOnMainThread(Callable callable) throws Exception { diff --git a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java index b553be5d..a449d30b 100644 --- a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java +++ b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java @@ -1,6 +1,10 @@ package nl.pim16aap2.lightkeeper.agent.spigot; +import nl.pim16aap2.lightkeeper.nms.api.BotLoginRequest; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginOutcome; import nl.pim16aap2.lightkeeper.nms.api.IBotPlayerNmsAdapter; +import nl.pim16aap2.lightkeeper.protocol.AgentErrorCode; +import nl.pim16aap2.lightkeeper.protocol.AgentProtocolException; import nl.pim16aap2.lightkeeper.protocol.CreatePlayer; import nl.pim16aap2.lightkeeper.protocol.ExecutePlayerCommand; import nl.pim16aap2.lightkeeper.protocol.HasPlayerPermission; @@ -18,13 +22,24 @@ import org.bukkit.block.Block; import org.bukkit.block.BlockFace; import org.bukkit.entity.Player; +import org.bukkit.event.EventPriority; +import org.bukkit.event.HandlerList; +import org.bukkit.event.Listener; import org.bukkit.event.block.Action; import org.bukkit.event.player.PlayerInteractEvent; +import org.bukkit.event.player.PlayerJoinEvent; import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.ItemStack; +import org.bukkit.plugin.EventExecutor; import org.bukkit.plugin.java.JavaPlugin; +import org.jspecify.annotations.Nullable; + +import java.time.Duration; import java.util.Objects; import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; /** * Protocol action handler for synthetic player lifecycle, movement, and world interactions. @@ -74,20 +89,41 @@ final class AgentPlayerActions } /** - * Handles {@code CREATE_PLAYER} by spawning a synthetic player and registering it in the store. + * Handles {@code CREATE_PLAYER} by routing to the full-login pipeline or the legacy spawn path. * * @param command - * Typed command carrying player name, UUID, world, spawn coordinates, health, and permissions. + * Typed command carrying player name, UUID, world, spawn coordinates, health, permissions, join mode, + * and locale. * @return Response containing the created player's UUID and name. * * @throws Exception - * Propagates validation and main-thread execution failures. + * Propagates validation, login, and main-thread execution failures. */ CreatePlayer.Response handleCreatePlayer(CreatePlayer.Command command) throws Exception + { + return switch (command.joinMode()) + { + case FULL_LOGIN -> handleFullLoginPlayer(command); + case LEGACY_SPAWN -> handleLegacySpawnPlayer(command); + }; + } + + /** + * Spawns a synthetic player through the internal legacy spawn path (main thread). + * + * @param command + * Typed create-player command in {@code LEGACY_SPAWN} mode (a non-null UUID is guaranteed by validation). + * @return Response containing the created player's UUID and name. + * + * @throws Exception + * Propagates validation and main-thread execution failures. + */ + private CreatePlayer.Response handleLegacySpawnPlayer(CreatePlayer.Command command) + throws Exception { final String name = command.name(); - final UUID uuid = command.uuid(); + final UUID uuid = Objects.requireNonNull(command.uuid(), "uuid may not be null under LEGACY_SPAWN."); final String worldName = command.worldName(); final Double x = command.x(); final Double y = command.y(); @@ -120,12 +156,122 @@ CreatePlayer.Response handleCreatePlayer(CreatePlayer.Command command) }); plugin.getLogger().info( - "LK_AGENT: Created synthetic player '%s' (%s) in world '%s'." + "LK_AGENT: Spawned synthetic player '%s' (%s) in world '%s'." .formatted(player.getName(), player.getUniqueId(), worldName) ); return new CreatePlayer.Response(player.getUniqueId(), player.getName()); } + /** + * Joins a synthetic player through the full vanilla login pipeline. + * + *

The driver runs off the main thread (it fires {@code AsyncPlayerPreLoginEvent} on the connection + * thread; the join completes on the main thread), so this method blocks the calling connection thread — not + * the server main thread — while awaiting the {@code PlayerJoinEvent} for the bot's name. Whitelist/ban/full/ + * plugin denials and timeouts are surfaced as typed {@link AgentProtocolException}s. + * + * @param command + * Typed create-player command in {@code FULL_LOGIN} mode. + * @return Response containing the server-assigned offline UUID and name. + * + * @throws Exception + * Propagates login failures and main-thread execution failures. + */ + private CreatePlayer.Response handleFullLoginPlayer(CreatePlayer.Command command) + throws Exception + { + final String name = command.name(); + final String locale = command.locale(); + final Double health = command.health(); + final String permissionsCsv = command.permissionsCsv(); + final long timeoutSeconds = mainThreadExecutor.syncOperationTimeoutSeconds(); + + final CountDownLatch joinLatch = new CountDownLatch(1); + final AtomicReference joinedPlayer = new AtomicReference<>(); + final Listener listener = new Listener() + { + }; + final EventExecutor executor = (ignoredListener, event) -> + { + if (event instanceof PlayerJoinEvent joinEvent && joinEvent.getPlayer().getName().equals(name)) + { + joinedPlayer.set(joinEvent.getPlayer()); + joinLatch.countDown(); + } + }; + mainThreadExecutor.callOnMainThread(() -> + { + Bukkit.getPluginManager().registerEvent( + PlayerJoinEvent.class, listener, EventPriority.MONITOR, executor, plugin); + return Boolean.TRUE; + }); + + try + { + final int port = Bukkit.getServer().getPort(); + final IBotLoginOutcome outcome = botPlayerNmsAdapter.loginDriver().login( + new BotLoginRequest(name, port, locale, Duration.ofSeconds(timeoutSeconds))); + + if (outcome instanceof IBotLoginOutcome.Denied denied) + throw new AgentProtocolException( + AgentErrorCode.PLAYER_JOIN_DENIED, + "Bot '%s' was denied during %s: %s".formatted(name, denied.phase(), denied.reason())); + if (outcome instanceof IBotLoginOutcome.TimedOut) + throw new AgentProtocolException( + AgentErrorCode.PLAYER_JOIN_TIMEOUT, + "Bot '%s' did not complete the login pipeline within %d seconds.".formatted(name, timeoutSeconds)); + + if (!joinLatch.await(timeoutSeconds, TimeUnit.SECONDS)) + throw new AgentProtocolException( + AgentErrorCode.PLAYER_JOIN_TIMEOUT, + "Bot '%s' completed login but no PlayerJoinEvent fired within %d seconds." + .formatted(name, timeoutSeconds)); + + final Player player = Objects.requireNonNull(joinedPlayer.get(), "joined player"); + registerJoinedPlayer(player, health, permissionsCsv); + + plugin.getLogger().info( + "LK_AGENT: Full-login player '%s' (%s) joined.".formatted(player.getName(), player.getUniqueId())); + return new CreatePlayer.Response(player.getUniqueId(), player.getName()); + } + finally + { + mainThreadExecutor.callOnMainThread(() -> + { + HandlerList.unregisterAll(listener); + return Boolean.TRUE; + }); + } + } + + /** + * Registers a freshly joined full-login player in the store and applies health/permissions on the main thread. + * + * @param player + * The joined Bukkit player. + * @param health + * Starting health, or {@code null} to leave the server default. + * @param permissionsCsv + * Comma-separated permission nodes to grant, or {@code null} for none. + * + * @throws Exception + * Propagates main-thread execution failures. + */ + private void registerJoinedPlayer(Player player, @Nullable Double health, @Nullable String permissionsCsv) + throws Exception + { + final UUID uuid = player.getUniqueId(); + mainThreadExecutor.callOnMainThread(() -> + { + playerStore.registerSyntheticPlayer(uuid, player); + if (health != null) + player.setHealth(Math.min(player.getMaxHealth(), health)); + if (permissionsCsv != null && !permissionsCsv.isBlank()) + playerStore.setPermissions(plugin, uuid, player, permissionsCsv); + return Boolean.TRUE; + }); + } + /** * Handles {@code REMOVE_PLAYER} by removing permissions, despawning the synthetic player, and cleaning state. * diff --git a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java index e88f0025..7bc529bd 100644 --- a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java +++ b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java @@ -1,7 +1,12 @@ package nl.pim16aap2.lightkeeper.agent.spigot; import tools.jackson.databind.ObjectMapper; +import nl.pim16aap2.lightkeeper.nms.api.BotJoinPhase; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginDriver; +import nl.pim16aap2.lightkeeper.nms.api.IBotLoginOutcome; import nl.pim16aap2.lightkeeper.nms.api.IBotPlayerNmsAdapter; +import nl.pim16aap2.lightkeeper.protocol.AgentErrorCode; +import nl.pim16aap2.lightkeeper.protocol.AgentProtocolException; import nl.pim16aap2.lightkeeper.protocol.CreatePlayer; import nl.pim16aap2.lightkeeper.protocol.DropItem; import nl.pim16aap2.lightkeeper.protocol.DropResult; @@ -10,6 +15,7 @@ import nl.pim16aap2.lightkeeper.protocol.GetPlayerInventory; import nl.pim16aap2.lightkeeper.protocol.GetPlayerMessages; import nl.pim16aap2.lightkeeper.protocol.HasPlayerPermission; +import nl.pim16aap2.lightkeeper.protocol.JoinMode; import nl.pim16aap2.lightkeeper.protocol.LeftClickBlock; import nl.pim16aap2.lightkeeper.protocol.MutatePlayerPermission; import nl.pim16aap2.lightkeeper.protocol.PlacePlayerBlock; @@ -557,7 +563,8 @@ void handleCreatePlayer_shouldSpawnPlayerWithCoordinatesAndOptionalPermissions() final PermissionAttachment attachment = mock(); when(player.addAttachment(any())).thenReturn(attachment); final CreatePlayer.Command command = new CreatePlayer.Command( - "request-create", "testbot", uuid, "world", 10.0, 64.0, 20.0, null, "test.perm" + "request-create", "testbot", uuid, "world", 10.0, 64.0, 20.0, null, "test.perm", + JoinMode.LEGACY_SPAWN, null ); // execute @@ -579,6 +586,35 @@ void handleCreatePlayer_shouldSpawnPlayerWithCoordinatesAndOptionalPermissions() verify(player).removeAttachment(attachment); } + @Test + void handleCreatePlayer_shouldSurfaceFullLoginDenialAsTypedError() + { + // setup + final PlayerActionsFixture fixture = createPlayerActionsFixture(); + final IBotLoginDriver loginDriver = mock(); + when(fixture.nmsAdapter().loginDriver()).thenReturn(loginDriver); + when(loginDriver.login(any())).thenReturn(new IBotLoginOutcome.Denied(BotJoinPhase.LOGIN, "Banned")); + final org.bukkit.Server server = mock(); + when(server.getPort()).thenReturn(25_565); + final PluginManager pluginManager = mock(); + final CreatePlayer.Command command = new CreatePlayer.Command( + "request-full", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, "en_us"); + + // execute + verify + try (MockedStatic bukkit = mockStatic(Bukkit.class)) + { + bukkit.when(Bukkit::isPrimaryThread).thenReturn(true); + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getPluginManager).thenReturn(pluginManager); + + assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) + .isInstanceOf(AgentProtocolException.class) + .satisfies(thrown -> assertThat(((AgentProtocolException) thrown).errorCode()) + .isEqualTo(AgentErrorCode.PLAYER_JOIN_DENIED)) + .hasMessageContaining("Banned"); + } + } + @Test void handlePlayerChat_shouldInvokePlayerChatOnMainThread() throws Exception diff --git a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentRequestDispatcherTest.java b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentRequestDispatcherTest.java index c19bb3ec..2edd552d 100644 --- a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentRequestDispatcherTest.java +++ b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentRequestDispatcherTest.java @@ -30,6 +30,7 @@ import nl.pim16aap2.lightkeeper.protocol.HasPlayerPermission; import nl.pim16aap2.lightkeeper.protocol.IAgentCommand; import nl.pim16aap2.lightkeeper.protocol.IsChunkLoaded; +import nl.pim16aap2.lightkeeper.protocol.JoinMode; import nl.pim16aap2.lightkeeper.protocol.LeftClickBlock; import nl.pim16aap2.lightkeeper.protocol.LoadChunk; import nl.pim16aap2.lightkeeper.protocol.MainWorld; @@ -380,7 +381,7 @@ void handleRequestLine_shouldDispatchSupportedActionsToTheirHandlers() dispatchExpectingSuccess(fixture, toJson(new ExecuteCommand.Command("request-2", CommandSource.CONSOLE, "time set day"))); dispatchExpectingSuccess(fixture, toJson(new BlockType.Command("request-3", "world", 0, 64, 0))); dispatchExpectingSuccess(fixture, toJson(new SetBlock.Command("request-4", "world", 0, 64, 0, "stone", null))); - dispatchExpectingSuccess(fixture, toJson(new CreatePlayer.Command("request-5", "bot", uuid, "world", null, null, null, null, null))); + dispatchExpectingSuccess(fixture, toJson(new CreatePlayer.Command("request-5", "bot", uuid, "world", null, null, null, null, null, JoinMode.LEGACY_SPAWN, null))); dispatchExpectingSuccess(fixture, toJson(new RemovePlayer.Command("request-6", uuid))); dispatchExpectingSuccess(fixture, toJson(new ExecutePlayerCommand.Command("request-7", uuid, "gamemode creative"))); dispatchExpectingSuccess(fixture, toJson(new PlacePlayerBlock.Command("request-8", uuid, "stone", 0, 64, 0))); From 66c076cfd7a2aa556c3a2686bc8aca757b910316 Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 11:23:10 +0200 Subject: [PATCH 04/10] feat(framework): builder.fullLogin()/withLocale; blocking join; typed BotJoin{Denied,Timeout}Exception Goal: expose the S6/LK-18 full-login path to consumers as an opt-in on the bot builder, while keeping the default legacy spawn, so tests can join real-login bots and assert denials/timeouts as typed errors. How this slots into S6/LK-18: - IPlayerBuilder gains withLocale(String) (LK-12; validated non-blank) and a fullLogin() join-mode selector. The default stays LEGACY_SPAWN (flipping the default is a PR2/design decision), and bots().join(...) keeps LEGACY_SPAWN. - The join blocks until joined because the agent withholds the CreatePlayer response until PlayerJoinEvent; no new framework wait loop is needed. Under FULL_LOGIN the wire command carries no UUID (the server derives the offline UUID from the name); the response UUID is adopted as the handle's UUID as today. - New public BotJoinDeniedException / BotJoinTimeoutException; UdsAgentTransport maps the PLAYER_JOIN_DENIED / PLAYER_JOIN_TIMEOUT wire codes to them at the single failure choke point so every other command keeps its existing IllegalStateException behavior. - IFrameworkGatewayView is unchanged (the builder path does not flow through it). - joinMode/locale are threaded builder -> framework -> BotsFacade -> UdsAgentClient; the BotsFacade unit test pins the LEGACY_SPAWN default. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../framework/BotJoinDeniedException.java | 24 ++++++++++++++++++ .../framework/BotJoinTimeoutException.java | 21 ++++++++++++++++ .../lightkeeper/framework/IPlayerBuilder.java | 25 ++++++++++++++++++- .../framework/internal/BotsFacade.java | 11 ++++++-- .../internal/DefaultLightkeeperFramework.java | 6 +++-- .../internal/DefaultPlayerBuilder.java | 24 +++++++++++++++++- .../framework/internal/UdsAgentClient.java | 15 ++++++++--- .../framework/internal/UdsAgentTransport.java | 16 +++++++++--- .../framework/internal/BotsFacadeTest.java | 7 ++++-- 9 files changed, 135 insertions(+), 14 deletions(-) create mode 100644 lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinDeniedException.java create mode 100644 lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinTimeoutException.java diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinDeniedException.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinDeniedException.java new file mode 100644 index 00000000..ac4af6d1 --- /dev/null +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinDeniedException.java @@ -0,0 +1,24 @@ +package nl.pim16aap2.lightkeeper.framework; + +/** + * Thrown when a {@code FULL_LOGIN} bot is refused by the server (whitelist, ban, full server, or a plugin + * denial at pre-login/login). + * + *

The message carries the protocol phase the connection was denied in and the server's kick reason text, + * so consumers can assert on denials as typed errors rather than parsing generic failures. + */ +public class BotJoinDeniedException extends RuntimeException +{ + private static final long serialVersionUID = 1L; + + /** + * Creates a denial exception. + * + * @param message + * Human-readable denial detail, including the phase and the server's kick reason. + */ + public BotJoinDeniedException(String message) + { + super(message); + } +} diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinTimeoutException.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinTimeoutException.java new file mode 100644 index 00000000..f7b1d192 --- /dev/null +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/BotJoinTimeoutException.java @@ -0,0 +1,21 @@ +package nl.pim16aap2.lightkeeper.framework; + +/** + * Thrown when a {@code FULL_LOGIN} bot does not reach the play phase (and fire {@code PlayerJoinEvent}) within + * the agent's bounded timeout. + */ +public class BotJoinTimeoutException extends RuntimeException +{ + private static final long serialVersionUID = 1L; + + /** + * Creates a timeout exception. + * + * @param message + * Human-readable timeout detail. + */ + public BotJoinTimeoutException(String message) + { + super(message); + } +} diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/IPlayerBuilder.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/IPlayerBuilder.java index c3e80751..7285db77 100644 --- a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/IPlayerBuilder.java +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/IPlayerBuilder.java @@ -73,7 +73,30 @@ public interface IPlayerBuilder IPlayerBuilder withPermissions(String... permissions); /** - * Builds and spawns the configured synthetic player. + * Sets the client locale sent during the configuration phase of a full-login join (LK-12). + * + *

Only meaningful with {@link #fullLogin()}; ignored for the default legacy spawn path. + * + * @param locale + * Locale identifier such as {@code "en_us"}; must not be blank. + * @return This builder. + */ + IPlayerBuilder withLocale(String locale); + + /** + * Joins the player through the full vanilla login pipeline over real loopback TCP. + * + *

Behavioral differences from the default spawn: the real {@code AsyncPlayerPreLoginEvent}, + * {@code PlayerLoginEvent}, and {@code PlayerJoinEvent} fire; the server derives the offline UUID from the + * name (any builder UUID is ignored); and the join can be denied (whitelist/ban/full server), surfaced as a + * {@link BotJoinDeniedException}, or time out as a {@link BotJoinTimeoutException}. + * + * @return This builder. + */ + IPlayerBuilder fullLogin(); + + /** + * Builds and joins the configured synthetic player. * * @return The created player handle. */ diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacade.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacade.java index 12a9f868..35ef1ecc 100644 --- a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacade.java +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacade.java @@ -5,6 +5,7 @@ import nl.pim16aap2.lightkeeper.framework.IPlayerBuilder; import nl.pim16aap2.lightkeeper.framework.PlayerHandle; import nl.pim16aap2.lightkeeper.framework.WorldHandle; +import nl.pim16aap2.lightkeeper.protocol.JoinMode; import org.jspecify.annotations.Nullable; import java.util.Objects; @@ -59,6 +60,8 @@ public PlayerHandle join(String name, UUID uuid, WorldHandle world) null, null, null, + null, + JoinMode.LEGACY_SPAWN, null ); final PlayerHandle handle = registerAndWrap(createdPlayer); @@ -91,7 +94,9 @@ PlayerHandle createFromBuilder( @Nullable Double y, @Nullable Double z, @Nullable Double health, - Set permissions) + Set permissions, + JoinMode joinMode, + @Nullable String locale) { final AgentPlayerData createdPlayer = agentClient.createPlayer( name, @@ -101,7 +106,9 @@ PlayerHandle createFromBuilder( y, z, health, - permissions + permissions, + joinMode, + locale ); return registerAndWrap(createdPlayer); } diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFramework.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFramework.java index f1df8a60..208fbced 100644 --- a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFramework.java +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFramework.java @@ -787,8 +787,10 @@ PlayerHandle createPlayerFromBuilder( @Nullable Double y, @Nullable Double z, @Nullable Double health, - java.util.Set permissions) + java.util.Set permissions, + nl.pim16aap2.lightkeeper.protocol.JoinMode joinMode, + @Nullable String locale) { - return botsFacade.createFromBuilder(name, uuid, worldHandle, x, y, z, health, permissions); + return botsFacade.createFromBuilder(name, uuid, worldHandle, x, y, z, health, permissions, joinMode, locale); } } diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultPlayerBuilder.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultPlayerBuilder.java index ffd02c73..5a011828 100644 --- a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultPlayerBuilder.java +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultPlayerBuilder.java @@ -3,6 +3,7 @@ import nl.pim16aap2.lightkeeper.framework.IPlayerBuilder; import nl.pim16aap2.lightkeeper.framework.PlayerHandle; import nl.pim16aap2.lightkeeper.framework.WorldHandle; +import nl.pim16aap2.lightkeeper.protocol.JoinMode; import org.jspecify.annotations.Nullable; import java.util.Arrays; @@ -23,6 +24,8 @@ final class DefaultPlayerBuilder implements IPlayerBuilder private @Nullable Double z; private @Nullable Double health; private final Set permissions = new HashSet<>(); + private JoinMode joinMode = JoinMode.LEGACY_SPAWN; + private @Nullable String locale; DefaultPlayerBuilder(DefaultLightkeeperFramework framework) { @@ -96,6 +99,23 @@ public IPlayerBuilder withPermissions(String... permissions) return this; } + @Override + public IPlayerBuilder withLocale(String locale) + { + final String trimmedLocale = Objects.requireNonNull(locale, "locale may not be null.").trim(); + if (trimmedLocale.isEmpty()) + throw new IllegalArgumentException("locale may not be blank."); + this.locale = trimmedLocale; + return this; + } + + @Override + public IPlayerBuilder fullLogin() + { + this.joinMode = JoinMode.FULL_LOGIN; + return this; + } + @Override public PlayerHandle build() { @@ -113,7 +133,9 @@ public PlayerHandle build() y, z, health, - permissions + permissions, + joinMode, + locale ); } } diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClient.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClient.java index 621c7b1a..7be0c16c 100644 --- a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClient.java +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClient.java @@ -32,6 +32,7 @@ import nl.pim16aap2.lightkeeper.protocol.IAgentResponse; import nl.pim16aap2.lightkeeper.protocol.IsChunkLoaded; import nl.pim16aap2.lightkeeper.protocol.ItemSnapshot; +import nl.pim16aap2.lightkeeper.protocol.JoinMode; import nl.pim16aap2.lightkeeper.protocol.LeftClickBlock; import nl.pim16aap2.lightkeeper.protocol.LoadChunk; import nl.pim16aap2.lightkeeper.protocol.MainWorld; @@ -197,22 +198,30 @@ AgentPlayerData createPlayer( @Nullable Double y, @Nullable Double z, @Nullable Double health, - @Nullable Set permissions) + @Nullable Set permissions, + JoinMode joinMode, + @Nullable String locale) { final String permissionsCsv = permissions == null || permissions.isEmpty() ? null : String.join(",", permissions); + // Under FULL_LOGIN the server derives the offline UUID from the name, so the wire command carries no + // UUID (the protocol rejects a non-null UUID in that mode); LEGACY_SPAWN keeps the caller's UUID. + final UUID commandUuid = joinMode == JoinMode.FULL_LOGIN ? null : uuid; + final CreatePlayer.Command command = new CreatePlayer.Command( nextRequestId(), name, - uuid, + commandUuid, worldName, x, y, z, health, - permissionsCsv + permissionsCsv, + joinMode, + locale ); final CreatePlayer.Response response = send(command); return new AgentPlayerData(response.uuid(), response.name()); diff --git a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentTransport.java b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentTransport.java index f150d792..8a2e1960 100644 --- a/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentTransport.java +++ b/lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentTransport.java @@ -1,5 +1,7 @@ package nl.pim16aap2.lightkeeper.framework.internal; +import nl.pim16aap2.lightkeeper.framework.BotJoinDeniedException; +import nl.pim16aap2.lightkeeper.framework.BotJoinTimeoutException; import nl.pim16aap2.lightkeeper.protocol.AgentErrorCode; import nl.pim16aap2.lightkeeper.protocol.AgentProtocolMapper; import nl.pim16aap2.lightkeeper.protocol.IAgentCommand; @@ -157,10 +159,18 @@ private static void throwWhenRequestFailed(JsonNode root) return; final String wireErrorCode = root.path("errorCode").asString(); - final String displayedErrorCode = AgentErrorCode.fromWireCode(wireErrorCode) - .map(AgentErrorCode::wireCode) - .orElseGet(() -> "UNKNOWN (wire='%s')".formatted(wireErrorCode)); + final AgentErrorCode errorCode = AgentErrorCode.fromWireCode(wireErrorCode).orElse(null); final String errorMessage = root.path("errorMessage").asString(""); + + // Full-login join failures are surfaced as typed framework exceptions so tests can assert on them. + if (errorCode == AgentErrorCode.PLAYER_JOIN_DENIED) + throw new BotJoinDeniedException(errorMessage); + if (errorCode == AgentErrorCode.PLAYER_JOIN_TIMEOUT) + throw new BotJoinTimeoutException(errorMessage); + + final String displayedErrorCode = errorCode != null + ? errorCode.wireCode() + : "UNKNOWN (wire='%s')".formatted(wireErrorCode); throw new IllegalStateException( "Agent request failed. code=%s message=%s".formatted(displayedErrorCode, errorMessage)); } diff --git a/lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacadeTest.java b/lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacadeTest.java index e71fa808..8318e3ae 100644 --- a/lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacadeTest.java +++ b/lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/BotsFacadeTest.java @@ -4,6 +4,7 @@ import nl.pim16aap2.lightkeeper.framework.IPlayerBuilder; import nl.pim16aap2.lightkeeper.framework.PlayerHandle; import nl.pim16aap2.lightkeeper.framework.WorldHandle; +import nl.pim16aap2.lightkeeper.protocol.JoinMode; import nl.pim16aap2.lightkeeper.runtime.RuntimeManifest; import org.junit.jupiter.api.Test; @@ -28,7 +29,8 @@ void join_shouldCreatePlayerRegisterItAndReturnHandle() final UdsAgentClient agentClient = mock(UdsAgentClient.class); final PlayerScopeRegistry playerScopeRegistry = mock(PlayerScopeRegistry.class); final UUID uuid = UUID.randomUUID(); - when(agentClient.createPlayer("lkbot001", uuid, "world", null, null, null, null, null)) + when(agentClient.createPlayer( + "lkbot001", uuid, "world", null, null, null, null, null, JoinMode.LEGACY_SPAWN, null)) .thenReturn(new AgentPlayerData(uuid, "lkbot001")); final DefaultLightkeeperFramework framework = framework(agentClient, playerScopeRegistry); final WorldHandle world = FrameworkHandleFactory.worldHandle(framework, "world"); @@ -50,7 +52,8 @@ void join_shouldDefaultToRandomUuidWhenNotProvided() final PlayerScopeRegistry playerScopeRegistry = mock(PlayerScopeRegistry.class); final UUID generatedUuid = UUID.randomUUID(); when(agentClient.createPlayer( - eq("lkbot002"), any(UUID.class), eq("world"), isNull(), isNull(), isNull(), isNull(), isNull())) + eq("lkbot002"), any(UUID.class), eq("world"), isNull(), isNull(), isNull(), isNull(), isNull(), + eq(JoinMode.LEGACY_SPAWN), isNull())) .thenReturn(new AgentPlayerData(generatedUuid, "lkbot002")); final DefaultLightkeeperFramework framework = framework(agentClient, playerScopeRegistry); final WorldHandle world = FrameworkHandleFactory.worldHandle(framework, "world"); From cb8b242aa15e79c2d58c6363902c0409b442351e Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 11:50:44 +0200 Subject: [PATCH 05/10] fix(nms): send unit packets as codec singletons; advance phase before protocol switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Goal: make the S6/LK-18 full-login driver actually complete the pipeline on both Paper and Spigot. Both fixes were surfaced by the mandated two-distro IT gate (A3/A4), each as a named, local failure — the fail-loud policy working as designed. How this slots into S6/LK-18: - ServerboundLoginAcknowledged/FinishConfiguration/AcceptCodeOfConduct are codec singletons: the IdDispatchCodec encodes them by identity against the registered INSTANCE, so a freshly constructed instance fails to encode ("Can't encode ..., expected ..."). Resolve and send the INSTANCE field (by type, since Spigot obfuscates its name). - setupInboundProtocol validates that the listener's protocol() already matches the protocol it is being switched to, so the Proxy must report the new phase before the transition. Advance phase to CONFIGURATION/PLAY before calling enterConfigurationPhase/ enterPlayPhase (was: after). - Surface the pipeline's root-cause message on the wire so future driver gaps are named at the framework boundary, not only in the server log. Verified: the two full-login smoke tests pass on both the Paper and Spigot integration lanes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../nms/v121r7/BotLoginReflection.java | 25 +++++++++++++------ .../nms/v121r7/BotLoginSession.java | 19 +++++++++++--- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java index 0e3d8115..6f76cc86 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java @@ -232,11 +232,14 @@ enum PacketKind "net.minecraft.network.protocol.login.ServerboundHelloPacket", "net.minecraft.network.protocol.login.PacketLoginInStart"); helloPacketConstructor = helloClass.getConstructor(String.class, UUID.class); - loginAcknowledgedPacket = newNoArgPacket( + // These are codec singletons: the IdDispatchCodec encodes them by identity against the registered + // INSTANCE, so a freshly constructed instance fails to encode. Resolve the INSTANCE field by type + // (its name is obfuscated on Spigot). + loginAcknowledgedPacket = resolveSingletonInstance( "net.minecraft.network.protocol.login.ServerboundLoginAcknowledgedPacket"); - finishConfigurationPacket = newNoArgPacket( + finishConfigurationPacket = resolveSingletonInstance( "net.minecraft.network.protocol.configuration.ServerboundFinishConfigurationPacket"); - acceptCodeOfConductPacket = newNoArgPacket( + acceptCodeOfConductPacket = resolveSingletonInstance( "net.minecraft.network.protocol.configuration.ServerboundAcceptCodeOfConductPacket"); selectKnownPacksConstructor = resolveFirst( "net.minecraft.network.protocol.configuration.ServerboundSelectKnownPacks") @@ -626,13 +629,19 @@ private static Method resolveConnectToServer(Class connectionClass, Class + connectionClass.getName() + "."); } - private Object newNoArgPacket(String className) + private Object resolveSingletonInstance(String className) throws ReflectiveOperationException { - final Constructor constructor = - NmsReflectionUtils.resolveClass(className, serverClassLoader).getDeclaredConstructor(); - constructor.setAccessible(true); - return constructor.newInstance(); + final Class packetType = NmsReflectionUtils.resolveClass(className, serverClassLoader); + for (final Field field : packetType.getDeclaredFields()) + { + if (!Modifier.isStatic(field.getModifiers()) || !packetType.equals(field.getType())) + continue; + field.setAccessible(true); + return Objects.requireNonNull(field.get(null), "Singleton field on " + className + " was null."); + } + throw new NoSuchFieldException( + "No static singleton instance field of type " + className + " found on itself."); } private ProtocolInfos resolveProtocolInfos(Object minecraftServer) diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java index 19240fd1..bffee083 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java @@ -88,7 +88,8 @@ IBotLoginOutcome run(BotLoginRequest request) { reflection.closeConnection(openedConnection); throw new IllegalStateException( - "Full-login pipeline failed for bot '%s'.".formatted(request.name()), exception); + "Full-login pipeline failed for bot '%s': %s".formatted(request.name(), rootMessage(exception)), + exception); } catch (InterruptedException exception) { @@ -163,6 +164,16 @@ else if (parameterTypes.length == 1) return args == null || args.length == 0 ? null : args[0]; } + private static String rootMessage(Throwable throwable) + { + Throwable cursor = throwable; + // Depth-capped so a self-referential cause chain cannot loop forever. + for (int depth = 0; depth < 32 && cursor.getCause() != null; ++depth) + cursor = cursor.getCause(); + final String message = cursor.getMessage(); + return message != null ? message : cursor.getClass().getName(); + } + private void dispatchPacket(@Nullable Object packet) { if (packet == null || outcome.isDone()) @@ -176,15 +187,17 @@ private void dispatchPacket(@Nullable Object packet) case COMPRESSION -> reflection.applyCompression(connection(), packet); case LOGIN_FINISHED -> { - reflection.enterConfigurationPhase(connection(), proxy(), locale); + // Advance the phase before the transition: setupInboundProtocol validates that the listener's + // protocol() already matches the configuration protocol it is being switched to. phase = BotJoinPhase.CONFIGURATION; + reflection.enterConfigurationPhase(connection(), proxy(), locale); } case KNOWN_PACKS -> reflection.echoKnownPacks(connection(), packet); case CODE_OF_CONDUCT -> reflection.acceptCodeOfConduct(connection()); case FINISH_CONFIGURATION -> { - reflection.enterPlayPhase(connection(), proxy()); phase = BotJoinPhase.PLAY; + reflection.enterPlayPhase(connection(), proxy()); } case KEEP_ALIVE -> reflection.answerKeepAlive(connection(), packet); case PING -> reflection.answerPing(connection(), packet); From c6f9dd6eb4f85934555c5020a868cd870e0f33c5 Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 11:50:44 +0200 Subject: [PATCH 06/10] test(integration): full-login smoke tests on Paper + Spigot (two-distro gate) Goal: prove the S6/LK-18 FULL_LOGIN driver end-to-end on both distributions and guard the default legacy-spawn path, satisfying the A3/A4 two-distro-before-merge gate. How this slots into S6/LK-18: - fullLogin_shouldJoinThroughRealLoginPipelineOnBothDistros: builder .fullLogin() join; registers AsyncPlayerPreLoginEvent + PlayerJoinEvent captures BEFORE joining and asserts both fired (real login), and asserts the handle carries the server-derived offline UUID. - legacySpawn_shouldStillSpawnBotsAfterDriverIntroduction: regression guard that the default builder path still spawns a usable, driveable bot after the driver's introduction. - Both run automatically on the integration-paper and integration-spigot Failsafe lanes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../maven/test/LightkeeperFullLoginIT.java | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java diff --git a/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java b/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java new file mode 100644 index 00000000..5610413f --- /dev/null +++ b/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java @@ -0,0 +1,81 @@ +package nl.pim16aap2.lightkeeper.maven.test; + +import nl.pim16aap2.lightkeeper.framework.ILightkeeperFramework; +import nl.pim16aap2.lightkeeper.framework.LightkeeperExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * PR1 smoke tests for the FULL_LOGIN bot driver, run on both the Paper and Spigot integration lanes. + * + *

These prove the reflective login pipeline (LK-18) end-to-end on both distributions and guard the + * default legacy-spawn path against regressions from the driver's introduction. + */ +@ExtendWith(LightkeeperExtension.class) +class LightkeeperFullLoginIT +{ + private static UUID offlineUuid(String name) + { + return UUID.nameUUIDFromBytes(("OfflinePlayer:" + name).getBytes(StandardCharsets.UTF_8)); + } + + @Test + void fullLogin_shouldJoinThroughRealLoginPipelineOnBothDistros(ILightkeeperFramework framework) + { + // setup — register the event captures BEFORE joining so the login events cannot be missed. + final var world = framework.worlds().main(); + final String name = "lkfulllogin"; + + try (var preLoginCapture = + framework.events().capture("org.bukkit.event.player.AsyncPlayerPreLoginEvent"); + var joinCapture = + framework.events().capture("org.bukkit.event.player.PlayerJoinEvent")) + { + // execute — a full-login join drives the real handshake/login/configuration/play pipeline. + final var player = framework.bots().builder() + .withName(name) + .atSpawn(world) + .withLocale("en_us") + .fullLogin() + .build(); + + // verify — the bot exists with the server-derived offline UUID (FULL_LOGIN ignores any builder UUID). + assertThat(player.name()).isEqualTo(name); + assertThat(player.uniqueId()).isEqualTo(offlineUuid(name)); + + // Both the async pre-login and the (main-thread) join events must have fired for a real login. + framework.waitUntil(() -> !joinCapture.getCapturedEvents().isEmpty(), Duration.ofSeconds(10)); + assertThat(joinCapture.getCapturedEvents()).isNotEmpty(); + assertThat(preLoginCapture.getCapturedEvents()).isNotEmpty(); + + player.remove(); + } + } + + @Test + void legacySpawn_shouldStillSpawnBotsAfterDriverIntroduction(ILightkeeperFramework framework) + { + // setup + final var world = framework.worlds().main(); + final String name = "lklegacyspawn"; + + // execute — the default builder path stays LEGACY_SPAWN (no fullLogin()). + final var player = framework.bots().builder() + .withName(name) + .atSpawn(world) + .build(); + + // verify — the legacy reflective spawn still produces a usable, driveable bot. + assertThat(player.name()).isEqualTo(name); + assertThat(player.uniqueId()).isNotNull(); + player.teleport(world, 0, 101, 0); + + player.remove(); + } +} From 15f922776ca9b363df39c61adcaae3807a9822d1 Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 11:50:44 +0200 Subject: [PATCH 07/10] docs: document full-login bots (real login events, offline-UUID derivation, denials) Goal: document the S6/LK-18 FULL_LOGIN behavior consumers now opt into via builder.fullLogin()/withLocale, per PR1 scope. How this slots into S6/LK-18: - New "Full-login bots" README section covering the three behavioral differences from the default spawn: real AsyncPreLogin/Login/Join events fire, the offline UUID is server-derived from the name (builder UUID ignored), and joins can be denied (BotJoinDeniedException) or time out (BotJoinTimeoutException); the call blocks until joined. Notes the offline-mode / proxy-forwarding-off requirement. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- README.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/README.md b/README.md index 9d67fbda..5d13067f 100644 --- a/README.md +++ b/README.md @@ -263,6 +263,39 @@ class MyPluginIT - Server directory access (`server().directory()`, `server().pluginDataDirectory(name)`) for seeding files while the server is stopped +### Full-login bots + +By default, `bots().join(...)` and `bots().builder()...build()` spawn a synthetic player through a fast +internal path that fires `PlayerJoinEvent` but skips the login handshake. For tests that need a *real* login, +call `.fullLogin()` on the builder: + +```java +final PlayerHandle bot = framework.bots().builder() + .withName("realbot") + .atSpawn(framework.worlds().main()) + .withLocale("en_us") // optional client locale (LK-12), sent during the configuration phase + .fullLogin() + .build(); +``` + +A full-login bot connects over a real loopback TCP connection and drives the entire vanilla login pipeline +(handshake → login → the 1.20.2+ configuration phase → play), so it behaves like a genuine client. This differs +from the default spawn in three ways: + +- **Real login events fire.** `AsyncPlayerPreLoginEvent`, `PlayerLoginEvent`, and `PlayerJoinEvent` all fire, in + each platform's own order. This is what makes permission plugins such as LuckPerms (which load a user at + pre-login and inject their `Permissible` at login) behave correctly for the bot. +- **The offline UUID is server-derived.** The server derives the bot's UUID from its name + (`UUID.nameUUIDFromBytes("OfflinePlayer:")`); any UUID passed to the builder is ignored under + `fullLogin()`. The returned handle carries the server-assigned UUID. +- **The join can be denied.** Because it is a genuine login, a full-login bot is subject to the whitelist, bans, + the max-player limit, and plugin denials. A denial is surfaced as a `BotJoinDeniedException` (carrying the + server's kick reason); a login that does not complete in time throws a `BotJoinTimeoutException`. The call + blocks until the bot has fully joined (or is denied/times out). + +Full-login joins require the server to run in offline mode with proxy forwarding off (both are the provisioner's +defaults). + ## World and Plugin Provisioning `prepare-server` supports custom worlds and plugins in plugin configuration: From 4895836e3ed52b4be82ff62f31b751bea5d4d6fc Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 12:10:20 +0200 Subject: [PATCH 08/10] fix(nms): mark gameContextInvoke return @Nullable for NullAway (-Perrorprone) Goal: keep the S6/LK-18 driver clean under the errorprone/NullAway profile, which only runs under -Perrorprone. How this slots into S6/LK-18: the synthesized GameProtocols.Context InvocationHandler returns null for unhandled methods, so its return type must be @Nullable under the module's @NullMarked package. Surfaced by the -Perrorprone verify lane. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java index 6f76cc86..90bb4176 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java @@ -708,7 +708,7 @@ private ProtocolInfos resolveProtocolInfos(Object minecraftServer) } @SuppressWarnings({"PMD.UseVarargs", "PMD.CompareObjectsWithEquals"}) // InvocationHandler contract; proxy identity. - private static Object gameContextInvoke(Object proxy, Method method, Object @Nullable [] args) + private static @Nullable Object gameContextInvoke(Object proxy, Method method, Object @Nullable [] args) { if (method.getReturnType() == boolean.class) return Boolean.FALSE; From dafc2d9b59c8bfce8559bdebd37b90d8c51e5bab Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 12:59:45 +0200 Subject: [PATCH 09/10] fix: address pre-PR deep-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BLOCKER 1: BotLoginSession no longer stops servicing packets when the outcome completes — that dropped every post-join KEEP_ALIVE/PING/TELEPORT, so the server's keep-alive watchdog kicked FULL_LOGIN bots shortly after joining. Packet dispatch is now gated on a volatile 'terminated' flag set only on denial, disconnect, transport close, timeout, or an internal servicing failure; after a successful join the session keeps answering keep-alives/pings and acknowledging teleports for the connection's lifetime. BLOCKER 1b (found by the new keep-alive IT on Paper): Paper arms its inbound packet-rate limiter (Connection.allPacketCounts) on every Connection instance, including the bot's own client-side one, which legitimately receives thousands of clientbound packets per second (chunks, entity movement) — so the bot's connection killed ITSELF for "packet spam" ~7s after joining (killForPacketSpam; the tell was the client trying to encode a clientbound disconnect through its serverbound outbound codec). The driver now disarms the limiter on its own client connection (nulls the counter field, resolved by type) before the handshake; the server's per-player connection and its protections are a separate instance and remain untouched. No-op on Spigot. MAJOR 2: handleFullLoginPlayer no longer stacks two full sync-operation timeouts (driver login + join-event wait ~ 240s worst case, past the 150s client watchdog). Both waits now share ONE deadline bounded by syncOperationTimeoutSeconds: the join latch only gets the residual time, preserving the RuntimeProtocol margin invariant (agent times out first). MINOR 3: BotLoginSession.run() closes the connection when sendHello fails after openConnection succeeded (terminate() now covers every terminal path except Joined). MINOR 5: handleFullLoginPlayer fails fast with IllegalStateException when invoked on the server main thread (the driver + join latch would deadlock the very loop that must fire PlayerJoinEvent). MINOR 6: (a) new IT fullLogin_shouldSurviveKeepAliveCycleWhileIdle — full-login bot idles 35s (a full keep-alive interval + kick window) and must not fire PlayerQuitEvent, then still accept a teleport; runs on both Paper and Spigot lanes and caught both BLOCKER 1 and BLOCKER 1b. (b) unit tests for the TimedOut->PLAYER_JOIN_TIMEOUT mapping and the login-ok-but-no-PlayerJoinEvent residual-timeout branch (short-budget fixture, inline-scheduler Bukkit statics); the denial test reworked off the main thread to match the MINOR 5 guard. NIT 7: BotJoinPhase javadoc now links IBotLoginOutcome.Denied. NIT 8: NmsReflectionUtils.resolveFieldByNameOrAcceptedType's fallback skips static fields and Object-typed fields so the Connection channel-field resolution rests on a meaningful type match, not declaration order. DEFERRED (documented only): one-line comments on the resource-pack and cookie-request CONSUME entries noting they are consumed without a response (PR2 provisioner territory). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../agent/spigot/AgentPlayerActions.java | 18 +++- .../agent/spigot/AgentPlayerActionsTest.java | 92 +++++++++++++++++-- .../maven/test/LightkeeperFullLoginIT.java | 29 ++++++ .../lightkeeper/nms/api/BotJoinPhase.java | 2 +- .../nms/v121r7/BotLoginReflection.java | 48 ++++++++++ .../nms/v121r7/BotLoginSession.java | 60 ++++++++++-- .../nms/v121r7/NmsReflectionUtils.java | 8 +- 7 files changed, 237 insertions(+), 20 deletions(-) diff --git a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java index a449d30b..cded56f5 100644 --- a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java +++ b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java @@ -170,6 +170,10 @@ private CreatePlayer.Response handleLegacySpawnPlayer(CreatePlayer.Command comma * the server main thread — while awaiting the {@code PlayerJoinEvent} for the bot's name. Whitelist/ban/full/ * plugin denials and timeouts are surfaced as typed {@link AgentProtocolException}s. * + *

The driver wait and the join-event wait share ONE deadline bounded by the agent's sync-operation + * timeout, preserving the {@code RuntimeProtocol} invariant that the agent reports its own detailed + * {@code TIMEOUT} before the client watchdog (timeout + margin) gives up. + * * @param command * Typed create-player command in {@code FULL_LOGIN} mode. * @return Response containing the server-assigned offline UUID and name. @@ -180,11 +184,18 @@ private CreatePlayer.Response handleLegacySpawnPlayer(CreatePlayer.Command comma private CreatePlayer.Response handleFullLoginPlayer(CreatePlayer.Command command) throws Exception { + if (Bukkit.isPrimaryThread()) + throw new IllegalStateException( + "FULL_LOGIN joins must not run on the server main thread: the join blocks on PlayerJoinEvent, " + + "which fires on the main thread, so this would deadlock."); + final String name = command.name(); final String locale = command.locale(); final Double health = command.health(); final String permissionsCsv = command.permissionsCsv(); final long timeoutSeconds = mainThreadExecutor.syncOperationTimeoutSeconds(); + // One shared deadline for the whole FULL_LOGIN join (driver + join-event wait). + final long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(timeoutSeconds); final CountDownLatch joinLatch = new CountDownLatch(1); final AtomicReference joinedPlayer = new AtomicReference<>(); @@ -221,10 +232,13 @@ private CreatePlayer.Response handleFullLoginPlayer(CreatePlayer.Command command AgentErrorCode.PLAYER_JOIN_TIMEOUT, "Bot '%s' did not complete the login pipeline within %d seconds.".formatted(name, timeoutSeconds)); - if (!joinLatch.await(timeoutSeconds, TimeUnit.SECONDS)) + // The join event gets only the time remaining on the shared deadline, never a fresh budget: the + // driver has already reached the play phase, so the event wait is normally near-instant. + final long remainingNanos = deadlineNanos - System.nanoTime(); + if (remainingNanos <= 0L || !joinLatch.await(remainingNanos, TimeUnit.NANOSECONDS)) throw new AgentProtocolException( AgentErrorCode.PLAYER_JOIN_TIMEOUT, - "Bot '%s' completed login but no PlayerJoinEvent fired within %d seconds." + "Bot '%s' completed login but no PlayerJoinEvent fired within the %d-second join budget." .formatted(name, timeoutSeconds)); final Player player = Objects.requireNonNull(joinedPlayer.get(), "joined player"); diff --git a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java index 7bc529bd..631af8c5 100644 --- a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java +++ b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java @@ -36,12 +36,15 @@ import org.bukkit.permissions.PermissionAttachment; import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.java.JavaPlugin; +import org.bukkit.scheduler.BukkitScheduler; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; import java.util.List; import java.util.UUID; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; import static org.assertj.core.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; @@ -594,18 +597,13 @@ void handleCreatePlayer_shouldSurfaceFullLoginDenialAsTypedError() final IBotLoginDriver loginDriver = mock(); when(fixture.nmsAdapter().loginDriver()).thenReturn(loginDriver); when(loginDriver.login(any())).thenReturn(new IBotLoginOutcome.Denied(BotJoinPhase.LOGIN, "Banned")); - final org.bukkit.Server server = mock(); - when(server.getPort()).thenReturn(25_565); - final PluginManager pluginManager = mock(); final CreatePlayer.Command command = new CreatePlayer.Command( "request-full", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, "en_us"); // execute + verify try (MockedStatic bukkit = mockStatic(Bukkit.class)) { - bukkit.when(Bukkit::isPrimaryThread).thenReturn(true); - bukkit.when(Bukkit::getServer).thenReturn(server); - bukkit.when(Bukkit::getPluginManager).thenReturn(pluginManager); + stubFullLoginBukkitStatics(bukkit); assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) .isInstanceOf(AgentProtocolException.class) @@ -615,6 +613,78 @@ void handleCreatePlayer_shouldSurfaceFullLoginDenialAsTypedError() } } + @Test + void handleCreatePlayer_shouldSurfaceFullLoginDriverTimeoutAsTypedError() + { + // setup + final PlayerActionsFixture fixture = createPlayerActionsFixture(); + final IBotLoginDriver loginDriver = mock(); + when(fixture.nmsAdapter().loginDriver()).thenReturn(loginDriver); + when(loginDriver.login(any())).thenReturn(new IBotLoginOutcome.TimedOut()); + final CreatePlayer.Command command = new CreatePlayer.Command( + "request-full-to", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, null); + + // execute + verify + try (MockedStatic bukkit = mockStatic(Bukkit.class)) + { + stubFullLoginBukkitStatics(bukkit); + + assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) + .isInstanceOf(AgentProtocolException.class) + .satisfies(thrown -> assertThat(((AgentProtocolException) thrown).errorCode()) + .isEqualTo(AgentErrorCode.PLAYER_JOIN_TIMEOUT)) + .hasMessageContaining("did not complete the login pipeline"); + } + } + + @Test + void handleCreatePlayer_shouldSurfaceMissingJoinEventAsTypedTimeout() + { + // setup — a short 1-second budget so the join-latch residual wait expires quickly. The driver reports + // Joined, but no PlayerJoinEvent ever fires (the mocked plugin manager registers nothing). + final PlayerActionsFixture fixture = createPlayerActionsFixture(1L); + final IBotLoginDriver loginDriver = mock(); + when(fixture.nmsAdapter().loginDriver()).thenReturn(loginDriver); + when(loginDriver.login(any())).thenReturn(new IBotLoginOutcome.Joined("fullbot")); + final CreatePlayer.Command command = new CreatePlayer.Command( + "request-full-nj", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, null); + + // execute + verify + try (MockedStatic bukkit = mockStatic(Bukkit.class)) + { + stubFullLoginBukkitStatics(bukkit); + + assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) + .isInstanceOf(AgentProtocolException.class) + .satisfies(thrown -> assertThat(((AgentProtocolException) thrown).errorCode()) + .isEqualTo(AgentErrorCode.PLAYER_JOIN_TIMEOUT)) + .hasMessageContaining("no PlayerJoinEvent fired"); + } + } + + /** + * Stubs the Bukkit statics a FULL_LOGIN join touches: NOT the primary thread (the handler rejects + * main-thread execution), a server exposing a port, a no-op plugin manager, and a scheduler that runs + * main-thread callables inline. + */ + private static void stubFullLoginBukkitStatics(MockedStatic bukkit) + { + final org.bukkit.Server server = mock(); + when(server.getPort()).thenReturn(25_565); + final PluginManager pluginManager = mock(); + final BukkitScheduler scheduler = mock(); + when(scheduler.callSyncMethod(any(), any())).thenAnswer(invocation -> + { + final Callable callable = invocation.getArgument(1); + return CompletableFuture.completedFuture(callable.call()); + }); + + bukkit.when(Bukkit::isPrimaryThread).thenReturn(false); + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getPluginManager).thenReturn(pluginManager); + bukkit.when(Bukkit::getScheduler).thenReturn(scheduler); + } + @Test void handlePlayerChat_shouldInvokePlayerChatOnMainThread() throws Exception @@ -663,10 +733,18 @@ private static AgentPlayerActions createPlayerActions() } private static PlayerActionsFixture createPlayerActionsFixture() + { + return createPlayerActionsFixture(null); + } + + private static PlayerActionsFixture createPlayerActionsFixture( + @org.jspecify.annotations.Nullable Long syncOperationTimeoutSeconds) { final JavaPlugin plugin = mock(); when(plugin.getLogger()).thenReturn(java.util.logging.Logger.getLogger("test")); - final AgentMainThreadExecutor mainThreadExecutor = new AgentMainThreadExecutor(plugin); + final AgentMainThreadExecutor mainThreadExecutor = syncOperationTimeoutSeconds == null + ? new AgentMainThreadExecutor(plugin) + : new AgentMainThreadExecutor(plugin, syncOperationTimeoutSeconds); final AgentSyntheticPlayerStore playerStore = new AgentSyntheticPlayerStore(); final ObjectMapper objectMapper = new ObjectMapper(); final IBotPlayerNmsAdapter nmsAdapter = mock(); diff --git a/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java b/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java index 5610413f..0791a7e3 100644 --- a/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java +++ b/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java @@ -58,6 +58,35 @@ void fullLogin_shouldJoinThroughRealLoginPipelineOnBothDistros(ILightkeeperFrame } } + @Test + void fullLogin_shouldSurviveKeepAliveCycleWhileIdle(ILightkeeperFramework framework) + { + // setup — capture quit events BEFORE joining so a keep-alive kick cannot slip past the assertion. + final var world = framework.worlds().main(); + final String name = "lkkeepalive"; + + try (var quitCapture = framework.events().capture("org.bukkit.event.player.PlayerQuitEvent")) + { + final var player = framework.bots().builder() + .withName(name) + .atSpawn(world) + .fullLogin() + .build(); + + // execute — idle past a full keep-alive interval (15s) plus the server's pending-kick window, + // during which the driver must keep answering keep-alives for the already-joined bot. + final long idleDeadlineNanos = System.nanoTime() + Duration.ofSeconds(35).toNanos(); + framework.waitUntil(() -> System.nanoTime() - idleDeadlineNanos >= 0, Duration.ofSeconds(60)); + + // verify — the bot was not kicked while idle and still responds to interactions (the teleport's + // position packet must be acknowledged by the driver post-join). + assertThat(quitCapture.getCapturedEvents()).isEmpty(); + player.teleport(world, 1, 101, 1); + + player.remove(); + } + } + @Test void legacySpawn_shouldStillSpawnBotsAfterDriverIntroduction(ILightkeeperFramework framework) { diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java index 02a4aefd..19726bf4 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/BotJoinPhase.java @@ -3,7 +3,7 @@ /** * Protocol phase a {@link IBotLoginDriver} was in when a full-login join was denied. * - *

Surfaced on {@link BotLoginOutcome.Denied} so callers can distinguish, for example, a whitelist/ban + *

Surfaced on {@link IBotLoginOutcome.Denied} so callers can distinguish, for example, a whitelist/ban * kick during login from a kick during the configuration or play phase. */ public enum BotJoinPhase diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java index 90bb4176..28cf30a4 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java @@ -107,6 +107,13 @@ enum PacketKind private final Method setupCompressionMethod; private final Field channelField; private final Method channelCloseMethod; + /** + * Paper's per-connection inbound packet-rate counter ({@code Connection.allPacketCounts}), or {@code null} + * on distributions without it (Spigot). Paper initializes it from config on EVERY {@code Connection} — + * including our client-side one, which legitimately receives thousands of packets per second (chunks, + * entity movement), so left armed it kills the bot's own connection for "packet spam" shortly after join. + */ + private final @Nullable Field paperPacketLimiterField; private final Object configurationClientboundInfo; private final Object configurationServerboundInfo; @@ -219,6 +226,7 @@ enum PacketKind channelField = NmsReflectionUtils.resolveFieldByNameOrAcceptedType( connectionClass, "channel", channelClass); channelCloseMethod = channelClass.getMethod("close"); + paperPacketLimiterField = resolvePaperPacketLimiterField(connectionClass); // Bound protocol infos for the configuration and play phases. final ProtocolInfos protocolInfos = resolveProtocolInfos(minecraftServer); @@ -298,10 +306,48 @@ Object openConnection(int port, Object listenerProxy) { final InetSocketAddress address = new InetSocketAddress(InetAddress.getLoopbackAddress(), port); final Object connection = connectToServerMethod.invoke(null, address, eventLoopGroupHolder, null); + disarmPaperPacketLimiter(connection); initiateLoginConnectionMethod.invoke(connection, "127.0.0.1", port, listenerProxy); return connection; } + /** + * Disarms Paper's inbound packet-rate limiter on the bot's own client-side connection. + * + *

Paper arms {@code Connection.allPacketCounts} from server config on every {@code Connection} instance, + * client-side ones included. The bot's connection legitimately receives thousands of clientbound packets per + * second (chunk data, entity movement), so an armed limiter makes the connection kill itself for + * "packet spam" within seconds of joining. Nulling the counter only affects this bot's client connection; + * the server's per-player connection (and its protections) is a separate instance and stays untouched. + * Called before the handshake is initiated, so no inbound packet can race the write. No-op on Spigot. + * + * @param connection + * The freshly created client connection. + * @throws ReflectiveOperationException + * When clearing the counter field fails. + */ + private void disarmPaperPacketLimiter(Object connection) + throws ReflectiveOperationException + { + if (paperPacketLimiterField != null) + paperPacketLimiterField.set(connection, null); + } + + private @Nullable Field resolvePaperPacketLimiterField(Class connectionClass) + { + try + { + final Class counterClass = NmsReflectionUtils.resolveClass( + "io.papermc.paper.util.IntervalledCounter", serverClassLoader); + return NmsReflectionUtils.findFieldByType(connectionClass, counterClass); + } + catch (ClassNotFoundException ignored) + { + // Not a Paper server; there is no packet limiter to disarm. + return null; + } + } + /** * Sends the login-start (hello) packet that begins the login exchange. * @@ -577,6 +623,7 @@ private Set> resolveConsumeClasses() "net.minecraft.network.protocol.configuration.ClientboundResetChatPacket", "net.minecraft.network.protocol.common.ClientboundCustomPayloadPacket", "net.minecraft.network.protocol.common.ClientboundUpdateTagsPacket", + // Limitation: consumed without a response — a server that requires a resource-pack answer stalls (PR2). "net.minecraft.network.protocol.common.ClientboundResourcePackPushPacket", "net.minecraft.network.protocol.common.ClientboundResourcePackPopPacket", "net.minecraft.network.protocol.common.ClientboundStoreCookiePacket", @@ -585,6 +632,7 @@ private Set> resolveConsumeClasses() "net.minecraft.network.protocol.common.ClientboundServerLinksPacket", "net.minecraft.network.protocol.common.ClientboundClearDialogPacket", "net.minecraft.network.protocol.common.ClientboundShowDialogPacket", + // Limitation: consumed without a cookie response — a server that awaits one stalls the join (PR2). "net.minecraft.network.protocol.cookie.ClientboundCookieRequestPacket"}) { addOptional(classes, className); diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java index bffee083..b7b0d6aa 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java @@ -25,6 +25,11 @@ *

Dispatch keys on the packet class ({@code method.getParameterTypes()[0]}), never the method name, * because Spigot obfuscates every listener method to {@code a}. Unhandled packets are fail-loud in the * login/configuration phase (they would silently stall the join) and warn-once in the play phase. + * + *

Packet servicing outlives the {@code Joined} outcome: once the play phase is reached, the session keeps + * answering keep-alives/pings and acknowledging teleports for the lifetime of the connection — otherwise the + * server's keep-alive watchdog would kick the bot shortly after joining. Servicing stops only when the + * session is {@link #terminated} (denial, disconnect, transport close, or an internal failure). */ final class BotLoginSession implements InvocationHandler { @@ -39,6 +44,12 @@ final class BotLoginSession implements InvocationHandler private volatile @Nullable Object listenerProxy; private volatile @Nullable String locale; private volatile String playerName = ""; + /** + * Set when the connection is no longer live (denied, disconnected, timed out, or failed). Gates packet + * servicing instead of {@code outcome.isDone()}, which flips on a successful join while the connection — + * and its keep-alive obligations — very much lives on. + */ + private volatile boolean terminated; BotLoginSession(BotLoginReflection reflection) { @@ -63,30 +74,40 @@ IBotLoginOutcome run(BotLoginRequest request) try { openedConnection = reflection.openConnection(request.port(), proxy); - this.connection = openedConnection; - reflection.sendHello(openedConnection, request.name()); } catch (ReflectiveOperationException exception) { throw new IllegalStateException( "Failed to open full-login connection for bot '%s'.".formatted(request.name()), exception); } + this.connection = openedConnection; + + try + { + reflection.sendHello(openedConnection, request.name()); + } + catch (ReflectiveOperationException exception) + { + terminate(openedConnection); + throw new IllegalStateException( + "Failed to start login for bot '%s'.".formatted(request.name()), exception); + } try { final IBotLoginOutcome result = outcome.get(request.timeout().toMillis(), TimeUnit.MILLISECONDS); if (!(result instanceof IBotLoginOutcome.Joined)) - reflection.closeConnection(openedConnection); + terminate(openedConnection); return result; } catch (TimeoutException exception) { - reflection.closeConnection(openedConnection); + terminate(openedConnection); return new IBotLoginOutcome.TimedOut(); } catch (ExecutionException exception) { - reflection.closeConnection(openedConnection); + terminate(openedConnection); throw new IllegalStateException( "Full-login pipeline failed for bot '%s': %s".formatted(request.name(), rootMessage(exception)), exception); @@ -94,12 +115,24 @@ IBotLoginOutcome run(BotLoginRequest request) catch (InterruptedException exception) { Thread.currentThread().interrupt(); - reflection.closeConnection(openedConnection); + terminate(openedConnection); throw new IllegalStateException( "Interrupted while awaiting full-login of bot '%s'.".formatted(request.name()), exception); } } + /** + * Stops packet servicing and closes the connection; used on every terminal path except a successful join. + * + * @param liveConnection + * The connection to close. + */ + private void terminate(Object liveConnection) + { + terminated = true; + reflection.closeConnection(liveConnection); + } + // ------------------------------------------------------------------------------------------------------- // InvocationHandler: called by the server's packet dispatch on the Netty event-loop thread. // ------------------------------------------------------------------------------------------------------- @@ -176,7 +209,9 @@ private static String rootMessage(Throwable throwable) private void dispatchPacket(@Nullable Object packet) { - if (packet == null || outcome.isDone()) + // Gate on connection liveness, NOT on outcome completion: after a successful join the outcome is done + // but keep-alives/pings/teleports must keep being serviced or the server's watchdog kicks the bot. + if (packet == null || terminated) return; final Class packetClass = packet.getClass(); @@ -213,8 +248,12 @@ private void dispatchPacket(@Nullable Object packet) } catch (ReflectiveOperationException exception) { - outcome.completeExceptionally(new IllegalStateException( - "Failed to handle %s packet %s.".formatted(phase, packetClass.getName()), exception)); + // A reflective failure means the driver can no longer service the connection reliably; stop. + terminated = true; + final IllegalStateException failure = new IllegalStateException( + "Failed to handle %s packet %s.".formatted(phase, packetClass.getName()), exception); + if (!outcome.completeExceptionally(failure)) + LOG.log(System.Logger.Level.WARNING, "Full-login packet servicing failed after join.", failure); } } @@ -228,6 +267,7 @@ private void handleUnknownPacket(Class packetClass) } // Fail-loud: an unhandled login/configuration packet would silently stall the join. + terminated = true; outcome.completeExceptionally(new IllegalStateException( "Unhandled %s packet from server: %s. Add it to the login driver's packet table." .formatted(phase, packetClass.getName()))); @@ -235,6 +275,7 @@ private void handleUnknownPacket(Class packetClass) private void handleTransportDisconnect() { + terminated = true; if (!outcome.isDone()) outcome.complete(new IBotLoginOutcome.Denied(phase, "Connection closed during " + phase + ".")); } @@ -247,6 +288,7 @@ private void completeJoined() private void completeDenied(Object disconnectPacket) { + terminated = true; if (!outcome.isDone()) outcome.complete(new IBotLoginOutcome.Denied(phase, reflection.disconnectReason(disconnectPacket))); } diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java index b3919522..3f4a0913 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java @@ -190,6 +190,10 @@ static Field resolveFieldByNameOrType(Class ownerClass, String preferredName, * not found. Unlike {@link #resolveFieldByNameOrType}, the fallback checks whether the field's type is * assignable from {@code acceptedType} (i.e., the field is a supertype of the accepted type). * + *

The fallback skips static fields and fields declared as {@code Object}: an {@code Object}-typed field is + * a supertype of everything, so accepting it would make resolution depend on declaration order instead of on + * a meaningful type match. + * * @param ownerClass * Class to search (including superclass chain). * @param preferredName @@ -220,7 +224,9 @@ static Field resolveFieldByNameOrAcceptedType( { for (final Field field : cursor.getDeclaredFields()) { - if (!field.getType().isAssignableFrom(acceptedType)) + if (Modifier.isStatic(field.getModifiers())) + continue; + if (field.getType() == Object.class || !field.getType().isAssignableFrom(acceptedType)) continue; field.setAccessible(true); return field; From 1a5c92f0fbf2114c361c16fa3e3a8ec1b7215e89 Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Wed, 15 Jul 2026 17:42:23 +0200 Subject: [PATCH 10/10] fix: address bot round-1 feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1 (Codex P1): resolveEnumConstant hardened with a documented ordinal fallback. Declaration orders verified against BOTH 1.21.11 jars (javap on Paper; runtime probe on Spigot, whose enum FIELDS are obfuscated to a/b.. but whose Enum.name() strings are preserved): PacketFlow SERVERBOUND=0, CLIENTBOUND=1; ConnectionProtocol HANDSHAKING=0, PLAY=1, STATUS=2, LOGIN=3, CONFIGURATION=4. Fallback logs at DEBUG; out-of-range ordinal still fails loud. 2 (Codex P2 + CodeRabbit 3586962018): FULL_LOGIN now honors the command's placement. The wire sends worldName always and x/y/z only from the builder's atLocation, so the post-join main-thread registration teleports to (world,x,y,z) when coordinates are present and to the world's spawn otherwise; the world is validated up front (before any connection) in the listener-registration callable. The join smoke IT now joins at an explicit location and asserts the position via the entity-query API on both lanes. 3 (CodeRabbit 3586962026): every blocking phase draws from the ONE shared deadline. The finally-block listener cleanup is bounded to clamp(remaining, 2s, 10s) via a new explicit- timeout callOnMainThread overload, and can no longer mask the primary failure: a cleanup exception is addSuppressed onto it when one exists and logged otherwise. 4 (CodeRabbit 3586962030): joinLatch.await interruption now re-interrupts and surfaces as AgentProtocolException(INTERRUPTED), matching AgentMainThreadExecutor's contract. 5 (CodeRabbit 3586962045): openConnection closes the freshly opened TCP connection when limiter disarming or handshake initiation throws after connectToServer succeeded. 6 (CodeRabbit 3586962058): reply-required packets now get vanilla-client answers instead of being consumed: login custom query -> ServerboundCustomQueryAnswerPacket(txId, null) ("unknown"); cookie request -> ServerboundCookieResponsePacket(key, null) (absent payload); resource-pack push -> DECLINED (a require-pack server then kicks, surfacing as a typed denial). Constructions verified against both jars; the per-distro cookie key type (Identifier vs MinecraftKey) and the nested action enum ($Action vs $a) are resolved structurally from the response constructors; DECLINED ordinal 1 verified on both. The old CONSUME limitation comments are gone; only the pop (never answered by vanilla) remains consumed. 7 (CodeRabbit 3586962063): packet-servicing failures now catch RuntimeException too (a missing structural accessor throws IllegalStateException), so the outcome can never be left pending; terminate() now ALWAYS closes the channel, including servicing failures after a successful join. 8 (CodeRabbit 3586962041 + 3586962073): the three FULL_LOGIN agent tests and the enum helper test use strict // setup / // execute / // verify blocks (catchThrowable pattern). 9 (Copilot 3586969189): the locale local is annotated @Nullable for clarity. 10 (Copilot 3586969220): AgentMainThreadExecutor's orphaned javadoc reattached; both callOnMainThread overloads and the timeout getter each carry their own doc. 13 (CI, highest priority): the integration-spigot CI failure (fullLogin smoke test burning its full 120s in the driver while the same lane's keep-alive login passed 40s earlier on a warm server) is a silent-close defect, not cold-start slowness. Evidence: channelInactive only RECORDS the disconnection; PacketListener.onDisconnect fires solely from Connection.handleDisconnection(), which is driven by the client/server tick loops — and the driver's client connection is in neither, so a raw remote close (e.g. the server's 30-second ReadTimeoutHandler abandoning a stalled login, verified present in the server pipeline, which sends no disconnect packet) was invisible and the join waited out its whole budget. Fix: a reflective closeFuture listener on the bot's channel surfaces any close immediately as a typed Denied("Connection closed during "), and TimedOut now carries the stalled phase so any future timeout names where the login stopped. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn --- .../agent/spigot/AgentMainThreadExecutor.java | 54 +++- .../agent/spigot/AgentPlayerActions.java | 134 ++++++++-- .../agent/spigot/AgentPlayerActionsTest.java | 38 ++- .../maven/test/LightkeeperFullLoginIT.java | 13 +- .../lightkeeper/nms/api/IBotLoginOutcome.java | 13 +- .../nms/v121r7/BotLoginReflection.java | 230 ++++++++++++++++-- .../nms/v121r7/BotLoginSession.java | 47 ++-- .../nms/v121r7/NmsReflectionUtils.java | 28 ++- .../v121r7/BotLoginReflectionHelpersTest.java | 36 ++- 9 files changed, 496 insertions(+), 97 deletions(-) diff --git a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java index f9991d1a..1a2210ad 100644 --- a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java +++ b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentMainThreadExecutor.java @@ -64,7 +64,22 @@ final class AgentMainThreadExecutor } /** - * Submits the callable to the Bukkit main thread and waits for completion. + * Returns the configured maximum wait, in seconds, for a scheduled synchronous server operation. + * + *

Reused by long-running orchestration (such as awaiting a full-login {@code PlayerJoinEvent}) so a + * single, configurable bound governs how long the agent blocks before reporting a timeout to the client. + * + * @return + * Positive timeout in seconds. + */ + long syncOperationTimeoutSeconds() + { + return syncOperationTimeoutSeconds; + } + + /** + * Submits the callable to the Bukkit main thread and waits for completion, bounded by the configured + * sync-operation timeout. * * @param callable * Operation to execute synchronously on the server thread. @@ -77,23 +92,36 @@ final class AgentMainThreadExecutor * {@link AgentProtocolException} with {@link AgentErrorCode#TIMEOUT} when the operation exceeds the * configured timeout, or {@link AgentErrorCode#INTERRUPTED} when the waiting thread is interrupted. */ + T callOnMainThread(Callable callable) + throws Exception + { + return callOnMainThread(callable, syncOperationTimeoutSeconds); + } + /** - * Returns the configured maximum wait, in seconds, for a scheduled synchronous server operation. + * Submits the callable to the Bukkit main thread and waits for completion, bounded by an explicit timeout. * - *

Reused by long-running orchestration (such as awaiting a full-login {@code PlayerJoinEvent}) so a - * single, configurable bound governs how long the agent blocks before reporting a timeout to the client. + *

Used where a caller must stay inside an outer deadline (such as the full-login listener cleanup) + * instead of drawing a fresh full sync-operation budget. * + * @param callable + * Operation to execute synchronously on the server thread. + * @param timeoutSeconds + * Maximum time to wait for the operation, in seconds; must be positive. + * @param + * Callable return type. * @return - * Positive timeout in seconds. + * Result returned by the callable. + * @throws Exception + * Propagates the callable's own failure (unwrapped from {@link ExecutionException}); throws + * {@link AgentProtocolException} with {@link AgentErrorCode#TIMEOUT} when the operation exceeds the + * given timeout, or {@link AgentErrorCode#INTERRUPTED} when the waiting thread is interrupted. */ - long syncOperationTimeoutSeconds() - { - return syncOperationTimeoutSeconds; - } - - T callOnMainThread(Callable callable) + T callOnMainThread(Callable callable, long timeoutSeconds) throws Exception { + if (timeoutSeconds <= 0L) + throw new IllegalArgumentException("timeoutSeconds must be > 0 but was " + timeoutSeconds + "."); if (Bukkit.isPrimaryThread()) return callable.call(); @@ -101,7 +129,7 @@ T callOnMainThread(Callable callable) final Throwable callableFailure; try { - return future.get(syncOperationTimeoutSeconds, TimeUnit.SECONDS); + return future.get(timeoutSeconds, TimeUnit.SECONDS); } catch (ExecutionException exception) { @@ -115,7 +143,7 @@ T callOnMainThread(Callable callable) future.cancel(true); throw new AgentProtocolException( AgentErrorCode.TIMEOUT, - "Server operation did not complete within %d seconds.".formatted(syncOperationTimeoutSeconds), + "Server operation did not complete within %d seconds.".formatted(timeoutSeconds), exception ); } diff --git a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java index cded56f5..4b493ab8 100644 --- a/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java +++ b/lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java @@ -40,6 +40,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.Level; /** * Protocol action handler for synthetic player lifecycle, movement, and world interactions. @@ -190,11 +191,10 @@ private CreatePlayer.Response handleFullLoginPlayer(CreatePlayer.Command command + "which fires on the main thread, so this would deadlock."); final String name = command.name(); - final String locale = command.locale(); - final Double health = command.health(); - final String permissionsCsv = command.permissionsCsv(); + final @Nullable String locale = command.locale(); final long timeoutSeconds = mainThreadExecutor.syncOperationTimeoutSeconds(); - // One shared deadline for the whole FULL_LOGIN join (driver + join-event wait). + // One shared deadline for the whole FULL_LOGIN join: listener registration (first, so it naturally has + // the full budget), the driver, the join-event wait, and the post-join placement all draw from it. final long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(timeoutSeconds); final CountDownLatch joinLatch = new CountDownLatch(1); @@ -212,11 +212,15 @@ private CreatePlayer.Response handleFullLoginPlayer(CreatePlayer.Command command }; mainThreadExecutor.callOnMainThread(() -> { + // Fail fast on an unknown world before opening any connection. + if (Bukkit.getWorld(command.worldName()) == null) + throw new IllegalArgumentException("World '%s' does not exist.".formatted(command.worldName())); Bukkit.getPluginManager().registerEvent( PlayerJoinEvent.class, listener, EventPriority.MONITOR, executor, plugin); return Boolean.TRUE; }); + Exception primaryFailure = null; try { final int port = Bukkit.getServer().getPort(); @@ -227,56 +231,142 @@ private CreatePlayer.Response handleFullLoginPlayer(CreatePlayer.Command command throw new AgentProtocolException( AgentErrorCode.PLAYER_JOIN_DENIED, "Bot '%s' was denied during %s: %s".formatted(name, denied.phase(), denied.reason())); - if (outcome instanceof IBotLoginOutcome.TimedOut) + if (outcome instanceof IBotLoginOutcome.TimedOut timedOut) throw new AgentProtocolException( AgentErrorCode.PLAYER_JOIN_TIMEOUT, - "Bot '%s' did not complete the login pipeline within %d seconds.".formatted(name, timeoutSeconds)); + "Bot '%s' did not complete the login pipeline within %d seconds (stalled in the %s phase)." + .formatted(name, timeoutSeconds, timedOut.phase())); - // The join event gets only the time remaining on the shared deadline, never a fresh budget: the - // driver has already reached the play phase, so the event wait is normally near-instant. - final long remainingNanos = deadlineNanos - System.nanoTime(); - if (remainingNanos <= 0L || !joinLatch.await(remainingNanos, TimeUnit.NANOSECONDS)) - throw new AgentProtocolException( - AgentErrorCode.PLAYER_JOIN_TIMEOUT, - "Bot '%s' completed login but no PlayerJoinEvent fired within the %d-second join budget." - .formatted(name, timeoutSeconds)); + awaitJoinEvent(joinLatch, deadlineNanos, name, timeoutSeconds); final Player player = Objects.requireNonNull(joinedPlayer.get(), "joined player"); - registerJoinedPlayer(player, health, permissionsCsv); + registerJoinedPlayer(player, command); plugin.getLogger().info( "LK_AGENT: Full-login player '%s' (%s) joined.".formatted(player.getName(), player.getUniqueId())); return new CreatePlayer.Response(player.getUniqueId(), player.getName()); } + catch (Exception exception) + { + primaryFailure = exception; + throw exception; + } finally + { + unregisterJoinListener(listener, deadlineNanos, primaryFailure); + } + } + + /** + * Awaits the join event on the residual of the shared deadline. + * + *

The join event gets only the time remaining, never a fresh budget: the driver has already reached the + * play phase, so this wait is normally near-instant. + * + * @param joinLatch + * Latch tripped by the one-shot {@code PlayerJoinEvent} listener. + * @param deadlineNanos + * The shared join deadline ({@link System#nanoTime()} based). + * @param name + * Bot name, for error messages. + * @param timeoutSeconds + * Total join budget, for error messages. + * @throws AgentProtocolException + * With {@code PLAYER_JOIN_TIMEOUT} when the deadline expires, or {@code INTERRUPTED} when the waiting + * thread is interrupted (matching {@link AgentMainThreadExecutor}'s contract). + */ + private static void awaitJoinEvent(CountDownLatch joinLatch, long deadlineNanos, String name, long timeoutSeconds) + throws AgentProtocolException + { + final long remainingNanos = deadlineNanos - System.nanoTime(); + try + { + if (remainingNanos <= 0L || !joinLatch.await(remainingNanos, TimeUnit.NANOSECONDS)) + throw new AgentProtocolException( + AgentErrorCode.PLAYER_JOIN_TIMEOUT, + "Bot '%s' completed login but no PlayerJoinEvent fired within the %d-second join budget." + .formatted(name, timeoutSeconds)); + } + catch (InterruptedException exception) + { + Thread.currentThread().interrupt(); + throw new AgentProtocolException( + AgentErrorCode.INTERRUPTED, + "Interrupted while awaiting PlayerJoinEvent for bot '%s'.".formatted(name), + exception); + } + } + + /** + * Unregisters the one-shot join listener without masking a primary failure or blowing the shared deadline. + * + *

The cleanup gets {@code min(remaining, 10s)} of the deadline with a 2-second floor: enough to run a + * trivial main-thread task even when the budget is exhausted, but never a fresh full budget. A cleanup + * failure is suppressed onto the primary failure when one exists, and logged otherwise. + * + * @param listener + * The listener to unregister. + * @param deadlineNanos + * The shared join deadline ({@link System#nanoTime()} based). + * @param primaryFailure + * The failure currently propagating out of the join, or {@code null} when the join succeeded. + */ + private void unregisterJoinListener(Listener listener, long deadlineNanos, @Nullable Exception primaryFailure) + { + final long remainingSeconds = TimeUnit.NANOSECONDS.toSeconds(deadlineNanos - System.nanoTime()); + final long cleanupBudgetSeconds = Math.max(2L, Math.min(10L, remainingSeconds)); + try { mainThreadExecutor.callOnMainThread(() -> { HandlerList.unregisterAll(listener); return Boolean.TRUE; - }); + }, cleanupBudgetSeconds); + } + catch (Exception cleanupFailure) + { + if (primaryFailure != null) + primaryFailure.addSuppressed(cleanupFailure); + else + plugin.getLogger().log( + Level.WARNING, "Failed to unregister the full-login join listener.", cleanupFailure); } } /** - * Registers a freshly joined full-login player in the store and applies health/permissions on the main thread. + * Registers a freshly joined full-login player in the store and applies the requested placement, health, + * and permissions on the main thread. + * + *

Placement honors the command's world and, when present, its explicit coordinates; a command that only + * names a world places the bot at that world's spawn. This mirrors the legacy-spawn semantics for both the + * {@code join(name, world)} wire shape (world only) and the builder's {@code atLocation} shape (world + + * coordinates). * * @param player * The joined Bukkit player. - * @param health - * Starting health, or {@code null} to leave the server default. - * @param permissionsCsv - * Comma-separated permission nodes to grant, or {@code null} for none. + * @param command + * The originating create-player command carrying world, coordinates, health, and permissions. * * @throws Exception * Propagates main-thread execution failures. */ - private void registerJoinedPlayer(Player player, @Nullable Double health, @Nullable String permissionsCsv) + private void registerJoinedPlayer(Player player, CreatePlayer.Command command) throws Exception { final UUID uuid = player.getUniqueId(); + final Double health = command.health(); + final String permissionsCsv = command.permissionsCsv(); mainThreadExecutor.callOnMainThread(() -> { + final World world = Bukkit.getWorld(command.worldName()); + if (world == null) + throw new IllegalArgumentException("World '%s' does not exist.".formatted(command.worldName())); + + final Location target = command.x() == null || command.y() == null || command.z() == null + ? world.getSpawnLocation() + : new Location(world, command.x(), command.y(), command.z()); + player.teleport(target); + playerStore.registerSyntheticPlayer(uuid, player); if (health != null) player.setHealth(Math.min(player.getMaxHealth(), health)); diff --git a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java index 631af8c5..a01698c9 100644 --- a/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java +++ b/lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java @@ -600,14 +600,17 @@ void handleCreatePlayer_shouldSurfaceFullLoginDenialAsTypedError() final CreatePlayer.Command command = new CreatePlayer.Command( "request-full", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, "en_us"); - // execute + verify try (MockedStatic bukkit = mockStatic(Bukkit.class)) { stubFullLoginBukkitStatics(bukkit); - assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) + // execute + final Throwable thrown = catchThrowable(() -> fixture.playerActions().handleCreatePlayer(command)); + + // verify + assertThat(thrown) .isInstanceOf(AgentProtocolException.class) - .satisfies(thrown -> assertThat(((AgentProtocolException) thrown).errorCode()) + .satisfies(failure -> assertThat(((AgentProtocolException) failure).errorCode()) .isEqualTo(AgentErrorCode.PLAYER_JOIN_DENIED)) .hasMessageContaining("Banned"); } @@ -620,20 +623,24 @@ void handleCreatePlayer_shouldSurfaceFullLoginDriverTimeoutAsTypedError() final PlayerActionsFixture fixture = createPlayerActionsFixture(); final IBotLoginDriver loginDriver = mock(); when(fixture.nmsAdapter().loginDriver()).thenReturn(loginDriver); - when(loginDriver.login(any())).thenReturn(new IBotLoginOutcome.TimedOut()); + when(loginDriver.login(any())).thenReturn(new IBotLoginOutcome.TimedOut(BotJoinPhase.LOGIN)); final CreatePlayer.Command command = new CreatePlayer.Command( "request-full-to", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, null); - // execute + verify try (MockedStatic bukkit = mockStatic(Bukkit.class)) { stubFullLoginBukkitStatics(bukkit); - assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) + // execute + final Throwable thrown = catchThrowable(() -> fixture.playerActions().handleCreatePlayer(command)); + + // verify + assertThat(thrown) .isInstanceOf(AgentProtocolException.class) - .satisfies(thrown -> assertThat(((AgentProtocolException) thrown).errorCode()) + .satisfies(failure -> assertThat(((AgentProtocolException) failure).errorCode()) .isEqualTo(AgentErrorCode.PLAYER_JOIN_TIMEOUT)) - .hasMessageContaining("did not complete the login pipeline"); + .hasMessageContaining("did not complete the login pipeline") + .hasMessageContaining("LOGIN"); } } @@ -649,14 +656,17 @@ void handleCreatePlayer_shouldSurfaceMissingJoinEventAsTypedTimeout() final CreatePlayer.Command command = new CreatePlayer.Command( "request-full-nj", "fullbot", null, "world", null, null, null, null, null, JoinMode.FULL_LOGIN, null); - // execute + verify try (MockedStatic bukkit = mockStatic(Bukkit.class)) { stubFullLoginBukkitStatics(bukkit); - assertThatThrownBy(() -> fixture.playerActions().handleCreatePlayer(command)) + // execute + final Throwable thrown = catchThrowable(() -> fixture.playerActions().handleCreatePlayer(command)); + + // verify + assertThat(thrown) .isInstanceOf(AgentProtocolException.class) - .satisfies(thrown -> assertThat(((AgentProtocolException) thrown).errorCode()) + .satisfies(failure -> assertThat(((AgentProtocolException) failure).errorCode()) .isEqualTo(AgentErrorCode.PLAYER_JOIN_TIMEOUT)) .hasMessageContaining("no PlayerJoinEvent fired"); } @@ -664,13 +674,14 @@ void handleCreatePlayer_shouldSurfaceMissingJoinEventAsTypedTimeout() /** * Stubs the Bukkit statics a FULL_LOGIN join touches: NOT the primary thread (the handler rejects - * main-thread execution), a server exposing a port, a no-op plugin manager, and a scheduler that runs - * main-thread callables inline. + * main-thread execution), a server exposing a port, a known world named {@code "world"}, a no-op plugin + * manager, and a scheduler that runs main-thread callables inline. */ private static void stubFullLoginBukkitStatics(MockedStatic bukkit) { final org.bukkit.Server server = mock(); when(server.getPort()).thenReturn(25_565); + final World world = mock(); final PluginManager pluginManager = mock(); final BukkitScheduler scheduler = mock(); when(scheduler.callSyncMethod(any(), any())).thenAnswer(invocation -> @@ -681,6 +692,7 @@ private static void stubFullLoginBukkitStatics(MockedStatic bukkit) bukkit.when(Bukkit::isPrimaryThread).thenReturn(false); bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(() -> Bukkit.getWorld("world")).thenReturn(world); bukkit.when(Bukkit::getPluginManager).thenReturn(pluginManager); bukkit.when(Bukkit::getScheduler).thenReturn(scheduler); } diff --git a/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java b/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java index 0791a7e3..55615092 100644 --- a/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java +++ b/lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperFullLoginIT.java @@ -1,5 +1,6 @@ package nl.pim16aap2.lightkeeper.maven.test; +import nl.pim16aap2.lightkeeper.framework.BlockPos; import nl.pim16aap2.lightkeeper.framework.ILightkeeperFramework; import nl.pim16aap2.lightkeeper.framework.LightkeeperExtension; import org.junit.jupiter.api.Test; @@ -37,10 +38,11 @@ void fullLogin_shouldJoinThroughRealLoginPipelineOnBothDistros(ILightkeeperFrame var joinCapture = framework.events().capture("org.bukkit.event.player.PlayerJoinEvent")) { - // execute — a full-login join drives the real handshake/login/configuration/play pipeline. + // execute — a full-login join drives the real handshake/login/configuration/play pipeline; the + // explicit location must be honored even though the server chooses the initial spawn point. final var player = framework.bots().builder() .withName(name) - .atSpawn(world) + .atLocation(world, 8, 100, 8) .withLocale("en_us") .fullLogin() .build(); @@ -49,6 +51,13 @@ void fullLogin_shouldJoinThroughRealLoginPipelineOnBothDistros(ILightkeeperFrame assertThat(player.name()).isEqualTo(name); assertThat(player.uniqueId()).isEqualTo(offlineUuid(name)); + // The bot stands at the requested coordinates (placement happens before the join call returns). + assertThat(world.entities() + .ofType("minecraft:player") + .within(new BlockPos(7, 99, 7), new BlockPos(9, 101, 9)) + .count()) + .isEqualTo(1); + // Both the async pre-login and the (main-thread) join events must have fired for a real login. framework.waitUntil(() -> !joinCapture.getCapturedEvents().isEmpty(), Duration.ofSeconds(10)); assertThat(joinCapture.getCapturedEvents()).isNotEmpty(); diff --git a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java index e94d9c5c..504f086d 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-api/src/main/java/nl/pim16aap2/lightkeeper/nms/api/IBotLoginOutcome.java @@ -51,8 +51,19 @@ record Denied(BotJoinPhase phase, String reason) implements IBotLoginOutcome /** * The login pipeline did not reach the play phase within the requested timeout. + * + * @param phase + * Protocol phase the pipeline was stalled in when the timeout expired, so a timeout report names where + * the login stopped making progress. */ - record TimedOut() implements IBotLoginOutcome + record TimedOut(BotJoinPhase phase) implements IBotLoginOutcome { + /** + * Validates the outcome inputs. + */ + public TimedOut + { + Objects.requireNonNull(phase, "phase"); + } } } diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java index 28cf30a4..0226bf2a 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflection.java @@ -55,6 +55,9 @@ enum PacketKind /** Common ping. */ PING, /** Play teleport/position. */ TELEPORT, /** Play login (join confirmation). */ GAME_LOGIN, + /** Login-phase plugin custom query (answered "unknown"). */ CUSTOM_QUERY, + /** Cookie request (answered with an absent payload). */ COOKIE_REQUEST, + /** Resource-pack push (declined). */ RESOURCE_PACK_PUSH, /** Login-phase disconnect. */ LOGIN_DISCONNECT, /** Common disconnect. */ DISCONNECT, /** Known informational packet requiring no response. */ CONSUME, @@ -89,6 +92,9 @@ enum PacketKind private final Class disconnectClass; private final Class playerPositionClass; private final Class gameLoginClass; + private final Class customQueryClass; + private final Class cookieRequestClass; + private final Class resourcePackPushClass; /** * Clientbound packets that are expected in the login/configuration phase and require no response; kept @@ -131,6 +137,26 @@ enum PacketKind private final Constructor clientInformationPacketConstructor; private final Class clientInformationClass; private final Method clientInformationCreateDefaultMethod; + private final Constructor customQueryAnswerConstructor; + private final Constructor cookieResponseConstructor; + /** + * Key type of a cookie request/response ({@code ResourceLocation} on Paper 1.21.11 — surfaced there as + * {@code Identifier} — and {@code MinecraftKey} on Spigot); taken structurally from the cookie-response + * constructor so no per-distro name entry is needed. + */ + private final Class cookieKeyClass; + private final Constructor resourcePackResponseConstructor; + /** + * {@code ServerboundResourcePackPacket.Action.DECLINED}. The nested action enum's binary name differs per + * distribution ({@code $Action} on Paper, {@code $a} on Spigot), so the class is taken structurally from + * the response constructor's second parameter. + */ + private final Object resourcePackDeclinedAction; + + // ---- Channel close-notification plumbing (see registerCloseListener). ---- + private final Method channelCloseFutureMethod; + private final Method futureAddListenerMethod; + private final Class futureListenerInterface; private final Map accessorCache = new ConcurrentHashMap<>(); @@ -157,11 +183,16 @@ enum PacketKind final Class packetListenerClass = NmsReflectionUtils.resolveClass("net.minecraft.network.PacketListener", serverClassLoader); - clientboundFlow = NmsReflectionUtils.resolveEnumConstant(packetFlowClass, "CLIENTBOUND"); - final Object serverboundFlow = NmsReflectionUtils.resolveEnumConstant(packetFlowClass, "SERVERBOUND"); - protocolLogin = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "LOGIN"); - protocolConfiguration = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "CONFIGURATION"); - protocolPlay = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "PLAY"); + // Ordinal fallbacks record the 1.21.11 declaration orders, verified against both jars (javap on + // Paper; runtime probe on Spigot, whose fields are obfuscated but whose Enum.name() is preserved): + // PacketFlow: SERVERBOUND=0, CLIENTBOUND=1. + // ConnectionProtocol: HANDSHAKING=0, PLAY=1, STATUS=2, LOGIN=3, CONFIGURATION=4. + clientboundFlow = NmsReflectionUtils.resolveEnumConstant(packetFlowClass, "CLIENTBOUND", 1); + final Object serverboundFlow = NmsReflectionUtils.resolveEnumConstant(packetFlowClass, "SERVERBOUND", 0); + protocolLogin = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "LOGIN", 3); + protocolConfiguration = + NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "CONFIGURATION", 4); + protocolPlay = NmsReflectionUtils.resolveEnumConstant(connectionProtocolClass, "PLAY", 1); loginListenerInterface = NmsReflectionUtils.resolveFirstClass( serverClassLoader, @@ -198,6 +229,13 @@ enum PacketKind gameLoginClass = resolveFirst( "net.minecraft.network.protocol.game.ClientboundLoginPacket", "net.minecraft.network.protocol.game.PacketPlayOutLogin"); + customQueryClass = resolveFirst( + "net.minecraft.network.protocol.login.ClientboundCustomQueryPacket", + "net.minecraft.network.protocol.login.PacketLoginOutCustomPayload"); + cookieRequestClass = resolveFirst( + "net.minecraft.network.protocol.cookie.ClientboundCookieRequestPacket"); + resourcePackPushClass = resolveFirst( + "net.minecraft.network.protocol.common.ClientboundResourcePackPushPacket"); consumeClasses = resolveConsumeClasses(); @@ -227,6 +265,12 @@ enum PacketKind connectionClass, "channel", channelClass); channelCloseMethod = channelClass.getMethod("close"); paperPacketLimiterField = resolvePaperPacketLimiterField(connectionClass); + channelCloseFutureMethod = channelClass.getMethod("closeFuture"); + futureListenerInterface = NmsReflectionUtils.resolveClass( + "io.netty.util.concurrent.GenericFutureListener", serverClassLoader); + futureAddListenerMethod = NmsReflectionUtils.resolveClass( + "io.netty.util.concurrent.Future", serverClassLoader) + .getMethod("addListener", futureListenerInterface); // Bound protocol infos for the configuration and play phases. final ProtocolInfos protocolInfos = resolveProtocolInfos(minecraftServer); @@ -268,6 +312,28 @@ enum PacketKind "net.minecraft.network.protocol.common.ServerboundClientInformationPacket") .getConstructor(clientInformationClass); + // Reply factories for server-initiated requests (constructions verified against both jars): + // ServerboundCustomQueryAnswerPacket(int, @Nullable CustomQueryAnswerPayload) — shared names. + final Class customQueryAnswerPayloadClass = NmsReflectionUtils.resolveClass( + "net.minecraft.network.protocol.login.custom.CustomQueryAnswerPayload", serverClassLoader); + customQueryAnswerConstructor = resolveFirst( + "net.minecraft.network.protocol.login.ServerboundCustomQueryAnswerPacket") + .getConstructor(int.class, customQueryAnswerPayloadClass); + // ServerboundCookieResponsePacket(, byte @Nullable []) — the key class differs per distro, so + // it is taken from the constructor itself. + cookieResponseConstructor = resolveSingleTwoArgConstructor( + resolveFirst("net.minecraft.network.protocol.cookie.ServerboundCookieResponsePacket"), + byte[].class); + cookieKeyClass = cookieResponseConstructor.getParameterTypes()[0]; + // ServerboundResourcePackPacket(UUID, Action) — the nested Action enum's binary name differs per + // distro ($Action vs $a), so it too is taken from the constructor. DECLINED ordinal verified as 1 + // on both jars (order: SUCCESSFULLY_LOADED, DECLINED, FAILED_DOWNLOAD, ACCEPTED, ...). + resourcePackResponseConstructor = resolveSingleTwoArgConstructorByFirstParameter( + resolveFirst("net.minecraft.network.protocol.common.ServerboundResourcePackPacket"), + UUID.class); + resourcePackDeclinedAction = NmsReflectionUtils.resolveEnumConstant( + resourcePackResponseConstructor.getParameterTypes()[1], "DECLINED", 1); + // Keep the serverbound flow reachable so a future distro rename is caught at init, not at first send. Objects.requireNonNull(serverboundFlow, "SERVERBOUND packet flow constant."); } @@ -306,11 +372,68 @@ Object openConnection(int port, Object listenerProxy) { final InetSocketAddress address = new InetSocketAddress(InetAddress.getLoopbackAddress(), port); final Object connection = connectToServerMethod.invoke(null, address, eventLoopGroupHolder, null); - disarmPaperPacketLimiter(connection); - initiateLoginConnectionMethod.invoke(connection, "127.0.0.1", port, listenerProxy); + try + { + disarmPaperPacketLimiter(connection); + initiateLoginConnectionMethod.invoke(connection, "127.0.0.1", port, listenerProxy); + } + catch (ReflectiveOperationException | RuntimeException exception) + { + // The TCP connection is already open; without this close it would leak on the error path. + closeConnection(connection); + throw exception; + } return connection; } + /** + * Invokes {@code callback} as soon as the connection's channel closes — locally or remotely. + * + *

This is the only reliable close signal the driver gets: the vanilla {@code Connection} surfaces + * closures through {@code PacketListener.onDisconnect}, but only from {@code handleDisconnection()}, which + * is driven by the client/server tick loops — and this client connection is in neither. Without this + * listener, a raw remote close (e.g. the server's 30-second {@code ReadTimeoutHandler} abandoning a stalled + * login, which sends no disconnect packet) would be invisible and the join would silently burn its entire + * timeout budget. + * + * @param connection + * The connection whose channel to observe. + * @param callback + * Invoked once (on the channel's event loop) when the channel closes. + * @throws ReflectiveOperationException + * When the reflective wiring fails. + */ + void registerCloseListener(Object connection, Runnable callback) + throws ReflectiveOperationException + { + final Object channel = Objects.requireNonNull( + channelField.get(connection), "Connection channel is not initialized."); + final Object closeFuture = channelCloseFutureMethod.invoke(channel); + final Object listener = Proxy.newProxyInstance( + serverClassLoader, + new Class[]{futureListenerInterface}, + (proxy, method, args) -> + { + if (method.getDeclaringClass() == Object.class) + return invokeObjectIdentityMethod(proxy, method, args); + callback.run(); + return null; + }); + futureAddListenerMethod.invoke(closeFuture, listener); + } + + @SuppressWarnings({"PMD.UseVarargs", "PMD.CompareObjectsWithEquals"}) // InvocationHandler contract; identity. + private static @Nullable Object invokeObjectIdentityMethod(Object proxy, Method method, Object @Nullable [] args) + { + return switch (method.getName()) + { + case "equals" -> args != null && args.length == 1 && proxy == args[0]; + case "hashCode" -> System.identityHashCode(proxy); + case "toString" -> "BotLoginReflection$CloseListener"; + default -> null; + }; + } + /** * Disarms Paper's inbound packet-rate limiter on the bot's own client-side connection. * @@ -449,6 +572,42 @@ void acceptTeleport(Object connection, Object positionPacket) sendMethod.invoke(connection, acceptTeleportationConstructor.newInstance(id)); } + /** + * Answers a login-phase plugin custom query with the "unknown" answer (a {@code null} payload), exactly as + * a vanilla client answers queries it does not understand, so the login is never stalled by one. + */ + void answerCustomQuery(Object connection, Object customQueryPacket) + throws ReflectiveOperationException + { + final int transactionId = ((Number) readNoArg(customQueryPacket, int.class)).intValue(); + sendMethod.invoke(connection, customQueryAnswerConstructor.newInstance(transactionId, null)); + } + + /** + * Answers a cookie request with an absent payload ({@code null} bytes), exactly as a vanilla client + * answers for a cookie it has never stored. + */ + void answerCookieRequest(Object connection, Object cookieRequestPacket) + throws ReflectiveOperationException + { + final Object key = readNoArg(cookieRequestPacket, cookieKeyClass); + sendMethod.invoke(connection, cookieResponseConstructor.newInstance(key, null)); + } + + /** + * Declines a pushed resource pack. + * + *

A server configured to require its resource pack will respond to the decline by kicking the + * bot — which then surfaces loudly as a typed join denial carrying the kick reason, rather than a silent + * stall. + */ + void declineResourcePack(Object connection, Object resourcePackPushPacket) + throws ReflectiveOperationException + { + final Object id = readNoArg(resourcePackPushPacket, UUID.class); + sendMethod.invoke(connection, resourcePackResponseConstructor.newInstance(id, resourcePackDeclinedAction)); + } + /** * Extracts a best-effort plain-text kick reason from a disconnect packet. */ @@ -523,6 +682,12 @@ PacketKind classify(Class clientboundPacketClass) return PacketKind.TELEPORT; if (clientboundPacketClass == gameLoginClass) return PacketKind.GAME_LOGIN; + if (clientboundPacketClass == customQueryClass) + return PacketKind.CUSTOM_QUERY; + if (clientboundPacketClass == cookieRequestClass) + return PacketKind.COOKIE_REQUEST; + if (clientboundPacketClass == resourcePackPushClass) + return PacketKind.RESOURCE_PACK_PUSH; if (clientboundPacketClass == loginDisconnectClass) return PacketKind.LOGIN_DISCONNECT; if (clientboundPacketClass == disconnectClass) @@ -609,13 +774,11 @@ private Object resolveMinecraftServer() private Set> resolveConsumeClasses() { final Set> classes = new HashSet<>(); - // Login-phase informational packets (offline mode never sends encryption/custom-query, but tolerate). + // Login-phase encryption request: never sent in offline mode, but tolerated. (Custom queries are a + // dedicated PacketKind and get a real "unknown" answer.) addOptional(classes, "net.minecraft.network.protocol.login.ClientboundHelloPacket", "net.minecraft.network.protocol.login.PacketLoginOutEncryptionBegin"); - addOptional(classes, - "net.minecraft.network.protocol.login.ClientboundCustomQueryPacket", - "net.minecraft.network.protocol.login.PacketLoginOutCustomPayload"); // Configuration/common informational packets that require no response (Mojang names on both distros). for (final String className : new String[]{ "net.minecraft.network.protocol.configuration.ClientboundRegistryDataPacket", @@ -623,17 +786,15 @@ private Set> resolveConsumeClasses() "net.minecraft.network.protocol.configuration.ClientboundResetChatPacket", "net.minecraft.network.protocol.common.ClientboundCustomPayloadPacket", "net.minecraft.network.protocol.common.ClientboundUpdateTagsPacket", - // Limitation: consumed without a response — a server that requires a resource-pack answer stalls (PR2). - "net.minecraft.network.protocol.common.ClientboundResourcePackPushPacket", + // Resource-pack pushes and cookie requests get real replies (declined / absent payload) via their + // dedicated PacketKinds; only the pop (which a vanilla client never answers) is consumed here. "net.minecraft.network.protocol.common.ClientboundResourcePackPopPacket", "net.minecraft.network.protocol.common.ClientboundStoreCookiePacket", "net.minecraft.network.protocol.common.ClientboundTransferPacket", "net.minecraft.network.protocol.common.ClientboundCustomReportDetailsPacket", "net.minecraft.network.protocol.common.ClientboundServerLinksPacket", "net.minecraft.network.protocol.common.ClientboundClearDialogPacket", - "net.minecraft.network.protocol.common.ClientboundShowDialogPacket", - // Limitation: consumed without a cookie response — a server that awaits one stalls the join (PR2). - "net.minecraft.network.protocol.cookie.ClientboundCookieRequestPacket"}) + "net.minecraft.network.protocol.common.ClientboundShowDialogPacket"}) { addOptional(classes, className); } @@ -692,6 +853,43 @@ private Object resolveSingletonInstance(String className) "No static singleton instance field of type " + className + " found on itself."); } + /** + * Resolves the single public two-argument constructor whose second parameter is exactly + * {@code secondParameterType}; the first parameter's (per-distro) type is then read off the constructor. + */ + private static Constructor resolveSingleTwoArgConstructor(Class owner, Class secondParameterType) + throws NoSuchMethodException + { + for (final Constructor constructor : owner.getConstructors()) + { + final Class[] parameterTypes = constructor.getParameterTypes(); + if (parameterTypes.length == 2 && parameterTypes[1].equals(secondParameterType)) + return constructor; + } + throw new NoSuchMethodException( + "No public 2-arg constructor with second parameter " + secondParameterType.getName() + + " on " + owner.getName() + "."); + } + + /** + * Resolves the single public two-argument constructor whose first parameter is exactly + * {@code firstParameterType}; the second parameter's (per-distro) type is then read off the constructor. + */ + private static Constructor resolveSingleTwoArgConstructorByFirstParameter( + Class owner, Class firstParameterType) + throws NoSuchMethodException + { + for (final Constructor constructor : owner.getConstructors()) + { + final Class[] parameterTypes = constructor.getParameterTypes(); + if (parameterTypes.length == 2 && parameterTypes[0].equals(firstParameterType)) + return constructor; + } + throw new NoSuchMethodException( + "No public 2-arg constructor with first parameter " + firstParameterType.getName() + + " on " + owner.getName() + "."); + } + private ProtocolInfos resolveProtocolInfos(Object minecraftServer) throws ReflectiveOperationException { diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java index b7b0d6aa..1d0cf204 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginSession.java @@ -84,11 +84,15 @@ IBotLoginOutcome run(BotLoginRequest request) try { + // Surface raw TCP closes immediately: nothing else ever drives PacketListener.onDisconnect for + // this client-side connection (the vanilla client does that from its tick loop, which we lack), + // so without this a remote close would silently burn the entire join timeout. + reflection.registerCloseListener(openedConnection, this::handleTransportDisconnect); reflection.sendHello(openedConnection, request.name()); } - catch (ReflectiveOperationException exception) + catch (ReflectiveOperationException | RuntimeException exception) { - terminate(openedConnection); + terminate(); throw new IllegalStateException( "Failed to start login for bot '%s'.".formatted(request.name()), exception); } @@ -97,17 +101,17 @@ IBotLoginOutcome run(BotLoginRequest request) { final IBotLoginOutcome result = outcome.get(request.timeout().toMillis(), TimeUnit.MILLISECONDS); if (!(result instanceof IBotLoginOutcome.Joined)) - terminate(openedConnection); + terminate(); return result; } catch (TimeoutException exception) { - terminate(openedConnection); - return new IBotLoginOutcome.TimedOut(); + terminate(); + return new IBotLoginOutcome.TimedOut(phase); } catch (ExecutionException exception) { - terminate(openedConnection); + terminate(); throw new IllegalStateException( "Full-login pipeline failed for bot '%s': %s".formatted(request.name(), rootMessage(exception)), exception); @@ -115,22 +119,22 @@ IBotLoginOutcome run(BotLoginRequest request) catch (InterruptedException exception) { Thread.currentThread().interrupt(); - terminate(openedConnection); + terminate(); throw new IllegalStateException( "Interrupted while awaiting full-login of bot '%s'.".formatted(request.name()), exception); } } /** - * Stops packet servicing and closes the connection; used on every terminal path except a successful join. - * - * @param liveConnection - * The connection to close. + * Stops packet servicing and closes the connection (when one was opened); used on every terminal path + * except a successful join, and on servicing failures after one. */ - private void terminate(Object liveConnection) + private void terminate() { terminated = true; - reflection.closeConnection(liveConnection); + final Object current = connection; + if (current != null) + reflection.closeConnection(current); } // ------------------------------------------------------------------------------------------------------- @@ -238,6 +242,9 @@ private void dispatchPacket(@Nullable Object packet) case PING -> reflection.answerPing(connection(), packet); case TELEPORT -> reflection.acceptTeleport(connection(), packet); case GAME_LOGIN -> completeJoined(); + case CUSTOM_QUERY -> reflection.answerCustomQuery(connection(), packet); + case COOKIE_REQUEST -> reflection.answerCookieRequest(connection(), packet); + case RESOURCE_PACK_PUSH -> reflection.declineResourcePack(connection(), packet); case LOGIN_DISCONNECT, DISCONNECT -> completeDenied(packet); case CONSUME -> { @@ -246,10 +253,12 @@ private void dispatchPacket(@Nullable Object packet) case UNKNOWN -> handleUnknownPacket(packetClass); } } - catch (ReflectiveOperationException exception) + catch (ReflectiveOperationException | RuntimeException exception) { - // A reflective failure means the driver can no longer service the connection reliably; stop. - terminated = true; + // Any servicing failure (reflective or runtime, e.g. a missing structural accessor) means the + // driver can no longer run this connection reliably: stop servicing, close the channel, and make + // sure the outcome can never be left pending. + terminate(); final IllegalStateException failure = new IllegalStateException( "Failed to handle %s packet %s.".formatted(phase, packetClass.getName()), exception); if (!outcome.completeExceptionally(failure)) @@ -267,7 +276,7 @@ private void handleUnknownPacket(Class packetClass) } // Fail-loud: an unhandled login/configuration packet would silently stall the join. - terminated = true; + terminate(); outcome.completeExceptionally(new IllegalStateException( "Unhandled %s packet from server: %s. Add it to the login driver's packet table." .formatted(phase, packetClass.getName()))); @@ -275,7 +284,7 @@ private void handleUnknownPacket(Class packetClass) private void handleTransportDisconnect() { - terminated = true; + terminate(); if (!outcome.isDone()) outcome.complete(new IBotLoginOutcome.Denied(phase, "Connection closed during " + phase + ".")); } @@ -288,7 +297,7 @@ private void completeJoined() private void completeDenied(Object disconnectPacket) { - terminated = true; + terminate(); if (!outcome.isDone()) outcome.complete(new IBotLoginOutcome.Denied(phase, reflection.disconnectReason(disconnectPacket))); } diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java index 3f4a0913..91831791 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/NmsReflectionUtils.java @@ -395,18 +395,27 @@ static Method findCompatibleMethod(Class type, Class... argumentTypes) } /** - * Resolves an enum constant by its {@link Enum#name() name}, tolerating obfuscated field names. + * Resolves an enum constant by its {@link Enum#name() name}, falling back to the constant's known + * declaration ordinal when the runtime names are obfuscated. + * + *

Both distributions currently keep Mojang constant names at runtime (verified against the 1.21.11 + * jars: Spigot's remapper obfuscates the enum fields but preserves the name string passed to the + * enum constructor), so the name lookup is expected to succeed. Should a future remapper obfuscate the + * names too, obfuscation preserves declaration order, so the per-version ordinal recorded by the calling + * adapter remains a correct identifier; the fallback is logged so its use is visible. * * @param enumClass * Enum class to search. * @param name * Unobfuscated constant name (e.g. {@code "CLIENTBOUND"}). + * @param expectedOrdinal + * Declaration ordinal of the constant for the server version this adapter targets. * @return * Matching enum constant. * @throws IllegalStateException - * When no constant with that name exists. + * When no constant with that name exists and the expected ordinal is out of range. */ - static Object resolveEnumConstant(Class enumClass, String name) + static Object resolveEnumConstant(Class enumClass, String name, int expectedOrdinal) { final Object[] constants = Objects.requireNonNull( enumClass.getEnumConstants(), "Expected enum constants for " + enumClass.getName()); @@ -415,8 +424,17 @@ static Object resolveEnumConstant(Class enumClass, String name) if (((Enum) constant).name().equals(name)) return constant; } - throw new IllegalStateException( - "No enum constant '" + name + "' found in " + enumClass.getName() + "."); + + if (expectedOrdinal < 0 || expectedOrdinal >= constants.length) + throw new IllegalStateException( + "No enum constant '" + name + "' found in " + enumClass.getName() + + " and ordinal " + expectedOrdinal + " is out of range (0.." + (constants.length - 1) + ")."); + + System.getLogger(NmsReflectionUtils.class.getName()).log( + System.Logger.Level.DEBUG, + () -> "Enum constant '%s' not found by name in %s; falling back to declaration ordinal %d ('%s')." + .formatted(name, enumClass.getName(), expectedOrdinal, constants[expectedOrdinal])); + return constants[expectedOrdinal]; } /** diff --git a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java index c3a09761..c8d8d884 100644 --- a/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java +++ b/lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotLoginReflectionHelpersTest.java @@ -7,7 +7,7 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.catchThrowable; /** * Unit tests for the pure, server-decoupled reflection helpers the full-login driver relies on to resolve the @@ -47,20 +47,44 @@ private void ambiguous(int flipped, String order) @Test void resolveEnumConstant_shouldFindConstantByName() { + // setup — the ordinal deliberately points at the OTHER constant, so a name hit must win. + final int misleadingOrdinal = 0; + // execute - final Object constant = NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, "BETA"); + final Object constant = NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, "BETA", misleadingOrdinal); // verify assertThat(constant).isEqualTo(SampleEnum.BETA); } @Test - void resolveEnumConstant_shouldThrowWhenNameIsUnknown() + void resolveEnumConstant_shouldFallBackToOrdinalWhenNameIsUnknown() { - // execute + verify - assertThatThrownBy(() -> NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, "GAMMA")) + // setup — an obfuscated-name miss must resolve via the recorded declaration ordinal. + final String obfuscatedName = "GAMMA"; + + // execute + final Object constant = NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, obfuscatedName, 1); + + // verify + assertThat(constant).isEqualTo(SampleEnum.BETA); + } + + @Test + void resolveEnumConstant_shouldThrowWhenNameIsUnknownAndOrdinalOutOfRange() + { + // setup + final int outOfRangeOrdinal = 2; + + // execute + final Throwable thrown = catchThrowable( + () -> NmsReflectionUtils.resolveEnumConstant(SampleEnum.class, "GAMMA", outOfRangeOrdinal)); + + // verify + assertThat(thrown) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("GAMMA"); + .hasMessageContaining("GAMMA") + .hasMessageContaining("out of range"); } @Test