Simplify custom payload handling (#12347)

build-data/paper.at

@@ -491,8 +491,8 @@ public net.minecraft.world.inventory.AnvilMenu repairItemCountCost
 public net.minecraft.world.inventory.BrewingStandMenu brewingStandData
 public net.minecraft.world.inventory.CraftingMenu access
 public net.minecraft.world.inventory.DispenserMenu dispenser
-public net.minecraft.world.inventory.HorseInventoryMenu horse
 public net.minecraft.world.inventory.HorseInventoryMenu SLOT_BODY_ARMOR
+public net.minecraft.world.inventory.HorseInventoryMenu horse
 public net.minecraft.world.inventory.MerchantContainer selectionHint
 public net.minecraft.world.inventory.Slot slot
 public net.minecraft.world.item.AdventureModePredicate predicates

paper-server/patches/sources/net/minecraft/network/protocol/common/custom/DiscardedPayload.java.patch

@@ -1,21 +1,26 @@
 --- a/net/minecraft/network/protocol/common/custom/DiscardedPayload.java
 +++ b/net/minecraft/network/protocol/common/custom/DiscardedPayload.java
-@@ -4,13 +_,14 @@
+@@ -4,13 +_,19 @@
  import net.minecraft.network.codec.StreamCodec;
  import net.minecraft.resources.ResourceLocation;
  
 -public record DiscardedPayload(ResourceLocation id) implements CustomPacketPayload {
-+public record DiscardedPayload(ResourceLocation id, io.netty.buffer.ByteBuf data) implements CustomPacketPayload { // CraftBukkit - store data
++public record DiscardedPayload(ResourceLocation id, byte[] data) implements CustomPacketPayload { // Paper - store data
      public static <T extends FriendlyByteBuf> StreamCodec<T, DiscardedPayload> codec(ResourceLocation id, int maxSize) {
 -        return CustomPacketPayload.codec((value, output) -> {}, buffer -> {
 +        return CustomPacketPayload.codec((value, output) -> {
-+            output.writeBytes(value.data); // CraftBukkit - serialize
++            // Paper start
++            // Always write data
++            output.writeBytes(value.data);
 +        }, buffer -> {
              int i = buffer.readableBytes();
              if (i >= 0 && i <= maxSize) {
 -                buffer.skipBytes(i);
 -                return new DiscardedPayload(id);
-+                return new DiscardedPayload(id, buffer.readBytes(i)); // CraftBukkit
++                final byte[] data = new byte[i];
++                buffer.readBytes(data);
++                return new DiscardedPayload(id, data);
++                // Paper end
              } else {
                  throw new IllegalArgumentException("Payload may not be larger than " + maxSize + " bytes");
              }

paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch

@@ -95,7 +95,7 @@
          }
      }
  
-@@ -88,30 +_,119 @@
+@@ -88,30 +_,123 @@
      public void handlePong(ServerboundPongPacket packet) {
      }
  
@@ -105,64 +105,68 @@
      @Override
      public void handleCustomPayload(ServerboundCustomPayloadPacket packet) {
 -    }
-+        // CraftBukkit start
++        // Paper start
-+        // Paper start - Brand support
 +        if (packet.payload() instanceof net.minecraft.network.protocol.common.custom.BrandPayload(String brand)) {
 +            this.player.clientBrandName = brand;
 +        }
-+        // Paper end - Brand support
++
 +        if (!(packet.payload() instanceof final net.minecraft.network.protocol.common.custom.DiscardedPayload discardedPayload)) {
 +            return;
 +        }
-+        PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
-+        net.minecraft.resources.ResourceLocation identifier = packet.payload().type().id();
-+        io.netty.buffer.ByteBuf payload = discardedPayload.data();
 +
-+        if (identifier.equals(ServerCommonPacketListenerImpl.CUSTOM_REGISTER)) {
++        PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
-+            try {
++
-+                String channels = payload.toString(com.google.common.base.Charsets.UTF_8);
++        final net.minecraft.resources.ResourceLocation identifier = packet.payload().type().id();
-+                for (String channel : channels.split("\0")) {
++        final byte[] data = discardedPayload.data();
-+                    this.getCraftPlayer().addChannel(channel);
++        try {
-+                }
++            final boolean registerChannel = ServerCommonPacketListenerImpl.CUSTOM_REGISTER.equals(identifier);
-+            } catch (Exception ex) {
++            if (registerChannel || ServerCommonPacketListenerImpl.CUSTOM_UNREGISTER.equals(identifier)) {
-+                ServerGamePacketListenerImpl.LOGGER.error("Couldn't register custom payload", ex);
++                // Strings separated by zeros instead of length prefixes
-+                this.disconnect(Component.literal("Invalid payload REGISTER!"), org.bukkit.event.player.PlayerKickEvent.Cause.INVALID_PAYLOAD); // Paper - kick event cause
++                int startIndex = 0;
-+            }
++                for (int i = 0; i < data.length; i++) {
-+        } else if (identifier.equals(ServerCommonPacketListenerImpl.CUSTOM_UNREGISTER)) {
++                    final byte b = data[i];
-+            try {
++                    if (b != 0) {
-+                String channels = payload.toString(com.google.common.base.Charsets.UTF_8);
++                        continue;
-+                for (String channel : channels.split("\0")) {
-+                    this.getCraftPlayer().removeChannel(channel);
-+                }
-+            } catch (Exception ex) {
-+                ServerGamePacketListenerImpl.LOGGER.error("Couldn't unregister custom payload", ex);
-+                this.disconnect(Component.literal("Invalid payload UNREGISTER!"), org.bukkit.event.player.PlayerKickEvent.Cause.INVALID_PAYLOAD); // Paper - kick event cause
-+            }
-+        } else {
-+            try {
-+                byte[] data = new byte[payload.readableBytes()];
-+                payload.readBytes(data);
-+                // Paper start - Brand support; Retain this incase upstream decides to 'break' the new mechanism in favour of backwards compat...
-+                if (identifier.equals(MINECRAFT_BRAND)) {
-+                    try {
-+                        this.player.clientBrandName = new net.minecraft.network.FriendlyByteBuf(io.netty.buffer.Unpooled.copiedBuffer(data)).readUtf(256);
-+                    } catch (StringIndexOutOfBoundsException ex) {
-+                        this.player.clientBrandName = "illegal";
 +                    }
++
++                    readChannelIdentifier(data, startIndex, i, registerChannel);
++                    startIndex = i + 1;
 +                }
-+                // Paper end - Brand support
++
-+                this.cserver.getMessenger().dispatchIncomingMessage(this.player.getBukkitEntity(), identifier.toString(), data);
++                // Read the last one
-+            } catch (Exception ex) {
++                readChannelIdentifier(data, startIndex, data.length, registerChannel);
-+                ServerGamePacketListenerImpl.LOGGER.error("Couldn't dispatch custom payload", ex);
++                return;
-+                this.disconnect(Component.literal("Invalid custom payload!"), org.bukkit.event.player.PlayerKickEvent.Cause.INVALID_PAYLOAD); // Paper - kick event cause
 +            }
++
++            if (identifier.equals(MINECRAFT_BRAND)) {
++                this.player.clientBrandName = new net.minecraft.network.FriendlyByteBuf(io.netty.buffer.Unpooled.wrappedBuffer(data)).readUtf(256);
++            }
++
++            this.cserver.getMessenger().dispatchIncomingMessage(this.player.getBukkitEntity(), identifier.toString(), data);
++        } catch (final Exception e) {
++            ServerGamePacketListenerImpl.LOGGER.error("Couldn't handle custom payload on channel {}", identifier, e);
++            this.disconnect(Component.literal("Invalid custom payload payload!"), org.bukkit.event.player.PlayerKickEvent.Cause.INVALID_PAYLOAD); // Paper - kick event cause
++        }
++    }
++
++    private void readChannelIdentifier(final byte[] data, final int from, final int to, final boolean register) {
++        final int length = to - from;
++        if (length == 0) {
++            return;
++        }
++
++        final String channel = new String(data, from, length, java.nio.charset.StandardCharsets.US_ASCII);
++        if (register) {
++            this.getCraftPlayer().addChannel(channel);
++        } else {
++            this.getCraftPlayer().removeChannel(channel);
 +        }
 +    }
 +
 +    public final boolean isDisconnected() {
 +        return (!this.player.joining && !this.connection.isConnected()) || this.processedDisconnect; // Paper - Fix duplication bugs
 +    }
-+    // CraftBukkit end
++    // Paper end
  
      @Override
      public void handleResourcePackResponse(ServerboundResourcePackPacket packet) {

paper-server/patches/sources/net/minecraft/world/inventory/HorseInventoryMenu.java.patch

@@ -1,7 +1,7 @@
 --- a/net/minecraft/world/inventory/HorseInventoryMenu.java
 +++ b/net/minecraft/world/inventory/HorseInventoryMenu.java
 @@ -19,9 +_,23 @@
-     private final AbstractHorse horse;
+     public final AbstractHorse horse;
      public static final int SLOT_BODY_ARMOR = 1;
      private static final int SLOT_HORSE_INVENTORY_START = 2;
 +    // CraftBukkit start

paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java

@@ -6,7 +6,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.io.BaseEncoding;
 import com.mojang.authlib.GameProfile;
 import com.mojang.datafixers.util.Pair;
-import io.netty.buffer.Unpooled;
 import io.papermc.paper.FeatureHooks;
 import io.papermc.paper.configuration.GlobalConfiguration;
 import io.papermc.paper.entity.LookAnchor;
@@ -103,7 +102,6 @@ import net.minecraft.server.players.UserWhiteListEntry;
 import net.minecraft.sounds.SoundEvent;
 import net.minecraft.world.entity.Entity;
 import net.minecraft.world.entity.ai.attributes.AttributeInstance;
-import net.minecraft.world.entity.ai.attributes.AttributeMap;
 import net.minecraft.world.entity.ai.attributes.Attributes;
 import net.minecraft.world.entity.item.ItemEntity;
 import net.minecraft.world.food.FoodData;
@@ -174,7 +172,6 @@ import org.bukkit.craftbukkit.map.CraftMapView;
 import org.bukkit.craftbukkit.map.RenderData;
 import org.bukkit.craftbukkit.potion.CraftPotionEffectType;
 import org.bukkit.craftbukkit.potion.CraftPotionUtil;
-import org.bukkit.craftbukkit.profile.CraftPlayerProfile;
 import org.bukkit.craftbukkit.scoreboard.CraftScoreboard;
 import org.bukkit.craftbukkit.util.CraftChatMessage;
 import org.bukkit.craftbukkit.util.CraftLocation;
@@ -189,7 +186,6 @@ import org.bukkit.event.player.PlayerExpCooldownChangeEvent;
 import org.bukkit.event.player.PlayerHideEntityEvent;
 import org.bukkit.event.player.PlayerRegisterChannelEvent;
 import org.bukkit.event.player.PlayerShowEntityEvent;
-import org.bukkit.event.player.PlayerSpawnChangeEvent;
 import org.bukkit.event.player.PlayerTeleportEvent;
 import org.bukkit.event.player.PlayerUnregisterChannelEvent;
 import org.bukkit.inventory.EquipmentSlot;
@@ -202,7 +198,6 @@ import org.bukkit.plugin.Plugin;
 import org.bukkit.plugin.messaging.StandardMessenger;
 import org.bukkit.potion.PotionEffect;
 import org.bukkit.potion.PotionEffectType;
-import org.bukkit.profile.PlayerProfile;
 import org.bukkit.scoreboard.Scoreboard;
 import org.jetbrains.annotations.NotNull;
 
@@ -2463,7 +2458,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
     }
 
     private void sendCustomPayload(ResourceLocation id, byte[] message) {
-        ClientboundCustomPayloadPacket packet = new ClientboundCustomPayloadPacket(new DiscardedPayload(id, Unpooled.wrappedBuffer(message)));
+        ClientboundCustomPayloadPacket packet = new ClientboundCustomPayloadPacket(new DiscardedPayload(id, message));
         this.getHandle().connection.send(packet);
     }