From 6d42a3c37cd2cc194c148ffcc1e0eb4d7144bc33 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Wed, 15 May 2019 17:04:25 -0400 Subject: [PATCH] Explicitly rewrite legacy plugin message channels for 1.13+ clients. --- .../connection/client/ConnectedPlayer.java | 3 +- .../proxy/protocol/packet/PluginMessage.java | 10 +++- .../protocol/util/PluginMessageUtil.java | 38 +++++++++++++ .../proxy/util/VelocityChannelRegistrar.java | 38 +++++++++---- .../protocol/util/PluginMessageUtilTest.java | 36 ++++++++++++ .../util/VelocityChannelRegistrarTest.java | 55 +++++++++++++++++++ 6 files changed, 164 insertions(+), 16 deletions(-) create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtilTest.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java 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 d27e21862..d06f797b4 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 @@ -671,8 +671,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { // Otherwise, we need to see if the player already knows this channel or it's known by the // proxy. - return minecraftOrFmlMessage || knownChannels.contains(message.getChannel()) - || server.getChannelRegistrar().registered(message.getChannel()); + return minecraftOrFmlMessage || knownChannels.contains(message.getChannel()); } private class ConnectionRequestBuilderImpl implements ConnectionRequestBuilder { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java index 8ad1c8847..6fdd884fd 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java @@ -1,11 +1,13 @@ package com.velocitypowered.proxy.protocol.packet; import static com.velocitypowered.proxy.connection.VelocityConstants.EMPTY_BYTE_ARRAY; +import static com.velocitypowered.proxy.protocol.util.PluginMessageUtil.transformLegacyToModernChannel; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.util.PluginMessageUtil; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -38,13 +40,16 @@ public class PluginMessage implements MinecraftPacket { public String toString() { return "PluginMessage{" + "channel='" + channel + '\'' - + ", data=" + + + ", data=" + '}'; } @Override public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { this.channel = ProtocolUtils.readString(buf); + if (version.compareTo(ProtocolVersion.MINECRAFT_1_13) < 0) { + this.channel = transformLegacyToModernChannel(this.channel); + } this.data = new byte[buf.readableBytes()]; buf.readBytes(data); } @@ -54,7 +59,8 @@ public class PluginMessage implements MinecraftPacket { if (channel == null) { throw new IllegalStateException("Channel is not specified."); } - ProtocolUtils.writeString(buf, channel); + ProtocolUtils.writeString(buf, version.compareTo(ProtocolVersion.MINECRAFT_1_13) >= 0 + ? channel : transformLegacyToModernChannel(this.channel)); buf.writeBytes(data); } 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 37b4cc876..3366de6ca 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 @@ -14,6 +14,8 @@ import io.netty.buffer.Unpooled; import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.List; +import java.util.Locale; +import java.util.regex.Pattern; public class PluginMessageUtil { @@ -143,4 +145,40 @@ public class PluginMessageUtil { return newMsg; } + private static final Pattern INVALID_IDENTIFIER_REGEX = Pattern.compile("[^a-z0-9\\-_]*"); + + /** + * Transform a plugin message channel from a "legacy" (<1.13) form to a modern one. + * @param name the existing name + * @return the new name + */ + public static String transformLegacyToModernChannel(String name) { + checkNotNull(name, "name"); + + if (name.indexOf(':') != -1) { + // Probably valid. We won't check this for now and go on faith. + return name; + } + + // Before falling into the fallback, explicitly rewrite certain messages. + switch (name) { + case REGISTER_CHANNEL_LEGACY: + return REGISTER_CHANNEL; + case UNREGISTER_CHANNEL_LEGACY: + return UNREGISTER_CHANNEL; + case BRAND_CHANNEL_LEGACY: + return BRAND_CHANNEL; + case "BungeeCord": + // This is a special historical case we are compelled to support for the benefit of + // BungeeQuack. + return "bungeecord:main"; + default: + // This is very likely a legacy name, so transform it. Velocity uses the same scheme as + // BungeeCord does to transform channels, but also removes clearly invalid characters as + // well. + String lower = name.toLowerCase(Locale.ROOT); + return "legacy:" + INVALID_IDENTIFIER_REGEX.matcher(lower).replaceAll(""); + } + } + } 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 05a38b669..73dd38647 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/VelocityChannelRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/VelocityChannelRegistrar.java @@ -1,15 +1,15 @@ package com.velocitypowered.proxy.util; import com.google.common.base.Preconditions; -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.ChannelRegistrar; import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; -import java.util.ArrayList; +import com.velocitypowered.proxy.protocol.util.PluginMessageUtil; import java.util.Collection; +import java.util.HashSet; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -28,7 +28,13 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { } for (ChannelIdentifier identifier : identifiers) { - identifierMap.put(identifier.getId(), identifier); + if (identifier instanceof MinecraftChannelIdentifier) { + identifierMap.put(identifier.getId(), identifier); + } else { + String rewritten = PluginMessageUtil.transformLegacyToModernChannel(identifier.getId()); + identifierMap.put(identifier.getId(), identifier); + identifierMap.put(rewritten, identifier); + } } } @@ -41,12 +47,22 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { } for (ChannelIdentifier identifier : identifiers) { - identifierMap.remove(identifier.getId()); + if (identifier instanceof MinecraftChannelIdentifier) { + identifierMap.remove(identifier.getId()); + } else { + String rewritten = PluginMessageUtil.transformLegacyToModernChannel(identifier.getId()); + identifierMap.remove(identifier.getId()); + identifierMap.remove(rewritten); + } } } - public Collection getIdsForLegacyConnections() { - return ImmutableList.copyOf(identifierMap.keySet()); + public Collection getLegacyChannelIds() { + Collection ids = new HashSet<>(); + for (ChannelIdentifier value : identifierMap.values()) { + ids.add(value.getId()); + } + return ids; } /** @@ -55,19 +71,17 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { * @return the channel IDs for Minecraft 1.13 and above */ public Collection getModernChannelIds() { - Collection ids = new ArrayList<>(); + Collection ids = new HashSet<>(); for (ChannelIdentifier value : identifierMap.values()) { if (value instanceof MinecraftChannelIdentifier) { ids.add(value.getId()); + } else { + ids.add(PluginMessageUtil.transformLegacyToModernChannel(value.getId())); } } return ids; } - public boolean registered(String id) { - return identifierMap.containsKey(id); - } - public @Nullable ChannelIdentifier getFromId(String id) { return identifierMap.get(id); } @@ -81,6 +95,6 @@ public class VelocityChannelRegistrar implements ChannelRegistrar { if (protocolVersion.compareTo(ProtocolVersion.MINECRAFT_1_13) >= 0) { return getModernChannelIds(); } - return getIdsForLegacyConnections(); + return getLegacyChannelIds(); } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtilTest.java b/proxy/src/test/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtilTest.java new file mode 100644 index 000000000..1db767f47 --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtilTest.java @@ -0,0 +1,36 @@ +package com.velocitypowered.proxy.protocol.util; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +class PluginMessageUtilTest { + + @Test + void transformLegacyToModernChannelWorksWithModern() { + assertEquals("minecraft:brand", PluginMessageUtil + .transformLegacyToModernChannel("minecraft:brand")); + assertEquals("velocity:test", PluginMessageUtil + .transformLegacyToModernChannel("velocity:test")); + } + + @Test + void transformLegacyToModernChannelRewritesSpecialCases() { + assertEquals("minecraft:brand", PluginMessageUtil + .transformLegacyToModernChannel("MC|Brand")); + assertEquals("minecraft:register", PluginMessageUtil + .transformLegacyToModernChannel("REGISTER")); + assertEquals("minecraft:unregister", PluginMessageUtil + .transformLegacyToModernChannel("UNREGISTER")); + assertEquals("bungeecord:main", PluginMessageUtil + .transformLegacyToModernChannel("BungeeCord")); + } + + @Test + void transformLegacyToModernChannelRewritesGeneral() { + assertEquals("legacy:example", PluginMessageUtil + .transformLegacyToModernChannel("Example")); + assertEquals("legacy:pskeepalive", PluginMessageUtil + .transformLegacyToModernChannel("PS|KeepAlive")); + } +} \ No newline at end of file diff --git a/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java b/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java new file mode 100644 index 000000000..f2f592160 --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/util/VelocityChannelRegistrarTest.java @@ -0,0 +1,55 @@ +package com.velocitypowered.proxy.util; + +import static org.junit.jupiter.api.Assertions.*; + +import com.google.common.collect.ImmutableSet; +import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; +import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; +import org.junit.jupiter.api.Test; + +class VelocityChannelRegistrarTest { + + private static final MinecraftChannelIdentifier MODERN = MinecraftChannelIdentifier + .create("velocity", "test"); + private static final LegacyChannelIdentifier SIMPLE_LEGACY = + new LegacyChannelIdentifier("VelocityTest"); + + private static final MinecraftChannelIdentifier MODERN_SPECIAL_REMAP = MinecraftChannelIdentifier + .create("bungeecord", "main"); + private static final LegacyChannelIdentifier SPECIAL_REMAP_LEGACY = + new LegacyChannelIdentifier("BungeeCord"); + + private static final String SIMPLE_LEGACY_REMAPPED = "legacy:velocitytest"; + + @Test + void register() { + VelocityChannelRegistrar registrar = new VelocityChannelRegistrar(); + registrar.register(MODERN, SIMPLE_LEGACY); + + // 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()); + assertEquals(ImmutableSet.of(SIMPLE_LEGACY.getId(), MODERN.getId()), registrar.getLegacyChannelIds()); + } + + @Test + void registerSpecialRewrite() { + VelocityChannelRegistrar registrar = new VelocityChannelRegistrar(); + 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(), SPECIAL_REMAP_LEGACY.getId()), + registrar.getLegacyChannelIds()); + } + + @Test + void unregister() { + VelocityChannelRegistrar registrar = new VelocityChannelRegistrar(); + registrar.register(MODERN, SIMPLE_LEGACY); + registrar.unregister(SIMPLE_LEGACY); + + assertEquals(ImmutableSet.of(MODERN.getId()), registrar.getModernChannelIds()); + assertEquals(ImmutableSet.of(MODERN.getId()), registrar.getLegacyChannelIds()); + } +} \ No newline at end of file