diff --git a/CHANGELOG.md b/CHANGELOG.md index c2eb23c..44fbd37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.0.8 — Constructor-safe PacketEvents wrapper allocation + +- Fix `NullPointerException: wrappper.bufferLock is null` when FastLoginPlus sends `LOGIN.SERVER.ENCRYPTION_REQUEST` with a `PacketContainer` constructed from scratch. +- ProtocolLib packet constructors use `ObjectAllocator` to allocate wrappers without their constructors. For `LOGIN.ENCRYPTION_REQUEST`, use PacketEvents' own security-preserving public constructor (`shouldAuthenticate=true`). For other wrappers, restore mandatory per-instance `bufferLock` and registry holder using a genuine constructor-initialized PacketWrapper of the same client/server protocol version. +- Keep existing packet-specific fields and ProtocolLib write behavior (including FastLoginPlus's `shouldAuthenticate=true` field), and never share locks between newly created wrappers. +- Leave pre-login PacketEvents User/Channel routing from v1.0.7 intact; no changes to the AuthMe or FastLoginPlus login policies. + ## 1.0.7 — Deferred Pre-login Channel Fix - Fix deferred inbound `LOGIN.CLIENT.LOGIN_START` failure when ProtocolLib2PacketEvents had a generated `TemporaryPlayer` instead of a Bukkit `CraftPlayer`: use the already associated PacketEvents pre-login `User` and its live Netty Channel for silent transport instead of `PlayerManager.receivePacketSilently(Player, ...)`. diff --git a/pom.xml b/pom.xml index b527563..4de86ca 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ com.comphenix.protocol ProtocolLib2PacketEvents - 1.0.7 + 1.0.8 jar ProtocolLib2PacketEvents diff --git a/src/main/java/com/comphenix/protocol/events/PacketContainer.java b/src/main/java/com/comphenix/protocol/events/PacketContainer.java index 1391d5c..57b697b 100644 --- a/src/main/java/com/comphenix/protocol/events/PacketContainer.java +++ b/src/main/java/com/comphenix/protocol/events/PacketContainer.java @@ -40,6 +40,7 @@ import com.github.retrooper.packetevents.protocol.player.UserProfile; import com.github.retrooper.packetevents.wrapper.PacketTypeData; import com.github.retrooper.packetevents.wrapper.login.client.WrapperLoginClientLoginStart; +import com.github.retrooper.packetevents.wrapper.login.server.WrapperLoginServerEncryptionRequest; import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerChunkData; import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerUnloadChunk; import com.github.retrooper.packetevents.wrapper.PacketWrapper; @@ -877,11 +878,36 @@ private static PacketWrapper allocate(PacketType type) { + ", so it cannot be constructed from scratch. Build the packet bytes yourself and send " + "them through PacketEvents directly."); } - PacketWrapper wrapper = ObjectAllocator.allocate(wrapperClass); ServerVersion serverVersion = PacketEvents.getAPI().getServerManager().getVersion(); ClientVersion clientVersion = serverVersion.toClientVersion(); int id = type.toPacketEvents().getId(clientVersion); + if (id < 0) { + throw new IllegalStateException("Packet " + type + " is unsupported by client version " + clientVersion); + } + if (wrapperClass == WrapperLoginServerEncryptionRequest.class) { + // Authentication is security-sensitive: let PacketEvents' own public + // constructor initialize its immutable bufferLock, registries and + // shouldAuthenticate=true default. FastLoginPlus then fills in its + // actual public key, verify token and explicitly requests auth. + // Never synthesize a false online-authentication flag here. + WrapperLoginServerEncryptionRequest encryption = + new WrapperLoginServerEncryptionRequest("", new byte[0], new byte[0], true); + encryption.setClientVersion(clientVersion); + return encryption; + } + PacketWrapper wrapper = ObjectAllocator.allocate(wrapperClass); try { + // ObjectAllocator intentionally bypasses the subclass constructors, but + // PacketWrapper's superclass constructor is NOT optional: it creates the + // final bufferLock and initializes the registry holder. In particular, + // FastLogin's freshly built LOGIN.SERVER.ENCRYPTION_REQUEST otherwise + // crashes in PacketEvents ProtocolManager.transformWrappers() while + // sending through the pre-login User channel. + // + // Use a genuine, constructor-initialized PacketWrapper as the source + // of per-instance runtime state (never share a lock across packets). + PacketWrapper initialized = new PacketWrapper<>(clientVersion, serverVersion, id); + copyRuntimeStateFromConstructor(wrapper, initialized); setField(wrapper, "clientVersion", clientVersion); setField(wrapper, "serverVersion", serverVersion); setField(wrapper, "packetTypeData", new PacketTypeData(type.toPacketEvents(), id)); @@ -891,6 +917,20 @@ private static PacketWrapper allocate(PacketType type) { return wrapper; } + /** Restore the PacketWrapper superclass invariants after constructor-free allocation. */ + static void copyRuntimeStateFromConstructor(PacketWrapper allocated, + PacketWrapper constructorInitialized) + throws ReflectiveOperationException { + Field bufferLock = PacketWrapper.class.getDeclaredField("bufferLock"); + bufferLock.setAccessible(true); + Object lock = bufferLock.get(constructorInitialized); + if (lock == null) throw new IllegalStateException("PacketEvents constructor did not initialize bufferLock"); + bufferLock.set(allocated, lock); + Field registryHolder = PacketWrapper.class.getDeclaredField("registries"); + registryHolder.setAccessible(true); + registryHolder.set(allocated, registryHolder.get(constructorInitialized)); + } + private static Class> wrapperClassOf(PacketType type) { return type.toPacketEvents() == null ? null : type.toPacketEvents().getWrapperClass(); } diff --git a/src/main/resources/plugin.yml b/src/main/resources/plugin.yml index 65267ba..a6cbd99 100644 --- a/src/main/resources/plugin.yml +++ b/src/main/resources/plugin.yml @@ -6,11 +6,11 @@ main: com.comphenix.protocol.ProtocolLib # Bukkit.getPluginManager().getPlugin("ProtocolLib") - which resolves to this plugin # through `provides` below - and refuse to enable any ProtocolLib older than v5.0.0, # because ProtocolLib 4.x is genuinely incompatible with PacketEvents 2.x. P2P's real -# release version (1.0.7) is carried by the jar name and the GitHub releases. +# release version (1.0.8) is carried by the jar name and the GitHub releases. version: 5.4.0 author: CyoriaSMP Team description: A drop-in ProtocolLib compatibility layer powered by PacketEvents. -p2p-version: 1.0.7 +p2p-version: 1.0.8 protocol-lib-api-version: 5.4.0 depend: [packetevents] provides: [ProtocolLib] diff --git a/src/test/java/com/comphenix/protocol/events/PacketWrapperAllocationTest.java b/src/test/java/com/comphenix/protocol/events/PacketWrapperAllocationTest.java new file mode 100644 index 0000000..bfdcfd8 --- /dev/null +++ b/src/test/java/com/comphenix/protocol/events/PacketWrapperAllocationTest.java @@ -0,0 +1,48 @@ +package com.comphenix.protocol.events; + +import com.comphenix.protocol.reflect.ObjectAllocator; +import com.github.retrooper.packetevents.wrapper.PacketWrapper; +import com.github.retrooper.packetevents.wrapper.login.server.WrapperLoginServerEncryptionRequest; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; + +import static org.junit.jupiter.api.Assertions.*; + +class PacketWrapperAllocationTest { + @Test + void constructorFreeWrapperMustRecoverDistinctBufferLockAndRegistry() throws Exception { + // Tests the actual constructor-free type used by FastLoginPlus, without + // requiring PacketEvents to have started a Minecraft server. + PacketWrapper template = ObjectAllocator.allocate(PacketWrapper.class); + Field lockField = PacketWrapper.class.getDeclaredField("bufferLock"); + lockField.setAccessible(true); + Object expectedLock = new Object(); + lockField.set(template, expectedLock); + Field registryField = PacketWrapper.class.getDeclaredField("registries"); + registryField.setAccessible(true); + Object originalRegistry = registryField.get(template); + + WrapperLoginServerEncryptionRequest first = + ObjectAllocator.allocate(WrapperLoginServerEncryptionRequest.class); + WrapperLoginServerEncryptionRequest second = + ObjectAllocator.allocate(WrapperLoginServerEncryptionRequest.class); + assertNull(first.bufferLock); + assertNull(second.bufferLock); + PacketContainer.copyRuntimeStateFromConstructor(first, template); + assertSame(expectedLock, first.bufferLock); + assertSame(originalRegistry, registryField.get(first)); + lockField.set(template, new Object()); + PacketContainer.copyRuntimeStateFromConstructor(second, template); + assertNotNull(second.bufferLock); + assertNotSame(first.bufferLock, second.bufferLock); + } + + @Test + void missingConstructorLockIsRejectedInsteadOfSendingInvalidWrapper() throws Exception { + PacketWrapper template = ObjectAllocator.allocate(PacketWrapper.class); + PacketWrapper wrapper = ObjectAllocator.allocate(WrapperLoginServerEncryptionRequest.class); + assertThrows(IllegalStateException.class, + () -> PacketContainer.copyRuntimeStateFromConstructor(wrapper, template)); + } +}