From 8b8fd284b37caf3b7073c22ea818b4a05f0c96b6 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 12 Jun 2020 07:24:17 -0400 Subject: [PATCH 1/4] Call DisconnectEvent explicitly instead of relying on ConnectedPlayer#teardown() to do it for us Should fix (but not verified) #289 --- .../connection/client/ConnectedPlayer.java | 19 ++++++++++++++++--- .../client/LoginSessionHandler.java | 8 ++++---- 2 files changed, 20 insertions(+), 7 deletions(-) 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 95cbdefb..c65af53e 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 @@ -104,6 +104,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { private final CompletableFuture teardownFuture = new CompletableFuture<>(); private @MonotonicNonNull List serversToTry = null; + private boolean explicitlyDisconnected = false; ConnectedPlayer(VelocityServer server, GameProfile profile, MinecraftConnection connection, @Nullable InetSocketAddress virtualHost, boolean onlineMode) { @@ -281,9 +282,20 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { @Override public void disconnect(Component reason) { + if (connection.eventLoop().inEventLoop()) { + disconnect0(reason, false); + } else { + connection.eventLoop().execute(() -> disconnect0(reason, false)); + } + } + + public void disconnect0(Component reason, boolean duringLogin) { logger.info("{} has disconnected: {}", this, LegacyComponentSerializer.legacy().serialize(reason)); + this.explicitlyDisconnected = true; connection.closeWith(Disconnect.create(reason)); + + server.getEventManager().fireAndForget(new DisconnectEvent(this, duringLogin)); } @Override @@ -574,10 +586,11 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { connectedServer.disconnect(); } boolean isConnected = server.getPlayer(this.getUniqueId()).isPresent(); - server.unregisterConnection(this); - server.getEventManager().fire(new DisconnectEvent(this, !isConnected)) - .thenRun(() -> this.teardownFuture.complete(null)); + if (!this.explicitlyDisconnected) { + server.getEventManager().fire(new DisconnectEvent(this, !isConnected)) + .thenRun(() -> this.teardownFuture.complete(null)); + } } public CompletableFuture getTeardownFuture() { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java index 380fd983..831304d2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java @@ -204,7 +204,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { inbound.getVirtualHost().orElse(null), onlineMode); this.connectedPlayer = player; if (!server.canRegisterConnection(player)) { - player.disconnect(VelocityMessages.ALREADY_CONNECTED); + player.disconnect0(VelocityMessages.ALREADY_CONNECTED, true); return CompletableFuture.completedFuture(null); } @@ -246,10 +246,10 @@ public class LoginSessionHandler implements MinecraftSessionHandler { Optional reason = event.getResult().getReason(); if (reason.isPresent()) { - player.disconnect(reason.get()); + player.disconnect0(reason.get(), true); } else { if (!server.registerConnection(player)) { - player.disconnect(VelocityMessages.ALREADY_CONNECTED); + player.disconnect0(VelocityMessages.ALREADY_CONNECTED, true); return; } @@ -269,7 +269,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { .thenRunAsync(() -> { Optional toTry = event.getInitialServer(); if (!toTry.isPresent()) { - player.disconnect(VelocityMessages.NO_AVAILABLE_SERVERS); + player.disconnect0(VelocityMessages.NO_AVAILABLE_SERVERS, true); return; } player.createConnectionRequest(toTry.get()).fireAndForget(); From 8a6e77689b4b1df41ee7d8a6a4a46fdea3b6d895 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 12 Jun 2020 07:26:25 -0400 Subject: [PATCH 2/4] Checkstyle strikes again --- .../proxy/connection/client/ConnectedPlayer.java | 5 +++++ 1 file changed, 5 insertions(+) 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 c65af53e..9078f1d1 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 @@ -289,6 +289,11 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { } } + /** + * Disconnects the player from the proxy. + * @param reason the reason for disconnecting the player + * @param duringLogin whether the disconnect happened during login + */ public void disconnect0(Component reason, boolean duringLogin) { logger.info("{} has disconnected: {}", this, LegacyComponentSerializer.legacy().serialize(reason)); From 0109f4477ab0b560203cee3ea571fdd66b4361e2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 12 Jun 2020 07:27:51 -0400 Subject: [PATCH 3/4] Complete the teardown future anyway --- .../proxy/connection/client/ConnectedPlayer.java | 2 ++ 1 file changed, 2 insertions(+) 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 9078f1d1..0dab8935 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 @@ -595,6 +595,8 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { if (!this.explicitlyDisconnected) { server.getEventManager().fire(new DisconnectEvent(this, !isConnected)) .thenRun(() -> this.teardownFuture.complete(null)); + } else { + this.teardownFuture.complete(null); } } From b79d2d4a946b112c249ab76d1a38f6805eb5d3a9 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 19 Jun 2020 05:22:19 -0400 Subject: [PATCH 4/4] Fix tab complete using proper vanilla limit. --- .../proxy/protocol/packet/TabCompleteRequest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/TabCompleteRequest.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/TabCompleteRequest.java index 5db35433..30bf616a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/TabCompleteRequest.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/TabCompleteRequest.java @@ -14,6 +14,8 @@ import org.checkerframework.checker.nullness.qual.Nullable; public class TabCompleteRequest implements MinecraftPacket { + private static final int VANILLA_MAX_TAB_COMPLETE_LEN = 2048; + private @Nullable String command; private int transactionId; private boolean assumeCommand; @@ -78,9 +80,9 @@ public class TabCompleteRequest implements MinecraftPacket { public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { if (version.compareTo(MINECRAFT_1_13) >= 0) { this.transactionId = ProtocolUtils.readVarInt(buf); - this.command = ProtocolUtils.readString(buf, Chat.MAX_SERVERBOUND_MESSAGE_LENGTH); + this.command = ProtocolUtils.readString(buf, VANILLA_MAX_TAB_COMPLETE_LEN); } else { - this.command = ProtocolUtils.readString(buf, Chat.MAX_SERVERBOUND_MESSAGE_LENGTH); + this.command = ProtocolUtils.readString(buf, VANILLA_MAX_TAB_COMPLETE_LEN); if (version.compareTo(MINECRAFT_1_9) >= 0) { this.assumeCommand = buf.readBoolean(); }