Fix custom payload handling on modern servers

Co-authored-by: rkfsociety <romankuzminvital@gmail.com>

Author: dmulloy2

src/main/java/com/comphenix/protocol/updater/SpigotUpdateRunnable.java

@@ -0,0 +1,58 @@
+/**
+ * ProtocolLib - Bukkit server library that allows access to the Minecraft protocol.
+ * Copyright (C) 2015 dmulloy2
+ * <p>
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ * <p>
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ * <p>
+ * You should have received a copy of the GNU General Public License along with this program;
+ * if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ * 02111-1307 USA
+ */
+package com.comphenix.protocol.updater;
+
+import com.comphenix.protocol.ProtocolLibrary;
+import com.comphenix.protocol.error.Report;
+
+/**
+ * Update check worker (top-level class so the runtime always loads a single {@code .class} entry).
+ */
+final class SpigotUpdateRunnable implements Runnable {
+
+    private final SpigotUpdater updater;
+
+    SpigotUpdateRunnable(SpigotUpdater updater) {
+        this.updater = updater;
+    }
+
+    @Override
+    public void run() {
+        try {
+            String version = this.updater.getSpigotVersion();
+            this.updater.setRemoteVersion(version);
+
+            if (this.updater.versionCheck(version)) {
+                this.updater.result = Updater.UpdateResult.SPIGOT_UPDATE_AVAILABLE;
+            } else {
+                this.updater.result = Updater.UpdateResult.NO_UPDATE;
+            }
+        } catch (Throwable ex) {
+            if (ProtocolLibrary.getConfig().isDebug()) {
+                ProtocolLibrary.getErrorReporter().reportDetailed(
+                        this.updater,
+                        Report.newBuilder(Updater.REPORT_CANNOT_UPDATE_PLUGIN).error(ex).callerParam(this));
+            }
+
+            ProtocolLibrary.disableUpdates();
+        } finally {
+            for (Runnable listener : this.updater.listeners) {
+                ProtocolLibrary.getScheduler().runTask(listener);
+            }
+        }
+    }
+}

src/main/java/com/comphenix/protocol/updater/SpigotUpdater.java

@@ -16,8 +16,6 @@
  */
 package com.comphenix.protocol.updater;
 
-import com.comphenix.protocol.ProtocolLibrary;
-import com.comphenix.protocol.error.Report;
 import com.comphenix.protocol.utility.Closer;
 import org.bukkit.plugin.Plugin;
 
@@ -44,7 +42,7 @@ public void start(UpdateType type) {
         waitForThread();
 
         this.type = type;
-        this.thread = new Thread(new SpigotUpdateRunnable());
+        this.thread = new Thread(new SpigotUpdateRunnable(this));
         this.thread.start();
     }
 
@@ -54,36 +52,8 @@ public String getResult() {
         return String.format(result.toString(), remoteVersion, plugin.getDescription().getVersion(), RESOURCE_URL);
     }
 
-    private class SpigotUpdateRunnable implements Runnable {
-
-        @Override
-        public void run() {
-            try {
-                String version = getSpigotVersion();
-                remoteVersion = version;
-
-                if (versionCheck(version)) {
-                    result = UpdateResult.SPIGOT_UPDATE_AVAILABLE;
-                } else {
-                    result = UpdateResult.NO_UPDATE;
-                }
-            } catch (Throwable ex) {
-                if (ProtocolLibrary.getConfig().isDebug()) {
-                    ProtocolLibrary.getErrorReporter().reportDetailed(
-                            SpigotUpdater.this, Report.newBuilder(REPORT_CANNOT_UPDATE_PLUGIN).error(ex).callerParam(this));
-                } else {
-                    // People don't care
-                    // plugin.getLogger().log(Level.WARNING, "Failed to check for updates: " + ex);
-                }
-
-                ProtocolLibrary.disableUpdates();
-            } finally {
-                // Invoke the listeners on the main thread
-                for (Runnable listener : listeners) {
-                    ProtocolLibrary.getScheduler().runTask(listener);
-                }
-            }
-        }
+    void setRemoteVersion(String remoteVersion) {
+        this.remoteVersion = remoteVersion;
     }
 
     private static final String RESOURCE_URL = "https://www.spigotmc.org/resources/protocollib.1997/";

src/main/java/com/comphenix/protocol/wrappers/CustomPacketPayloadWrapper.java

@@ -18,6 +18,8 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
 
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
@@ -33,8 +35,8 @@
  * by default. Constructing a new wrapper instance will give out a handle to a completely new implemented type, that
  * allows to set a key and some kind of data of any choice.
  * <p>
- * Constructing this class from a generic handle requires a payload that retains its raw byte data, such as
- * {@code DiscardedPayload}. Typed payloads without raw bytes should be accessed through a structure modifier directly.
+ * Constructing this class from a generic handle requires either retained raw byte data, such as {@code DiscardedPayload},
+ * or a writable serialization method on the concrete payload type.
  *
  * @author Pasqual Koschmieder
  */
@@ -46,11 +48,14 @@ public final class CustomPacketPayloadWrapper {
     private static final ConstructorAccessor PAYLOAD_WRAPPER_CONSTRUCTOR;
 
     private static final MethodAccessor GET_ID_PAYLOAD_METHOD;
-    private static final MethodAccessor GET_TYPE_PAYLOAD_METHOD;
-    private static final MethodAccessor GET_ID_PAYLOAD_TYPE_METHOD;
+    private static final MethodAccessor GET_ID_HOLDER_PAYLOAD_METHOD;
+    private static final MethodAccessor GET_ID_PAYLOAD_HOLDER_METHOD;
     private static final MethodAccessor SERIALIZE_PAYLOAD_METHOD;
+    private static final Class<?> PAYLOAD_SERIALIZER_CLASS;
+    private static final ConcurrentHashMap<Class<?>, Optional<MethodAccessor>> SERIALIZE_PAYLOAD_METHODS =
+            new ConcurrentHashMap<>();
 
-    /** True when running on MC 1.21.5+ where DiscardedPayload is used instead of the ByteBuddy proxy. */
+    /** True when running on the nested-ID API where DiscardedPayload is used instead of the ByteBuddy proxy. */
     private static final boolean USE_DISCARDED_PAYLOAD;
     private static final boolean DISCARDED_PAYLOAD_USES_BYTE_ARRAY;
 
@@ -61,11 +66,9 @@ public final class CustomPacketPayloadWrapper {
             MINECRAFT_KEY_CLASS = MinecraftReflection.getMinecraftKeyClass();
             CUSTOM_PACKET_PAYLOAD_CLASS = MinecraftReflection.getMinecraftClass("network.protocol.common.custom.CustomPacketPayload");
 
-            // MC 1.21.5+: CustomPacketPayload no longer has id(); DiscardedPayload is the fallback payload
-            // MC < 1.21.5: CustomPacketPayload.id() returns Identifier directly
             Method getPayloadId = null;
-            Method getPayloadType = null;
-            Method getPayloadTypeId = null;
+            Method getPayloadIdHolder = null;
+            Method getPayloadIdFromHolder = null;
             boolean useDiscardedPayload = false;
             try {
                 getPayloadId = FuzzyReflection.fromClass(CUSTOM_PACKET_PAYLOAD_CLASS).getMethod(FuzzyMethodContract.newBuilder()
@@ -74,17 +77,19 @@ public final class CustomPacketPayloadWrapper {
                         .parameterCount(0)
                         .build());
             } catch (IllegalArgumentException ignored) {
-                // New API: CustomPacketPayload.type().id() exposes the identifier for every payload type.
-                getPayloadType = CUSTOM_PACKET_PAYLOAD_CLASS.getMethod("type");
-                getPayloadTypeId = getPayloadType.getReturnType().getMethod("id");
+                // Paper 26: the payload exposes its identifier through one intermediate record.
+                Method[] accessors = findChainedIdAccessors(CUSTOM_PACKET_PAYLOAD_CLASS, MINECRAFT_KEY_CLASS);
+                getPayloadIdHolder = accessors[0];
+                getPayloadIdFromHolder = accessors[1];
                 useDiscardedPayload = true;
             }
             GET_ID_PAYLOAD_METHOD = getPayloadId != null ? Accessors.getMethodAccessor(getPayloadId) : null;
-            GET_TYPE_PAYLOAD_METHOD = getPayloadType != null ? Accessors.getMethodAccessor(getPayloadType) : null;
-            GET_ID_PAYLOAD_TYPE_METHOD = getPayloadTypeId != null ? Accessors.getMethodAccessor(getPayloadTypeId) : null;
+            GET_ID_HOLDER_PAYLOAD_METHOD = getPayloadIdHolder != null
+                    ? Accessors.getMethodAccessor(getPayloadIdHolder) : null;
+            GET_ID_PAYLOAD_HOLDER_METHOD = getPayloadIdFromHolder != null
+                    ? Accessors.getMethodAccessor(getPayloadIdFromHolder) : null;
             USE_DISCARDED_PAYLOAD = useDiscardedPayload;
 
-            // serialize method may not exist in new API
             Method serializePayloadData = null;
             try {
                 serializePayloadData = FuzzyReflection.fromClass(CUSTOM_PACKET_PAYLOAD_CLASS).getMethod(FuzzyMethodContract.newBuilder()
@@ -94,15 +99,26 @@ public final class CustomPacketPayloadWrapper {
                         .parameterDerivedOf(ByteBuf.class, 0)
                         .build());
             } catch (IllegalArgumentException ignored) {
-                // Not present in new API
+                // Paper 26 declares the write method on each concrete payload instead.
             }
             SERIALIZE_PAYLOAD_METHOD = serializePayloadData != null
                     ? Accessors.getMethodAccessor(serializePayloadData) : null;
+            if (serializePayloadData == null) {
+                ByteBuf buffer = Unpooled.buffer();
+                try {
+                    PAYLOAD_SERIALIZER_CLASS = MinecraftReflection.getPacketDataSerializer(buffer).getClass();
+                } finally {
+                    if (buffer.refCnt() > 0) {
+                        buffer.release();
+                    }
+                }
+            } else {
+                PAYLOAD_SERIALIZER_CLASS = null;
+            }
 
             Constructor<?> payloadWrapperConstructor;
             boolean discardedPayloadUsesByteArray = false;
             if (useDiscardedPayload) {
-                // The discarded payload changed from ByteBuf to byte[] in 26.2.
                 Class<?> discardedPayloadClass = MinecraftReflection.getMinecraftClass(
                         "network.protocol.common.custom.DiscardedPayload");
                 try {
@@ -140,6 +156,41 @@ public Class<CustomPacketPayloadWrapper> getSpecificType() {
         }
     }
 
+    private static Method[] findChainedIdAccessors(Class<?> payloadClass, Class<?> keyClass) {
+        Method payloadAccessor = null;
+        Method keyAccessor = null;
+        FuzzyMethodContract keyAccessorContract = FuzzyMethodContract.newBuilder()
+                .banModifier(Modifier.STATIC)
+                .returnTypeExact(keyClass)
+                .parameterCount(0)
+                .build();
+
+        for (Method candidate : payloadClass.getMethods()) {
+            if (Modifier.isStatic(candidate.getModifiers()) || candidate.getParameterCount() != 0) {
+                continue;
+            }
+
+            Method candidateKeyAccessor;
+            try {
+                candidateKeyAccessor = FuzzyReflection.fromClass(candidate.getReturnType(), true)
+                        .getMethod(keyAccessorContract);
+            } catch (IllegalArgumentException ignored) {
+                continue;
+            }
+
+            if (payloadAccessor != null) {
+                throw new IllegalArgumentException("Multiple CustomPacketPayload identifier accessors found");
+            }
+            payloadAccessor = candidate;
+            keyAccessor = candidateKeyAccessor;
+        }
+
+        if (payloadAccessor == null) {
+            throw new IllegalArgumentException("Unable to find a CustomPacketPayload identifier accessor");
+        }
+        return new Method[]{payloadAccessor, keyAccessor};
+    }
+
     private static Constructor<?> makePayloadWrapper() throws Exception {
         return new ByteBuddy()
                 .subclass(Object.class)
@@ -162,6 +213,64 @@ private static Constructor<?> makePayloadWrapper() throws Exception {
                 .getConstructor(MinecraftReflection.getMinecraftKeyClass(), byte[].class);
     }
 
+    private static Optional<MethodAccessor> findSerializePayloadMethod(Class<?> payloadClass) {
+        Method serializeMethod = null;
+        for (Method candidate : FuzzyReflection.fromClass(payloadClass, true).getMethods()) {
+            if (Modifier.isStatic(candidate.getModifiers())
+                    || candidate.getReturnType() != void.class
+                    || candidate.getParameterCount() != 1) {
+                continue;
+            }
+
+            Class<?> parameterClass = candidate.getParameterTypes()[0];
+            if (parameterClass == Object.class || !parameterClass.isAssignableFrom(PAYLOAD_SERIALIZER_CLASS)) {
+                continue;
+            }
+            if (serializeMethod != null) {
+                throw new IllegalArgumentException("Multiple CustomPacketPayload serialization methods found on "
+                        + payloadClass.getName());
+            }
+            serializeMethod = candidate;
+        }
+
+        if (serializeMethod == null) {
+            return Optional.empty();
+        }
+        serializeMethod.setAccessible(true);
+        return Optional.of(Accessors.getMethodAccessor(serializeMethod));
+    }
+
+    private static MethodAccessor getSerializePayloadMethod(Object payload) {
+        if (SERIALIZE_PAYLOAD_METHOD != null) {
+            return SERIALIZE_PAYLOAD_METHOD;
+        }
+        return SERIALIZE_PAYLOAD_METHODS
+                .computeIfAbsent(payload.getClass(), CustomPacketPayloadWrapper::findSerializePayloadMethod)
+                .orElse(null);
+    }
+
+    private static byte[] serializePayload(Object payload) {
+        MethodAccessor serializeMethod = getSerializePayloadMethod(payload);
+        if (serializeMethod == null) {
+            throw new UnsupportedOperationException(
+                    "Cannot extract raw custom payload bytes from " + payload.getClass().getName()
+                            + ": payload has no byte array, ByteBuf, or serialization method");
+        }
+
+        ByteBuf buffer = Unpooled.buffer();
+        try {
+            Object serializer = MinecraftReflection.getPacketDataSerializer(buffer);
+            serializeMethod.invoke(payload, serializer);
+            byte[] result = new byte[buffer.readableBytes()];
+            buffer.getBytes(buffer.readerIndex(), result);
+            return result;
+        } finally {
+            if (buffer.refCnt() > 0) {
+                buffer.release();
+            }
+        }
+    }
+
     // ====== api methods ======
 
     /**
@@ -233,18 +342,7 @@ public static CustomPacketPayloadWrapper fromUnknownPayload(Object payload) {
                         buf.resetReaderIndex();
                         return data;
                     })
-                    .orElseGet(() -> {
-                        if (SERIALIZE_PAYLOAD_METHOD == null) {
-                            throw new UnsupportedOperationException(
-                                    "Cannot extract raw custom payload bytes from " + payload.getClass().getName()
-                                            + ": payload has no byte array or ByteBuf field and this Minecraft version "
-                                            + "does not expose CustomPacketPayload serialization");
-                        }
-                        ByteBuf buffer = Unpooled.buffer();
-                        Object serializer = MinecraftReflection.getPacketDataSerializer(buffer);
-                        SERIALIZE_PAYLOAD_METHOD.invoke(payload, serializer);
-                        return StreamSerializer.getDefault().getBytesAndRelease(buffer);
-                    });
+                    .orElseGet(() -> serializePayload(payload));
         }
 
         return new CustomPacketPayloadWrapper(messagePayload, id);
@@ -255,8 +353,8 @@ private static MinecraftKey getPayloadId(Object payload) {
         if (GET_ID_PAYLOAD_METHOD != null) {
             messageId = GET_ID_PAYLOAD_METHOD.invoke(payload);
         } else {
-            Object payloadType = GET_TYPE_PAYLOAD_METHOD.invoke(payload);
-            messageId = GET_ID_PAYLOAD_TYPE_METHOD.invoke(payloadType);
+            Object idHolder = GET_ID_HOLDER_PAYLOAD_METHOD.invoke(payload);
+            messageId = GET_ID_PAYLOAD_HOLDER_METHOD.invoke(idHolder);
         }
         return MinecraftKey.getConverter().getSpecific(messageId);
     }