From 27167ffded5e7e0b9f5b2ce5e9373091c1e6fe80 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 7 Nov 2021 20:43:11 -0500 Subject: [PATCH 1/2] Do not track plugin channels registered per-player on the proxy We don't need to track this information since Velocity uses the JoinGame packet, which is about as good of a server rejoin mechanism we're likely to get in vanilla Minecraft. --- .../backend/BackendPlaySessionHandler.java | 9 +-- .../backend/TransitionSessionHandler.java | 6 -- .../client/ClientPlaySessionHandler.java | 9 --- .../connection/client/ConnectedPlayer.java | 13 ---- .../client/InitialConnectSessionHandler.java | 6 +- .../proxy/util/collect/CappedSet.java | 69 ------------------ .../proxy/util/collect/CappedSetTest.java | 71 ------------------- 7 files changed, 3 insertions(+), 180 deletions(-) delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/util/collect/CappedSet.java delete mode 100644 proxy/src/test/java/com/velocitypowered/proxy/util/collect/CappedSetTest.java diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 39cc3738ae..2e93208001 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -158,13 +158,8 @@ public boolean handle(PluginMessage packet) { return true; } - // We need to specially handle REGISTER and UNREGISTER packets. Later on, we'll write them to - // the client. - if (PluginMessageUtil.isRegister(packet)) { - serverConn.getPlayer().getKnownChannels().addAll(PluginMessageUtil.getChannels(packet)); - return false; - } else if (PluginMessageUtil.isUnregister(packet)) { - serverConn.getPlayer().getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet)); + // Register and unregister packets are simply forwarded to the server as-is. + if (PluginMessageUtil.isRegister(packet) || PluginMessageUtil.isUnregister(packet)) { return false; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java index 3cb811ba54..99fa5ac1be 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java @@ -175,12 +175,6 @@ public boolean handle(PluginMessage packet) { return true; } - if (PluginMessageUtil.isRegister(packet)) { - serverConn.getPlayer().getKnownChannels().addAll(PluginMessageUtil.getChannels(packet)); - } else if (PluginMessageUtil.isUnregister(packet)) { - serverConn.getPlayer().getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet)); - } - // We always need to handle plugin messages, for Forge compatibility. if (serverConn.getPhase().handle(serverConn, serverConn.getPlayer(), packet)) { // Handled, but check the server connection phase. 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 3c36ad33b4..7f72efc7a3 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 @@ -112,7 +112,6 @@ public void activated() { if (!channels.isEmpty()) { PluginMessage register = constructChannelsPacket(player.getProtocolVersion(), channels); player.getConnection().write(register); - player.getKnownChannels().addAll(channels); } } @@ -223,7 +222,6 @@ public boolean handle(PluginMessage packet) { + "ready. Channel: {}. Packet discarded.", packet.getChannel()); } else if (PluginMessageUtil.isRegister(packet)) { List channels = PluginMessageUtil.getChannels(packet); - player.getKnownChannels().addAll(channels); List channelIdentifiers = new ArrayList<>(); for (String channel : channels) { try { @@ -236,7 +234,6 @@ public boolean handle(PluginMessage packet) { ImmutableList.copyOf(channelIdentifiers))); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isUnregister(packet)) { - player.getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet)); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isMcBrand(packet)) { String brand = PluginMessageUtil.readBrandMessage(packet.content()); @@ -404,12 +401,6 @@ public void handleBackendJoinGame(JoinGame joinGame, VelocityServerConnection de } serverBossBars.clear(); - // Tell the server about this client's plugin message channels. - ProtocolVersion serverVersion = serverMc.getProtocolVersion(); - if (!player.getKnownChannels().isEmpty()) { - serverMc.delayedWrite(constructChannelsPacket(serverVersion, player.getKnownChannels())); - } - // If we had plugin messages queued during login/FML handshake, send them now. PluginMessage pm; while ((pm = loginPluginMessages.poll()) != null) { 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 88bc3dc24d..740eb9e9c5 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 @@ -69,12 +69,10 @@ import com.velocitypowered.proxy.tablist.VelocityTabListLegacy; import com.velocitypowered.proxy.util.ClosestLocaleMatcher; import com.velocitypowered.proxy.util.DurationUtils; -import com.velocitypowered.proxy.util.collect.CappedSet; import io.netty.buffer.ByteBufUtil; import io.netty.buffer.Unpooled; import java.net.InetSocketAddress; import java.util.ArrayDeque; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -107,7 +105,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { - private static final int MAX_PLUGIN_CHANNELS = 1024; private static final PlainComponentSerializer PASS_THRU_TRANSLATE = new PlainComponentSerializer( c -> "", TranslatableComponent::key); static final PermissionProvider DEFAULT_PERMISSIONS = s -> PermissionFunction.ALWAYS_UNDEFINED; @@ -134,7 +131,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { private final VelocityTabList tabList; private final VelocityServer server; private ClientConnectionPhase connectionPhase; - private final Collection knownChannels; private final CompletableFuture teardownFuture = new CompletableFuture<>(); private @MonotonicNonNull List serversToTry = null; private @MonotonicNonNull Boolean previousResourceResponse; @@ -157,7 +153,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { this.virtualHost = virtualHost; this.permissionFunction = PermissionFunction.ALWAYS_UNDEFINED; this.connectionPhase = connection.getType().getInitialClientPhase(); - this.knownChannels = CappedSet.create(MAX_PLUGIN_CHANNELS); this.onlineMode = onlineMode; if (connection.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { @@ -1021,14 +1016,6 @@ public void setPhase(ClientConnectionPhase connectionPhase) { this.connectionPhase = connectionPhase; } - /** - * Return all the plugin message channels "known" to the client. - * @return the channels - */ - public Collection getKnownChannels() { - return knownChannels; - } - private class IdentityImpl implements Identity { @Override public @NonNull UUID uuid() { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java index 8dcc5c0412..a93348127e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java @@ -39,11 +39,7 @@ public boolean handle(PluginMessage packet) { return true; } - if (PluginMessageUtil.isRegister(packet)) { - player.getKnownChannels().addAll(PluginMessageUtil.getChannels(packet)); - } else if (PluginMessageUtil.isUnregister(packet)) { - player.getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet)); - } else if (BungeeCordMessageResponder.isBungeeCordMessage(packet)) { + if (BungeeCordMessageResponder.isBungeeCordMessage(packet)) { return true; } serverConn.ensureConnected().write(packet.retain()); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/collect/CappedSet.java b/proxy/src/main/java/com/velocitypowered/proxy/util/collect/CappedSet.java deleted file mode 100644 index 8e54d77548..0000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/collect/CappedSet.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * 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 3 of the License, or - * (at your option) any later version. - * - * 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. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.util.collect; - -import com.google.common.base.Preconditions; -import com.google.common.collect.ForwardingSet; -import java.util.Collection; -import java.util.HashSet; -import java.util.Set; - -/** - * An unsynchronized collection that puts an upper bound on the size of the collection. - */ -public final class CappedSet extends ForwardingSet { - - private final Set delegate; - private final int upperSize; - - private CappedSet(Set delegate, int upperSize) { - this.delegate = delegate; - this.upperSize = upperSize; - } - - /** - * Creates a capped collection backed by a {@link HashSet}. - * @param maxSize the maximum size of the collection - * @param the type of elements in the collection - * @return the new collection - */ - public static Set create(int maxSize) { - return new CappedSet<>(new HashSet<>(), maxSize); - } - - @Override - protected Set delegate() { - return delegate; - } - - @Override - public boolean add(T element) { - if (this.delegate.size() >= upperSize) { - Preconditions.checkState(this.delegate.contains(element), - "collection is too large (%s >= %s)", - this.delegate.size(), this.upperSize); - return false; - } - return this.delegate.add(element); - } - - @Override - public boolean addAll(Collection collection) { - return this.standardAddAll(collection); - } -} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/util/collect/CappedSetTest.java b/proxy/src/test/java/com/velocitypowered/proxy/util/collect/CappedSetTest.java deleted file mode 100644 index 73943be90f..0000000000 --- a/proxy/src/test/java/com/velocitypowered/proxy/util/collect/CappedSetTest.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * 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 3 of the License, or - * (at your option) any later version. - * - * 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. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.util.collect; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import com.google.common.collect.ImmutableSet; -import java.util.Collection; -import java.util.Set; -import org.junit.jupiter.api.Test; - -class CappedSetTest { - - @Test - void basicVerification() { - Collection coll = CappedSet.create(1); - assertTrue(coll.add("coffee"), "did not add single item"); - assertThrows(IllegalStateException.class, () -> coll.add("tea"), - "item was added to collection although it is too full"); - assertEquals(1, coll.size(), "collection grew in size unexpectedly"); - } - - @Test - void testAddAll() { - Set doesFill1 = ImmutableSet.of("coffee", "tea"); - Set doesFill2 = ImmutableSet.of("chocolate"); - Set overfill = ImmutableSet.of("Coke", "Pepsi"); - - Collection coll = CappedSet.create(3); - assertTrue(coll.addAll(doesFill1), "did not add items"); - assertTrue(coll.addAll(doesFill2), "did not add items"); - assertThrows(IllegalStateException.class, () -> coll.addAll(overfill), - "items added to collection although it is too full"); - assertEquals(3, coll.size(), "collection grew in size unexpectedly"); - } - - @Test - void handlesSetBehaviorCorrectly() { - Set doesFill1 = ImmutableSet.of("coffee", "tea"); - Set doesFill2 = ImmutableSet.of("coffee", "chocolate"); - Set overfill = ImmutableSet.of("coffee", "Coke", "Pepsi"); - - Collection coll = CappedSet.create(3); - assertTrue(coll.addAll(doesFill1), "did not add items"); - assertTrue(coll.addAll(doesFill2), "did not add items"); - assertThrows(IllegalStateException.class, () -> coll.addAll(overfill), - "items added to collection although it is too full"); - - assertFalse(coll.addAll(doesFill1), "added items?!?"); - - assertEquals(3, coll.size(), "collection grew in size unexpectedly"); - } -} \ No newline at end of file From f1e3d8607869193203fe2a05f6762ea85daceb19 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 17 Aug 2023 23:46:02 -0400 Subject: [PATCH 2/2] fix --- .../proxy/connection/backend/TransitionSessionHandler.java | 1 - .../proxy/connection/client/InitialConnectSessionHandler.java | 1 - 2 files changed, 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java index 39961688de..5fe6886dca 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java @@ -36,7 +36,6 @@ import com.velocitypowered.proxy.protocol.packet.JoinGame; import com.velocitypowered.proxy.protocol.packet.KeepAlive; import com.velocitypowered.proxy.protocol.packet.PluginMessage; -import com.velocitypowered.proxy.protocol.util.PluginMessageUtil; import java.io.IOException; import java.util.concurrent.CompletableFuture; import org.apache.logging.log4j.LogManager; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java index 8e211ef42a..f264d262ea 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java @@ -24,7 +24,6 @@ import com.velocitypowered.proxy.connection.backend.BungeeCordMessageResponder; import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; import com.velocitypowered.proxy.protocol.packet.PluginMessage; -import com.velocitypowered.proxy.protocol.util.PluginMessageUtil; import io.netty.buffer.ByteBufUtil; import io.netty.buffer.Unpooled; import org.apache.logging.log4j.LogManager;