From 676ec9cb21d4372973b7f5443b76a5e0bba2f0f8 Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sun, 6 Apr 2025 20:17:28 +0100 Subject: [PATCH 1/5] preliminary cleanup of plugin message channel handling --- .../backend/BungeeCordMessageResponder.java | 11 ++-- .../client/ClientPlaySessionHandler.java | 18 ++----- .../connection/client/ConnectedPlayer.java | 4 +- .../protocol/util/PluginMessageUtil.java | 51 +++++++++++++++++-- .../proxy/util/VelocityChannelRegistrar.java | 16 +++--- 5 files changed, 67 insertions(+), 33 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java index 6161c4c2..a2ea94d6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java @@ -20,6 +20,7 @@ package com.velocitypowered.proxy.connection.backend; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.api.proxy.Player; import com.velocitypowered.api.proxy.ServerConnection; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; import com.velocitypowered.api.proxy.server.RegisteredServer; @@ -316,9 +317,9 @@ public class BungeeCordMessageResponder { }); } - static String getBungeeCordChannel(ProtocolVersion version) { - return version.noLessThan(ProtocolVersion.MINECRAFT_1_13) ? MODERN_CHANNEL.getId() - : LEGACY_CHANNEL.getId(); + static ChannelIdentifier getBungeeCordChannel(ProtocolVersion version) { + return version.noLessThan(ProtocolVersion.MINECRAFT_1_13) ? MODERN_CHANNEL + : LEGACY_CHANNEL; } // Note: this method will always release the buffer! @@ -329,8 +330,8 @@ public class BungeeCordMessageResponder { // Note: this method will always release the buffer! private static void sendServerResponse(ConnectedPlayer player, ByteBuf buf) { MinecraftConnection serverConnection = player.ensureAndGetCurrentServer().ensureConnected(); - String chan = getBungeeCordChannel(serverConnection.getProtocolVersion()); - PluginMessagePacket msg = new PluginMessagePacket(chan, buf); + ChannelIdentifier chan = getBungeeCordChannel(serverConnection.getProtocolVersion()); + PluginMessagePacket msg = new PluginMessagePacket(chan.getId(), buf); serverConnection.write(msg); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index a7e490ed..7c3a4486 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -162,7 +162,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { @Override public void activated() { configSwitchFuture = new CompletableFuture<>(); - Collection channels = + Collection channels = server.getChannelRegistrar().getChannelsForProtocol(player.getProtocolVersion()); if (!channels.isEmpty()) { PluginMessagePacket register = constructChannelsPacket(player.getProtocolVersion(), channels); @@ -310,22 +310,14 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { logger.warn("A plugin message was received while the backend server was not " + "ready. Channel: {}. Packet discarded.", packet.getChannel()); } else if (PluginMessageUtil.isRegister(packet)) { - List channels = PluginMessageUtil.getChannels(packet); + List channels = PluginMessageUtil.getChannels(packet, this.player.getProtocolVersion()); player.getClientsideChannels().addAll(channels); - List channelIdentifiers = new ArrayList<>(); - for (String channel : channels) { - try { - channelIdentifiers.add(MinecraftChannelIdentifier.from(channel)); - } catch (IllegalArgumentException e) { - channelIdentifiers.add(new LegacyChannelIdentifier(channel)); - } - } server.getEventManager() .fireAndForget( - new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channelIdentifiers))); + new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channels))); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isUnregister(packet)) { - player.getClientsideChannels().removeAll(PluginMessageUtil.getChannels(packet)); + player.getClientsideChannels().removeAll(PluginMessageUtil.getChannels(packet, this.player.getProtocolVersion())); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isMcBrand(packet)) { String brand = PluginMessageUtil.readBrandMessage(packet.content()); @@ -589,7 +581,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // Tell the server about the proxy's plugin message channels. ProtocolVersion serverVersion = serverMc.getProtocolVersion(); - final Collection channels = server.getChannelRegistrar() + final Collection channels = server.getChannelRegistrar() .getChannelsForProtocol(serverMc.getProtocolVersion()); if (!channels.isEmpty()) { serverMc.delayedWrite(constructChannelsPacket(serverVersion, channels)); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 0f60b4dd..8f89add5 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java @@ -175,7 +175,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, private final InternalTabList tabList; private final VelocityServer server; private ClientConnectionPhase connectionPhase; - private final Collection clientsideChannels; + private final Collection clientsideChannels; private final CompletableFuture teardownFuture = new CompletableFuture<>(); private @MonotonicNonNull List serversToTry = null; private final ResourcePackHandler resourcePackHandler; @@ -1351,7 +1351,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, * * @return the channels */ - public Collection getClientsideChannels() { + public Collection getClientsideChannels() { return clientsideChannels; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java index 1cfd4a6f..801a313a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java @@ -22,13 +22,20 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.collect.ImmutableList; import com.velocitypowered.api.network.ProtocolVersion; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; +import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; +import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; import com.velocitypowered.api.util.ProxyVersion; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder; import com.velocitypowered.proxy.protocol.packet.PluginMessagePacket; +import com.velocitypowered.proxy.util.except.QuietDecoderException; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.regex.Pattern; @@ -85,13 +92,15 @@ public final class PluginMessageUtil { .equals(UNREGISTER_CHANNEL); } + private static final QuietDecoderException ILLEGAL_CHANNEL = new QuietDecoderException("Illegal channel"); /** * Fetches all the channels in a register or unregister plugin message. * * @param message the message to get the channels from * @return the channels, as an immutable list */ - public static List getChannels(PluginMessagePacket message) { + public static List getChannels(PluginMessagePacket message, + ProtocolVersion protocolVersion) { checkNotNull(message, "message"); checkArgument(isRegister(message) || isUnregister(message), "Unknown channel type %s", message.getChannel()); @@ -100,8 +109,25 @@ public final class PluginMessageUtil { // has caused issues with 1.13+ compatibility. Just return an empty list. return ImmutableList.of(); } - String channels = message.content().toString(StandardCharsets.UTF_8); - return ImmutableList.copyOf(channels.split("\0")); + String payload = message.content().toString(StandardCharsets.UTF_8); + String[] channels = payload.split("\0"); + List channelIdentifiers = new ArrayList<>(); + try { + for (String channel : channels) { + if (protocolVersion.noLessThan(ProtocolVersion.MINECRAFT_1_13)) { + channelIdentifiers.add(MinecraftChannelIdentifier.from(channel)); + } else { + channelIdentifiers.add(new LegacyChannelIdentifier(channel)); + } + } + } catch (IllegalArgumentException e) { + if (MinecraftDecoder.DEBUG) { + throw e; + } else { + throw ILLEGAL_CHANNEL; + } + } + return ImmutableList.copyOf(channelIdentifiers); } /** @@ -112,16 +138,31 @@ public final class PluginMessageUtil { * @return the plugin message to send */ public static PluginMessagePacket constructChannelsPacket(ProtocolVersion protocolVersion, - Collection channels) { + Collection channels) { checkNotNull(channels, "channels"); checkArgument(!channels.isEmpty(), "no channels specified"); String channelName = protocolVersion.noLessThan(ProtocolVersion.MINECRAFT_1_13) ? REGISTER_CHANNEL : REGISTER_CHANNEL_LEGACY; ByteBuf contents = Unpooled.buffer(); - contents.writeCharSequence(String.join("\0", channels), StandardCharsets.UTF_8); + contents.writeCharSequence(joinChannels(channels), StandardCharsets.UTF_8); return new PluginMessagePacket(channelName, contents); } + private static String joinChannels(Collection channels) { + checkNotNull(channels, "channels"); + checkArgument(!channels.isEmpty(), "no channels specified"); + StringBuilder sb = new StringBuilder(); + Iterator iterator = channels.iterator(); + while (iterator.hasNext()) { + ChannelIdentifier channel = iterator.next(); + sb.append(channel.getId()); + if (iterator.hasNext()) { + sb.append('\0'); + } + } + return sb.toString(); + } + /** * Rewrites the brand message to indicate the presence of Velocity. * diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/VelocityChannelRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/util/VelocityChannelRegistrar.java index d28ab9bf..e878cfd8 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/VelocityChannelRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/VelocityChannelRegistrar.java @@ -79,10 +79,10 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { * * @return all legacy channel IDs */ - public Collection getLegacyChannelIds() { - Collection ids = new HashSet<>(); + public Collection getLegacyChannelIds() { + Collection ids = new HashSet<>(); for (ChannelIdentifier value : identifierMap.values()) { - ids.add(value.getId()); + ids.add(new LegacyChannelIdentifier(value.getId())); } return ids; } @@ -92,13 +92,13 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { * * @return the channel IDs for Minecraft 1.13 and above */ - public Collection getModernChannelIds() { - Collection ids = new HashSet<>(); + public Collection getModernChannelIds() { + Collection ids = new HashSet<>(); for (ChannelIdentifier value : identifierMap.values()) { if (value instanceof MinecraftChannelIdentifier) { - ids.add(value.getId()); + ids.add(value); } else { - ids.add(PluginMessageUtil.transformLegacyToModernChannel(value.getId())); + ids.add(MinecraftChannelIdentifier.from(PluginMessageUtil.transformLegacyToModernChannel(value.getId()))); } } return ids; @@ -114,7 +114,7 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { * @param protocolVersion the protocol version in use * @return the list of channels to register */ - public Collection getChannelsForProtocol(ProtocolVersion protocolVersion) { + public Collection getChannelsForProtocol(ProtocolVersion protocolVersion) { if (protocolVersion.noLessThan(ProtocolVersion.MINECRAFT_1_13)) { return getModernChannelIds(); } From b482443e7915890acb789421c4465cbae1e2833c Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sun, 6 Apr 2025 21:22:22 +0100 Subject: [PATCH 2/5] Fix spot --- .../proxy/connection/client/ClientPlaySessionHandler.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index 7c3a4486..1fd72063 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -30,8 +30,6 @@ import com.velocitypowered.api.event.player.TabCompleteEvent; import com.velocitypowered.api.event.player.configuration.PlayerEnteredConfigurationEvent; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.api.proxy.messages.ChannelIdentifier; -import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; -import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.ConnectionTypes; import com.velocitypowered.proxy.connection.MinecraftConnection; From 747f70d80ae104968ba3c255c0a460d8ddae785d Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sun, 6 Apr 2025 21:25:03 +0100 Subject: [PATCH 3/5] Appease checkstyle --- .../velocitypowered/proxy/protocol/util/PluginMessageUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java index 801a313a..800ca5df 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java @@ -93,6 +93,7 @@ public final class PluginMessageUtil { } private static final QuietDecoderException ILLEGAL_CHANNEL = new QuietDecoderException("Illegal channel"); + /** * Fetches all the channels in a register or unregister plugin message. * From 9c1be72db0475fdd5ddc31ea04910731ffe34ecc Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sun, 6 Apr 2025 21:25:08 +0100 Subject: [PATCH 4/5] Fix tests --- .../util/VelocityChannelRegistrarTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java b/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java index 504df681..b6fb9cbd 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java @@ -20,8 +20,10 @@ package com.velocitypowered.proxy.util; import static org.junit.jupiter.api.Assertions.assertEquals; import com.google.common.collect.ImmutableSet; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; class VelocityChannelRegistrarTest { @@ -46,9 +48,9 @@ class VelocityChannelRegistrarTest { // Two channels cover the modern channel (velocity:test) and the legacy-mapped channel // (legacy:velocitytest). Make sure they're what we expect. assertEquals(ImmutableSet.of(MODERN.getId(), SIMPLE_LEGACY_REMAPPED), registrar - .getModernChannelIds()); + .getModernChannelIds().stream().map(ChannelIdentifier::getId).collect(Collectors.toSet())); assertEquals(ImmutableSet.of(SIMPLE_LEGACY.getId(), MODERN.getId()), registrar - .getLegacyChannelIds()); + .getLegacyChannelIds().stream().map(ChannelIdentifier::getId).collect(Collectors.toSet())); } @Test @@ -57,9 +59,10 @@ class VelocityChannelRegistrarTest { registrar.register(SPECIAL_REMAP_LEGACY, MODERN_SPECIAL_REMAP); // This one, just one channel for the modern case. - assertEquals(ImmutableSet.of(MODERN_SPECIAL_REMAP.getId()), registrar.getModernChannelIds()); + assertEquals(ImmutableSet.of(MODERN_SPECIAL_REMAP.getId()), + registrar.getModernChannelIds().stream().map(ChannelIdentifier::getId).collect(Collectors.toSet())); assertEquals(ImmutableSet.of(MODERN_SPECIAL_REMAP.getId(), SPECIAL_REMAP_LEGACY.getId()), - registrar.getLegacyChannelIds()); + registrar.getLegacyChannelIds().stream().map(ChannelIdentifier::getId).collect(Collectors.toSet())); } @Test @@ -68,7 +71,9 @@ class VelocityChannelRegistrarTest { registrar.register(MODERN, SIMPLE_LEGACY); registrar.unregister(SIMPLE_LEGACY); - assertEquals(ImmutableSet.of(MODERN.getId()), registrar.getModernChannelIds()); - assertEquals(ImmutableSet.of(MODERN.getId()), registrar.getLegacyChannelIds()); + assertEquals(ImmutableSet.of(MODERN.getId()), + registrar.getModernChannelIds().stream().map(ChannelIdentifier::getId).collect(Collectors.toSet()));; + assertEquals(ImmutableSet.of(MODERN.getId()), + registrar.getLegacyChannelIds().stream().map(ChannelIdentifier::getId).collect(Collectors.toSet())); } } \ No newline at end of file From a51711e4bb59b4c59a9e3980cfba049a22c470f2 Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sat, 12 Apr 2025 16:20:07 +0100 Subject: [PATCH 5/5] Use an ImmutableList Builder --- .../proxy/protocol/util/PluginMessageUtil.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java index 800ca5df..76d6f727 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java @@ -112,7 +112,7 @@ public final class PluginMessageUtil { } String payload = message.content().toString(StandardCharsets.UTF_8); String[] channels = payload.split("\0"); - List channelIdentifiers = new ArrayList<>(); + ImmutableList.Builder channelIdentifiers = ImmutableList.builderWithExpectedSize(channels.length); try { for (String channel : channels) { if (protocolVersion.noLessThan(ProtocolVersion.MINECRAFT_1_13)) { @@ -128,7 +128,7 @@ public final class PluginMessageUtil { throw ILLEGAL_CHANNEL; } } - return ImmutableList.copyOf(channelIdentifiers); + return channelIdentifiers.build(); } /**