From d6ddf77aac67f34d46d0c7126456a32c3bbddef6 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Mon, 18 Sep 2023 15:49:06 +0100 Subject: [PATCH] Fix unhandled packets disconnecting the client. --- .../client/ClientConfigurationNetworkAddon.java | 6 +----- .../client/ClientPlayNetworkAddon.java | 6 +----- .../AbstractChanneledNetworkAddon.java | 8 +++----- .../play/NetworkingPlayPacketTest.java | 9 +++++++++ .../play/NetworkingPlayPacketClientTest.java | 17 +++++++++++++++++ 5 files changed, 31 insertions(+), 15 deletions(-) diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java index 5ea6feebdf..9499e22e73 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientConfigurationNetworkAddon.java @@ -86,11 +86,7 @@ protected void receiveRegistration(boolean register, PacketByteBuf buf) { * @return true if the packet has been handled */ public boolean handle(PacketByteBufPayload payload) { - try { - return this.handle(payload.id(), payload.data()); - } finally { - payload.data().release(); - } + return this.handle(payload.id(), payload.data()); } @Override diff --git a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java index 56127a49f5..d10f39925a 100644 --- a/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java +++ b/fabric-networking-api-v1/src/client/java/net/fabricmc/fabric/impl/networking/client/ClientPlayNetworkAddon.java @@ -86,11 +86,7 @@ public void onServerReady() { * @return true if the packet has been handled */ public boolean handle(PacketByteBufPayload payload) { - try { - return this.handle(payload.id(), payload.data()); - } finally { - payload.data().release(); - } + return this.handle(payload.id(), payload.data()); } @Override diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java index 46ed2a1d7c..8818ebc933 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/AbstractChanneledNetworkAddon.java @@ -72,17 +72,17 @@ protected void registerPendingChannels(ChannelInfoHolder holder, NetworkState st } // always supposed to handle async! - protected boolean handle(Identifier channelName, PacketByteBuf originalBuf) { + protected boolean handle(Identifier channelName, PacketByteBuf buf) { this.logger.debug("Handling inbound packet from channel with name \"{}\"", channelName); // Handle reserved packets if (NetworkingImpl.REGISTER_CHANNEL.equals(channelName)) { - this.receiveRegistration(true, PacketByteBufs.slice(originalBuf)); + this.receiveRegistration(true, buf); return true; } if (NetworkingImpl.UNREGISTER_CHANNEL.equals(channelName)) { - this.receiveRegistration(false, PacketByteBufs.slice(originalBuf)); + this.receiveRegistration(false, buf); return true; } @@ -92,8 +92,6 @@ protected boolean handle(Identifier channelName, PacketByteBuf originalBuf) { return false; } - PacketByteBuf buf = PacketByteBufs.slice(originalBuf); - try { this.receive(handler, buf); } catch (Throwable ex) { diff --git a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java index b05d77be89..87ff660824 100644 --- a/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java +++ b/fabric-networking-api-v1/src/testmod/java/net/fabricmc/fabric/test/networking/play/NetworkingPlayPacketTest.java @@ -45,6 +45,7 @@ public final class NetworkingPlayPacketTest implements ModInitializer { public static final Identifier TEST_CHANNEL = NetworkingTestmods.id("test_channel"); + private static final Identifier UNKNOWN_TEST_CHANNEL = NetworkingTestmods.id("unknown_test_channel"); public static void sendToTestChannel(ServerPlayerEntity player, String stuff) { ServerPlayNetworking.getSender(player).sendPacket(new OverlayPacket(Text.literal(stuff)), future -> { @@ -52,6 +53,10 @@ public static void sendToTestChannel(ServerPlayerEntity player, String stuff) { }); } + private static void sendToUnknownChannel(ServerPlayerEntity player) { + ServerPlayNetworking.getSender(player).sendPacket(UNKNOWN_TEST_CHANNEL, PacketByteBufs.create()); + } + public static void registerCommand(CommandDispatcher dispatcher) { NetworkingTestmods.LOGGER.info("Registering test command"); @@ -61,6 +66,10 @@ public static void registerCommand(CommandDispatcher dispat sendToTestChannel(ctx.getSource().getPlayer(), stuff); return Command.SINGLE_SUCCESS; })) + .then(literal("unknown").executes(ctx -> { + sendToUnknownChannel(ctx.getSource().getPlayer()); + return Command.SINGLE_SUCCESS; + })) .then(literal("bundled").executes(ctx -> { PacketByteBuf buf1 = PacketByteBufs.create(); buf1.writeText(Text.literal("bundled #1")); diff --git a/fabric-networking-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/networking/client/play/NetworkingPlayPacketClientTest.java b/fabric-networking-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/networking/client/play/NetworkingPlayPacketClientTest.java index 0c0fb240ba..e87babc834 100644 --- a/fabric-networking-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/networking/client/play/NetworkingPlayPacketClientTest.java +++ b/fabric-networking-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/networking/client/play/NetworkingPlayPacketClientTest.java @@ -16,19 +16,36 @@ package net.fabricmc.fabric.test.networking.client.play; +import com.mojang.brigadier.Command; + import net.minecraft.client.MinecraftClient; import net.minecraft.client.network.ClientPlayerEntity; +import net.minecraft.util.Identifier; import net.fabricmc.api.ClientModInitializer; +import net.fabricmc.fabric.api.client.command.v2.ClientCommandManager; +import net.fabricmc.fabric.api.client.command.v2.ClientCommandRegistrationCallback; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayConnectionEvents; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; +import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.PacketSender; +import net.fabricmc.fabric.test.networking.NetworkingTestmods; import net.fabricmc.fabric.test.networking.play.NetworkingPlayPacketTest; public final class NetworkingPlayPacketClientTest implements ClientModInitializer, ClientPlayNetworking.PlayPacketHandler { + private static final Identifier UNKNOWN_TEST_CHANNEL = NetworkingTestmods.id("unknown_test_channel"); + @Override public void onInitializeClient() { ClientPlayConnectionEvents.INIT.register((handler, client) -> ClientPlayNetworking.registerReceiver(NetworkingPlayPacketTest.OverlayPacket.PACKET_TYPE, this)); + + ClientCommandRegistrationCallback.EVENT.register((dispatcher, dedicated) -> dispatcher.register( + ClientCommandManager.literal("clientnetworktestcommand") + .then(ClientCommandManager.literal("unknown").executes(context -> { + ClientPlayNetworking.send(UNKNOWN_TEST_CHANNEL, PacketByteBufs.create()); + return Command.SINGLE_SUCCESS; + } + )))); } @Override