From 01be081bda2ecb2c53c15574fa6ff2350291c732 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 3 Sep 2018 15:01:18 -0400 Subject: [PATCH 01/49] Update link to Velocity repo --- .../java/com/velocitypowered/proxy/command/VelocityCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommand.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommand.java index 94825af5..f3acb9ca 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommand.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommand.java @@ -29,7 +29,7 @@ public class VelocityCommand implements Command { .append(TextComponent.of(" or the ").resetStyle()) .append(TextComponent.builder("Velocity GitHub") .color(TextColor.GREEN) - .clickEvent(new ClickEvent(ClickEvent.Action.OPEN_URL, "https://github.com/astei/velocity")) + .clickEvent(new ClickEvent(ClickEvent.Action.OPEN_URL, "https://github.com/VelocityPowered/Velocity")) .build()) .build(); From 3b5d20e62de16a8c68b7e54732b188bdbbd1dea2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 3 Sep 2018 21:02:11 -0400 Subject: [PATCH 02/49] Include git revision and build number --- build.gradle | 11 +++++++++++ proxy/build.gradle | 5 ++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 7de9e8fc..ecccaa6b 100644 --- a/build.gradle +++ b/build.gradle @@ -27,6 +27,17 @@ allprojects { return os.toString().trim() } } + + getCurrentShortRevision = { + new ByteArrayOutputStream().withStream { os -> + exec { + executable = "git" + args = ["rev-parse", "HEAD"] + standardOutput = os + } + return os.toString().trim().substring(0, 8) + } + } } repositories { diff --git a/proxy/build.gradle b/proxy/build.gradle index 547faed2..56507cb0 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -14,8 +14,11 @@ compileTestJava { jar { manifest { + def buildNumber = System.getenv("BUILD_NUMBER") ?: "unknown" + def version = "${project.version} (git-${project.ext.getCurrentShortRevision()}, build ${buildNumber})" + attributes 'Main-Class': 'com.velocitypowered.proxy.Velocity' - attributes 'Implementation-Version': project.version + attributes 'Implementation-Version': version } } From c30cc664613e216f2f71f2d3fe5c67f5b03aa769 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 3 Sep 2018 23:30:18 -0400 Subject: [PATCH 03/49] Test logic when exception is thrown in thread --- .../RecordingThreadFactoryTest.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java b/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java index cac1ef29..33fabfcf 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java @@ -4,6 +4,7 @@ import org.junit.jupiter.api.Test; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import static org.junit.jupiter.api.Assertions.*; @@ -33,4 +34,36 @@ class RecordingThreadFactoryTest { Thread.sleep(10); assertEquals(0, factory.size()); } + + @Test + void cleanUpAfterExceptionThrown() throws Exception { + CountDownLatch started = new CountDownLatch(1); + CountDownLatch endThread = new CountDownLatch(1); + CountDownLatch hasEnded = new CountDownLatch(1); + RecordingThreadFactory factory = new RecordingThreadFactory((ThreadFactory) r -> { + Thread t = new Thread(r); + t.setUncaughtExceptionHandler((t1, e) -> hasEnded.countDown()); + return t; + }); + factory.newThread(() -> { + started.countDown(); + assertTrue(factory.currentlyInFactory()); + assertEquals(1, factory.size()); + try { + endThread.await(); + } catch (InterruptedException e) { + fail(e); + } + throw new RuntimeException(""); + }).start(); + started.await(); + assertFalse(factory.currentlyInFactory()); + assertEquals(1, factory.size()); + endThread.countDown(); + hasEnded.await(); + + // Wait a little bit to ensure the thread got shut down + Thread.sleep(10); + assertEquals(0, factory.size()); + } } \ No newline at end of file From c5dcfb1ba6b5e8110dd38393bacc964915217b35 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 3 Sep 2018 23:45:11 -0400 Subject: [PATCH 04/49] Prettier, conciser, and more useful ProtocolVersion#toString() --- .../proxy/protocol/StateRegistry.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/StateRegistry.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/StateRegistry.java index d45d9404..91fab4eb 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/StateRegistry.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/StateRegistry.java @@ -1,5 +1,6 @@ package com.velocitypowered.proxy.protocol; +import com.google.common.base.Strings; import com.google.common.primitives.ImmutableIntArray; import com.velocitypowered.proxy.protocol.packet.*; import io.netty.util.collection.IntObjectHashMap; @@ -258,9 +259,18 @@ public enum StateRegistry { @Override public String toString() { + StringBuilder mappingAsString = new StringBuilder("{"); + for (Object2IntMap.Entry> entry : packetClassToId.object2IntEntrySet()) { + mappingAsString.append(entry.getKey().getSimpleName()).append(" -> ") + .append("0x") + .append(Strings.padStart(Integer.toHexString(entry.getIntValue()), 2, '0')) + .append(", "); + } + mappingAsString.setLength(mappingAsString.length() - 2); + mappingAsString.append("}"); return "ProtocolVersion{" + "id=" + id + - ", packetClassToId=" + packetClassToId + + ", packetClassToId=" + mappingAsString.toString() + '}'; } } From c021eb2020c6baf2c7c9fb116323e285cb2d185c Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 4 Sep 2018 00:50:24 -0400 Subject: [PATCH 05/49] Don't output ProtocolVersion toString() in decode errors This information is extremely technical in nature and the data is already available for developer use by examining StateRegistry. --- .../proxy/protocol/netty/MinecraftDecoder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java index 4e05b364..07ddb287 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java @@ -38,11 +38,11 @@ public class MinecraftDecoder extends MessageToMessageDecoder { packet.decode(msg, direction, protocolVersion.id); } catch (Exception e) { throw new CorruptedFrameException("Error decoding " + packet.getClass() + " Direction " + direction - + " Protocol " + protocolVersion + " State " + state + " ID " + Integer.toHexString(packetId), e); + + " Protocol " + protocolVersion.id + " State " + state + " ID " + Integer.toHexString(packetId), e); } if (msg.isReadable()) { throw new CorruptedFrameException("Did not read full packet for " + packet.getClass() + " Direction " + direction - + " Protocol " + protocolVersion + " State " + state + " ID " + Integer.toHexString(packetId)); + + " Protocol " + protocolVersion.id + " State " + state + " ID " + Integer.toHexString(packetId)); } out.add(packet); } From 69e88ca6fd7341d6ee2aa9c2c7662b2cdcfbad7f Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 4 Sep 2018 01:12:20 -0400 Subject: [PATCH 06/49] Adds KickedFromServerEvent. Fixes #49 --- .../event/player/KickedFromServerEvent.java | 98 +++++++++++++++++++ .../connection/client/ConnectedPlayer.java | 30 ++++-- 2 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java diff --git a/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java new file mode 100644 index 00000000..d92d44ec --- /dev/null +++ b/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java @@ -0,0 +1,98 @@ +package com.velocitypowered.api.event.player; + +import com.google.common.base.Preconditions; +import com.velocitypowered.api.event.ResultedEvent; +import com.velocitypowered.api.proxy.Player; +import com.velocitypowered.api.proxy.server.ServerInfo; +import net.kyori.text.Component; +import org.checkerframework.checker.nullness.qual.NonNull; + +/** + * Fired when a player is kicked from a server. You may either allow Velocity to kick the player (with an optional reason + * override) or redirect the player to a separate server. + */ +public class KickedFromServerEvent implements ResultedEvent { + private final Player player; + private final ServerInfo server; + private final Component originalReason; + private final boolean duringLogin; + private ServerKickResult result; + + public KickedFromServerEvent(Player player, ServerInfo server, Component originalReason, boolean duringLogin, Component fancyReason) { + this.player = Preconditions.checkNotNull(player, "player"); + this.server = Preconditions.checkNotNull(server, "server"); + this.originalReason = Preconditions.checkNotNull(originalReason, "originalReason"); + this.duringLogin = duringLogin; + this.result = new DisconnectPlayer(fancyReason); + } + + @Override + public ServerKickResult getResult() { + return result; + } + + @Override + public void setResult(@NonNull ServerKickResult result) { + this.result = Preconditions.checkNotNull(result, "result"); + } + + public Player getPlayer() { + return player; + } + + public ServerInfo getServer() { + return server; + } + + public Component getOriginalReason() { + return originalReason; + } + + public boolean kickedDuringLogin() { + return duringLogin; + } + + public interface ServerKickResult extends ResultedEvent.Result {} + + public static class DisconnectPlayer implements ServerKickResult { + private final Component component; + + private DisconnectPlayer(Component component) { + this.component = Preconditions.checkNotNull(component, "component"); + } + + @Override + public boolean isAllowed() { + return true; + } + + public Component getReason() { + return component; + } + + public static DisconnectPlayer create(Component component) { + return new DisconnectPlayer(component); + } + } + + public static class RedirectPlayer implements ServerKickResult { + private final ServerInfo server; + + private RedirectPlayer(ServerInfo server) { + this.server = Preconditions.checkNotNull(server, "server"); + } + + @Override + public boolean isAllowed() { + return false; + } + + public ServerInfo getServer() { + return server; + } + + public static RedirectPlayer create(ServerInfo info) { + return new RedirectPlayer(info); + } + } +} 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 d8c6765f..52657870 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 @@ -2,6 +2,7 @@ package com.velocitypowered.proxy.connection.client; import com.google.common.base.Preconditions; import com.google.gson.JsonObject; +import com.velocitypowered.api.event.player.KickedFromServerEvent; import com.velocitypowered.api.event.player.PlayerSettingsChangedEvent; import com.velocitypowered.api.event.player.ServerPreConnectEvent; import com.velocitypowered.api.permission.PermissionFunction; @@ -36,6 +37,7 @@ import net.kyori.text.serializer.PlainComponentSerializer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; import java.net.InetSocketAddress; import java.util.List; @@ -190,7 +192,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { logger.error("{}: unable to connect to server {}", this, info.getName(), throwable); userMessage = "Exception connecting to server " + info.getName(); } - handleConnectionException(info, TextComponent.builder() + handleConnectionException(info, null, TextComponent.builder() .content(userMessage + ": ") .color(TextColor.RED) .append(TextComponent.of(error, TextColor.WHITE)) @@ -205,14 +207,15 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { } else { logger.error("{}: disconnected while connecting to {}: {}", this, info.getName(), plainTextReason); } - handleConnectionException(info, TextComponent.builder() + handleConnectionException(info, disconnectReason, TextComponent.builder() .content("Unable to connect to " + info.getName() + ": ") .color(TextColor.RED) .append(disconnectReason) .build()); } - public void handleConnectionException(ServerInfo info, Component disconnectReason) { + private void handleConnectionException(ServerInfo info, @Nullable Component kickReason, Component friendlyReason) { + boolean alreadyConnected = connectedServer != null && connectedServer.getServerInfo().equals(info);; connectionInFlight = null; if (connectedServer == null) { // The player isn't yet connected to a server. @@ -220,14 +223,27 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { if (nextServer.isPresent()) { createConnectionRequest(nextServer.get()).fireAndForget(); } else { - connection.closeWith(Disconnect.create(disconnectReason)); + connection.closeWith(Disconnect.create(friendlyReason)); } } else if (connectedServer.getServerInfo().equals(info)) { // Already connected to the server being disconnected from. - // TODO: ServerKickEvent - connection.closeWith(Disconnect.create(disconnectReason)); + if (kickReason != null) { + server.getEventManager().fire(new KickedFromServerEvent(this, info, kickReason, !alreadyConnected, friendlyReason)) + .thenAcceptAsync(event -> { + if (event.getResult() instanceof KickedFromServerEvent.DisconnectPlayer) { + KickedFromServerEvent.DisconnectPlayer res = (KickedFromServerEvent.DisconnectPlayer) event.getResult(); + connection.closeWith(Disconnect.create(res.getReason())); + } else if (event.getResult() instanceof KickedFromServerEvent.RedirectPlayer) { + KickedFromServerEvent.RedirectPlayer res = (KickedFromServerEvent.RedirectPlayer) event.getResult(); + createConnectionRequest(res.getServer()).fireAndForget(); + } else { + // In case someone gets creative, assume we want to disconnect the player. + connection.closeWith(Disconnect.create(friendlyReason)); + } + }, connection.getChannel().eventLoop()); + } } else { - connection.write(Chat.create(disconnectReason)); + connection.write(Chat.create(friendlyReason)); } } From 108996fc1fd4a4000ba2dd67eef993034caafc77 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 4 Sep 2018 01:13:06 -0400 Subject: [PATCH 07/49] Missed this --- .../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 52657870..f418854a 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 @@ -241,6 +241,8 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { connection.closeWith(Disconnect.create(friendlyReason)); } }, connection.getChannel().eventLoop()); + } else { + connection.closeWith(Disconnect.create(friendlyReason)); } } else { connection.write(Chat.create(friendlyReason)); From d2133bf0b49ef6f9cbe93dc295e1971e9a5566e5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 4 Sep 2018 01:32:50 -0400 Subject: [PATCH 08/49] Javadocs here --- .../event/player/KickedFromServerEvent.java | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java index d92d44ec..04359eea 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java @@ -52,8 +52,14 @@ public class KickedFromServerEvent implements ResultedEvent Date: Tue, 4 Sep 2018 01:37:08 -0400 Subject: [PATCH 09/49] Enhance config checks. --- .../proxy/config/VelocityConfiguration.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index f426a2c9..2a7e214a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -103,13 +103,13 @@ public class VelocityConfiguration extends AnnotatedConfig { if (bind.isEmpty()) { logger.error("'bind' option is empty."); valid = false; - } - - try { - AddressUtil.parseAddress(bind); - } catch (IllegalArgumentException e) { - logger.error("'bind' option does not specify a valid IP address.", e); - valid = false; + } else { + try { + AddressUtil.parseAddress(bind); + } catch (IllegalArgumentException e) { + logger.error("'bind' option does not specify a valid IP address.", e); + valid = false; + } } if (!onlineMode) { @@ -118,11 +118,11 @@ public class VelocityConfiguration extends AnnotatedConfig { switch (playerInfoForwardingMode) { case NONE: - logger.info("Player info forwarding is disabled! All players will appear to be connecting from the proxy and will have offline-mode UUIDs."); + logger.warn("Player info forwarding is disabled! All players will appear to be connecting from the proxy and will have offline-mode UUIDs."); break; case MODERN: - if (forwardingSecret.length == 0) { - logger.error("You don't have a forwarding secret set."); + if (forwardingSecret == null || forwardingSecret.length == 0) { + logger.error("You don't have a forwarding secret set. This is required for security."); valid = false; } break; @@ -148,7 +148,7 @@ public class VelocityConfiguration extends AnnotatedConfig { for (String s : servers.getAttemptConnectionOrder()) { if (!servers.getServers().containsKey(s)) { - logger.error("Fallback server " + s + " doesn't exist!"); + logger.error("Fallback server " + s + " is not registered in your configuration!"); valid = false; } } @@ -165,18 +165,18 @@ public class VelocityConfiguration extends AnnotatedConfig { logger.error("Invalid compression level {}", advanced.compressionLevel); valid = false; } else if (advanced.compressionLevel == 0) { - logger.warn("ALL packets going through the proxy are going to be uncompressed. This will increase bandwidth usage."); + logger.warn("ALL packets going through the proxy will be uncompressed. This will increase bandwidth usage."); } if (advanced.compressionThreshold < -1) { logger.error("Invalid compression threshold {}", advanced.compressionLevel); valid = false; } else if (advanced.compressionThreshold == 0) { - logger.warn("ALL packets going through the proxy are going to be compressed. This may hurt performance."); + logger.warn("ALL packets going through the proxy will be compressed. This will compromise throughput and increase CPU usage!"); } if (advanced.loginRatelimit < 0) { - logger.error("Invalid login ratelimit {}", advanced.loginRatelimit); + logger.error("Invalid login ratelimit {}ms", advanced.loginRatelimit); valid = false; } @@ -217,7 +217,7 @@ public class VelocityConfiguration extends AnnotatedConfig { if (motd.startsWith("{")) { motdAsComponent = ComponentSerializers.JSON.deserialize(motd); } else { - motdAsComponent = ComponentSerializers.LEGACY.deserialize(LegacyChatColorUtils.translate('&', motd)); + motdAsComponent = ComponentSerializers.LEGACY.deserialize(motd, '&'); } } return motdAsComponent; From 5a424f5cbba30519990b7d3453446102c4cd9a4b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 4 Sep 2018 01:39:51 -0400 Subject: [PATCH 10/49] Include version in boot message --- proxy/src/main/java/com/velocitypowered/proxy/Velocity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java b/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java index 9c1ee307..c628775c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java @@ -19,7 +19,7 @@ public class Velocity { public static void main(String... args) { startTime = System.currentTimeMillis(); - logger.info("Booting up Velocity..."); + logger.info("Booting up Velocity {}...", Velocity.class.getPackage().getImplementationVersion()); VelocityServer server = new VelocityServer(); server.start(); From 9888f6f6f09140d5f1b8a81e7af013a0b3fb5415 Mon Sep 17 00:00:00 2001 From: Daniel Naylor Date: Wed, 5 Sep 2018 19:08:29 +0100 Subject: [PATCH 11/49] First attempt at getting Forge working nicely with Velocity. --- .../velocitypowered/api/util/GameProfile.java | 2 +- .../proxy/connection/MinecraftConnection.java | 9 +++++++++ .../proxy/connection/VelocityConstants.java | 4 ++++ .../backend/BackendPlaySessionHandler.java | 15 +++++++++++++++ .../backend/LoginSessionHandler.java | 4 ++++ .../backend/VelocityServerConnection.java | 18 ++++++++++++++++++ .../client/ClientPlaySessionHandler.java | 6 ++++++ .../connection/client/ConnectedPlayer.java | 8 ++++++++ .../client/HandshakeSessionHandler.java | 9 ++++++--- .../connection/client/LoginSessionHandler.java | 9 +++++++++ 10 files changed, 80 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/util/GameProfile.java b/api/src/main/java/com/velocitypowered/api/util/GameProfile.java index 5fd71643..91300d1c 100644 --- a/api/src/main/java/com/velocitypowered/api/util/GameProfile.java +++ b/api/src/main/java/com/velocitypowered/api/util/GameProfile.java @@ -57,7 +57,7 @@ public final class GameProfile { '}'; } - public final class Property { + public final static class Property { private final String name; private final String value; private final String signature; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java index 6612a9af..3cef702a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -45,6 +45,7 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { private MinecraftSessionHandler sessionHandler; private int protocolVersion; private MinecraftConnectionAssociation association; + private boolean isLegacyForge; private final VelocityServer server; public MinecraftConnection(Channel channel, VelocityServer server) { @@ -222,4 +223,12 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { public void setAssociation(MinecraftConnectionAssociation association) { this.association = association; } + + public boolean isLegacyForge() { + return isLegacyForge; + } + + public void setLegacyForge(boolean isForge) { + this.isLegacyForge = isForge; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java index f95053e7..b6956336 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java @@ -6,4 +6,8 @@ public class VelocityConstants { } public static final String VELOCITY_IP_FORWARDING_CHANNEL = "velocity:player_info"; + + public static final String FORGE_LEGACY_HANDSHAKE_CHANNEL = "FML|HS"; + + public static final byte[] FORGE_LEGACY_HANDSHAKE_RESET_DATA = new byte[] { -2, 0 }; } 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 60ed678c..e013c0b0 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 @@ -4,6 +4,7 @@ import com.velocitypowered.api.event.player.ServerConnectedEvent; import com.velocitypowered.api.proxy.messages.ChannelSide; import com.velocitypowered.api.proxy.messages.MessageHandler; import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolConstants; @@ -46,6 +47,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { connection.getPlayer().handleConnectionException(connection.getServerInfo(), original); } else if (packet instanceof JoinGame) { playerHandler.handleBackendJoinGame((JoinGame) packet); + connection.setHasCompletedJoin(true); } else if (packet instanceof BossBar) { BossBar bossBar = (BossBar) packet; switch (bossBar.getAction()) { @@ -68,6 +70,19 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { return; } + if (!connection.hasCompletedJoin() && pm.getChannel().equals(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) { + if (!connection.isModded()) { + connection.setModded(true); + + // We must always reset the handshake before a modded connection is established. + connection.getPlayer().sendLegacyForgeHandshakeResetPacket(); + } + + // Always forward these messages during login + connection.getPlayer().getConnection().write(pm); + return; + } + MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(connection, ChannelSide.FROM_SERVER, pm); if (status == MessageHandler.ForwardStatus.FORWARD) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java index fba41095..4d011110 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java @@ -84,6 +84,10 @@ public class LoginSessionHandler implements MinecraftSessionHandler { connection.getPlayer().getConnection().setSessionHandler(new ClientPlaySessionHandler(server, connection.getPlayer())); } else { // The previous server connection should become obsolete. + // Before we remove it, if the server we are departing is modded, we must always reset the client state. + if (existingConnection.isModded()) { + connection.getPlayer().sendLegacyForgeHandshakeResetPacket(); + } existingConnection.disconnect(); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index 367e3346..a387661d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -43,6 +43,8 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, private final ConnectedPlayer proxyPlayer; private final VelocityServer server; private MinecraftConnection minecraftConnection; + private boolean isModded = false; + private boolean hasCompletedJoin = false; public VelocityServerConnection(ServerInfo target, ConnectedPlayer proxyPlayer, VelocityServer server) { this.serverInfo = target; @@ -154,4 +156,20 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, message.setData(data); minecraftConnection.write(message); } + + public boolean isModded() { + return isModded; + } + + public void setModded(boolean modded) { + isModded = modded; + } + + public boolean hasCompletedJoin() { + return hasCompletedJoin; + } + + public void setHasCompletedJoin(boolean hasCompletedJoin) { + this.hasCompletedJoin = hasCompletedJoin; + } } 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 9c36766e..3a65e591 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 @@ -235,6 +235,12 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { return; } + if (player.getConnectedServer().isModded() && !player.getConnectedServer().hasCompletedJoin()) { + // Ensure that the messages are forwarded + player.getConnectedServer().getMinecraftConnection().write(packet); + return; + } + MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(player, ChannelSide.FROM_CLIENT, packet); if (status == MessageHandler.ForwardStatus.FORWARD) { 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 f418854a..b3973aae 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 @@ -15,6 +15,7 @@ import com.velocitypowered.api.util.MessagePosition; import com.velocitypowered.api.proxy.Player; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.MinecraftConnectionAssociation; +import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.proxy.connection.util.ConnectionMessages; import com.velocitypowered.proxy.connection.util.ConnectionRequestResults; import com.velocitypowered.api.util.GameProfile; @@ -294,6 +295,13 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { this.connectedServer = serverConnection; } + public void sendLegacyForgeHandshakeResetPacket() { + PluginMessage resetPacket = new PluginMessage(); + resetPacket.setChannel(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL); + resetPacket.setData(VelocityConstants.FORGE_LEGACY_HANDSHAKE_RESET_DATA); + connection.write(resetPacket); + } + public void close(TextComponent reason) { connection.closeWith(Disconnect.create(reason)); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java index 5a5abaf6..b6b2dafe 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java @@ -70,9 +70,12 @@ public class HandshakeSessionHandler implements MinecraftSessionHandler { return; } - // Make sure legacy forwarding is not in use on this connection. Make sure that we do _not_ reject Forge, - // although Velocity does not yet support Forge. - if (handshake.getServerAddress().contains("\0") && !handshake.getServerAddress().endsWith("\0FML\0")) { + // Determine if we're using Forge (1.8 to 1.12, may not be the case in 1.13) and store that in the connection + boolean isForge = handshake.getServerAddress().endsWith("\0FML\0"); + connection.setLegacyForge(isForge); + + // Make sure legacy forwarding is not in use on this connection. Make sure that we do _not_ reject Forge + if (handshake.getServerAddress().contains("\0") && !isForge) { connection.closeWith(Disconnect.create(TextComponent.of("Running Velocity behind Velocity is unsupported."))); return; } 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 df990b50..ff728178 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 @@ -8,6 +8,7 @@ import com.velocitypowered.api.event.permission.PermissionsSetupEvent; import com.velocitypowered.api.event.player.GameProfileRequestEvent; import com.velocitypowered.api.proxy.InboundConnection; import com.velocitypowered.api.proxy.server.ServerInfo; +import com.velocitypowered.proxy.config.PlayerInfoForwarding; import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.api.util.GameProfile; import com.velocitypowered.proxy.protocol.MinecraftPacket; @@ -31,7 +32,9 @@ import java.net.MalformedURLException; import java.net.URL; import java.security.GeneralSecurityException; import java.security.KeyPair; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Optional; import java.util.concurrent.ThreadLocalRandom; @@ -176,6 +179,12 @@ public class LoginSessionHandler implements MinecraftSessionHandler { } private void initializePlayer(GameProfile profile, boolean onlineMode) { + if (inbound.isLegacyForge() && server.getConfiguration().getPlayerInfoForwardingMode() == PlayerInfoForwarding.LEGACY) { + // We want to add the FML token to the properties + List properties = new ArrayList<>(profile.getProperties()); + properties.add(new GameProfile.Property("forgeClient", "true", "")); + profile = new GameProfile(profile.getId(), profile.getName(), properties); + } GameProfileRequestEvent profileRequestEvent = new GameProfileRequestEvent(apiInbound, profile, onlineMode); server.getEventManager().fire(profileRequestEvent).thenCompose(profileEvent -> { From 10da7daf1d75c1bcdd26d048a3070eadd1a2987d Mon Sep 17 00:00:00 2001 From: Daniel Naylor Date: Wed, 5 Sep 2018 19:19:37 +0100 Subject: [PATCH 12/49] Send the FML marker if we are not performing legacy forwarding. --- .../proxy/connection/backend/VelocityServerConnection.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index a387661d..6d37a8b6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -109,6 +109,8 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, handshake.setProtocolVersion(proxyPlayer.getConnection().getProtocolVersion()); if (forwardingMode == PlayerInfoForwarding.LEGACY) { handshake.setServerAddress(createBungeeForwardingAddress()); + } else if (proxyPlayer.getConnection().isLegacyForge()) { + handshake.setServerAddress(handshake.getServerAddress() + "\0FML\0"); } else { handshake.setServerAddress(serverInfo.getAddress().getHostString()); } From 7a5857a0b2ddc1116ba2fa91eef8786b5be72a0c Mon Sep 17 00:00:00 2001 From: Daniel Naylor Date: Thu, 6 Sep 2018 19:15:18 +0100 Subject: [PATCH 13/49] Modded clients can connect to vanilla servers again. Modded to modded seems to need some work still... --- .../proxy/connection/MinecraftConnection.java | 9 +++++++++ .../backend/BackendPlaySessionHandler.java | 13 +++++++------ .../connection/client/ClientPlaySessionHandler.java | 2 ++ .../proxy/connection/client/ConnectedPlayer.java | 11 +++++++---- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java index 3cef702a..94293ebc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -47,6 +47,7 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { private MinecraftConnectionAssociation association; private boolean isLegacyForge; private final VelocityServer server; + private boolean canSendLegacyFMLResetPacket = false; public MinecraftConnection(Channel channel, VelocityServer server) { this.channel = channel; @@ -231,4 +232,12 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { public void setLegacyForge(boolean isForge) { this.isLegacyForge = isForge; } + + public boolean canSendLegacyFMLResetPacket() { + return canSendLegacyFMLResetPacket; + } + + public void setCanSendLegacyFMLResetPacket(boolean canSendLegacyFMLResetPacket) { + this.canSendLegacyFMLResetPacket = isLegacyForge && canSendLegacyFMLResetPacket; + } } 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 e013c0b0..77ea3faf 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 @@ -47,7 +47,6 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { connection.getPlayer().handleConnectionException(connection.getServerInfo(), original); } else if (packet instanceof JoinGame) { playerHandler.handleBackendJoinGame((JoinGame) packet); - connection.setHasCompletedJoin(true); } else if (packet instanceof BossBar) { BossBar bossBar = (BossBar) packet; switch (bossBar.getAction()) { @@ -74,7 +73,8 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { if (!connection.isModded()) { connection.setModded(true); - // We must always reset the handshake before a modded connection is established. + // We must always reset the handshake before a modded connection is established if + // we haven't done so already. connection.getPlayer().sendLegacyForgeHandshakeResetPacket(); } @@ -113,13 +113,14 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { private boolean canForwardPluginMessage(PluginMessage message) { ClientPlaySessionHandler playerHandler = (ClientPlaySessionHandler) connection.getPlayer().getConnection().getSessionHandler(); - boolean isMCMessage; + boolean isMCOrFMLMessage; if (connection.getMinecraftConnection().getProtocolVersion() <= ProtocolConstants.MINECRAFT_1_12_2) { - isMCMessage = message.getChannel().startsWith("MC|"); + String channel = message.getChannel(); + isMCOrFMLMessage = channel.startsWith("MC|") || channel.startsWith(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL); } else { - isMCMessage = message.getChannel().startsWith("minecraft:"); + isMCOrFMLMessage = message.getChannel().startsWith("minecraft:"); } - return isMCMessage || playerHandler.getClientPluginMsgChannels().contains(message.getChannel()) || + return isMCOrFMLMessage || playerHandler.getClientPluginMsgChannels().contains(message.getChannel()) || server.getChannelRegistrar().registered(message.getChannel()); } } 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 3a65e591..3a93f033 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 @@ -197,6 +197,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // Flush everything player.getConnection().flush(); player.getConnectedServer().getMinecraftConnection().flush(); + player.getConnectedServer().setHasCompletedJoin(true); + player.getConnection().setCanSendLegacyFMLResetPacket(true); } public List getServerBossBars() { 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 b3973aae..605a376d 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 @@ -296,10 +296,13 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { } public void sendLegacyForgeHandshakeResetPacket() { - PluginMessage resetPacket = new PluginMessage(); - resetPacket.setChannel(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL); - resetPacket.setData(VelocityConstants.FORGE_LEGACY_HANDSHAKE_RESET_DATA); - connection.write(resetPacket); + if (connection.canSendLegacyFMLResetPacket()) { + PluginMessage resetPacket = new PluginMessage(); + resetPacket.setChannel(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL); + resetPacket.setData(VelocityConstants.FORGE_LEGACY_HANDSHAKE_RESET_DATA); + connection.write(resetPacket); + connection.setCanSendLegacyFMLResetPacket(false); + } } public void close(TextComponent reason) { From e86968e8997dd52fdda585cb6bf07deb51582b41 Mon Sep 17 00:00:00 2001 From: Daniel Naylor Date: Thu, 6 Sep 2018 19:38:50 +0100 Subject: [PATCH 14/49] Don't forward most packets while join is still in progress --- .../proxy/connection/backend/BackendPlaySessionHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 77ea3faf..8f231105 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 @@ -88,7 +88,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { if (status == MessageHandler.ForwardStatus.FORWARD) { connection.getPlayer().getConnection().write(pm); } - } else { + } else if (connection.hasCompletedJoin()) { // Just forward the packet on. We don't have anything to handle at this time. connection.getPlayer().getConnection().write(packet); } From 51a85d372b004b0d167f6e25bfe9e7862b1c3fb3 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 6 Sep 2018 16:09:13 -0400 Subject: [PATCH 15/49] Add hasCompletedJoin() check as suggested --- .../proxy/connection/backend/BackendPlaySessionHandler.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 8f231105..968feccf 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 @@ -102,7 +102,10 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { connection.getMinecraftConnection().close(); return; } - connection.getPlayer().getConnection().write(buf.retain()); + + if (connection.hasCompletedJoin()) { + connection.getPlayer().getConnection().write(buf.retain()); + } } @Override From 30baee84bd23ae5af7a14807cc37d1c48e56d0dc Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 6 Sep 2018 16:11:44 -0400 Subject: [PATCH 16/49] Add the same check to ClientPlaySessionHandler --- .../proxy/connection/client/ClientPlaySessionHandler.java | 8 ++++++-- 1 file changed, 6 insertions(+), 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 3a93f033..3fd25e84 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 @@ -122,12 +122,16 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } // If we don't want to handle this packet, just forward it on. - player.getConnectedServer().getMinecraftConnection().write(packet); + if (player.getConnectedServer().hasCompletedJoin()) { + player.getConnectedServer().getMinecraftConnection().write(packet); + } } @Override public void handleUnknown(ByteBuf buf) { - player.getConnectedServer().getMinecraftConnection().write(buf.retain()); + if (player.getConnectedServer().hasCompletedJoin()) { + player.getConnectedServer().getMinecraftConnection().write(buf.retain()); + } } @Override From f8a3f24d6b18ac51f17d17f2f74a757464e52068 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 7 Sep 2018 16:35:36 -0400 Subject: [PATCH 17/49] Make sure to always forward on keep-alives from the client --- .../proxy/connection/client/ClientPlaySessionHandler.java | 2 ++ 1 file changed, 2 insertions(+) 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 3fd25e84..b6831123 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 @@ -61,6 +61,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } player.setPing(System.currentTimeMillis() - lastPingSent); resetPingData(); + player.getConnectedServer().getMinecraftConnection().write(packet); + return; } if (packet instanceof ClientSettings) { From be9547612fd5240257b4a93c9d47f3e0f0281349 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 7 Sep 2018 17:54:59 -0400 Subject: [PATCH 18/49] Misc code cleanup --- .../connection/backend/BackendPlaySessionHandler.java | 4 ++-- .../proxy/connection/backend/LoginSessionHandler.java | 4 ++-- .../connection/backend/VelocityServerConnection.java | 10 +++++----- .../connection/client/ClientPlaySessionHandler.java | 2 +- .../proxy/connection/client/ClientSettingsWrapper.java | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) 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 968feccf..1a011a2e 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 @@ -70,8 +70,8 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { } if (!connection.hasCompletedJoin() && pm.getChannel().equals(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) { - if (!connection.isModded()) { - connection.setModded(true); + if (!connection.isLegacyForge()) { + connection.setLegacyForge(true); // We must always reset the handshake before a modded connection is established if // we haven't done so already. diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java index 4d011110..1a7f5e2d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java @@ -85,7 +85,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { } else { // The previous server connection should become obsolete. // Before we remove it, if the server we are departing is modded, we must always reset the client state. - if (existingConnection.isModded()) { + if (existingConnection.isLegacyForge()) { connection.getPlayer().sendLegacyForgeHandshakeResetPacket(); } existingConnection.disconnect(); @@ -123,7 +123,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { } } - static ByteBuf createForwardingData(byte[] hmacSecret, String address, GameProfile profile) { + private static ByteBuf createForwardingData(byte[] hmacSecret, String address, GameProfile profile) { ByteBuf dataToForward = Unpooled.buffer(); ByteBuf finalData = Unpooled.buffer(); try { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index 6d37a8b6..fe472afb 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -43,7 +43,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, private final ConnectedPlayer proxyPlayer; private final VelocityServer server; private MinecraftConnection minecraftConnection; - private boolean isModded = false; + private boolean legacyForge = false; private boolean hasCompletedJoin = false; public VelocityServerConnection(ServerInfo target, ConnectedPlayer proxyPlayer, VelocityServer server) { @@ -159,12 +159,12 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, minecraftConnection.write(message); } - public boolean isModded() { - return isModded; + public boolean isLegacyForge() { + return legacyForge; } - public void setModded(boolean modded) { - isModded = modded; + public void setLegacyForge(boolean modded) { + legacyForge = modded; } public boolean hasCompletedJoin() { 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 b6831123..34305448 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 @@ -243,7 +243,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { return; } - if (player.getConnectedServer().isModded() && !player.getConnectedServer().hasCompletedJoin()) { + if (player.getConnectedServer().isLegacyForge() && !player.getConnectedServer().hasCompletedJoin()) { // Ensure that the messages are forwarded player.getConnectedServer().getMinecraftConnection().write(packet); return; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientSettingsWrapper.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientSettingsWrapper.java index e82fd354..4977c5df 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientSettingsWrapper.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientSettingsWrapper.java @@ -12,7 +12,7 @@ public class ClientSettingsWrapper implements PlayerSettings { private final SkinParts parts; private Locale locale = null; - public ClientSettingsWrapper(ClientSettings settings) { + ClientSettingsWrapper(ClientSettings settings) { this.settings = settings; this.parts = new SkinParts((byte) settings.getSkinParts()); } From 3eca6e9df1b2434a169af2f8bb029979654c282c Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 7 Sep 2018 18:09:28 -0400 Subject: [PATCH 19/49] Mark Velocity as a modded server on the server list --- .../api/proxy/server/ServerPing.java | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java index 525cd2bc..88fe4400 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java @@ -4,7 +4,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.velocitypowered.api.util.Favicon; import net.kyori.text.Component; -import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import java.util.*; @@ -17,12 +16,18 @@ public class ServerPing { private final Players players; private final Component description; private final @Nullable Favicon favicon; + private final Modinfo modinfo; public ServerPing(Version version, @Nullable Players players, Component description, @Nullable Favicon favicon) { + this(version, players, description, favicon, Modinfo.DEFAULT); + } + + public ServerPing(Version version, @Nullable Players players, Component description, @Nullable Favicon favicon, Modinfo modinfo) { this.version = Preconditions.checkNotNull(version, "version"); this.players = players; this.description = Preconditions.checkNotNull(description, "description"); this.favicon = favicon; + this.modinfo = modinfo; } public Version getVersion() { @@ -74,6 +79,7 @@ public class ServerPing { private int onlinePlayers; private int maximumPlayers; private final List samplePlayers = new ArrayList<>(); + private final List mods = new ArrayList<>(); private Component description; private Favicon favicon; private boolean nullOutPlayers; @@ -102,6 +108,16 @@ public class ServerPing { return this; } + public Builder mods(Mod... mods) { + this.mods.addAll(Arrays.asList(mods)); + return this; + } + + public Builder clearMods() { + this.mods.clear(); + return this; + } + public Builder clearSamplePlayers() { this.samplePlayers.clear(); return this; @@ -123,7 +139,8 @@ public class ServerPing { } public ServerPing build() { - return new ServerPing(version, nullOutPlayers ? null : new Players(onlinePlayers, maximumPlayers, samplePlayers), description, favicon); + return new ServerPing(version, nullOutPlayers ? null : new Players(onlinePlayers, maximumPlayers, samplePlayers), description, favicon, + new Modinfo(mods)); } public Version getVersion() { @@ -150,6 +167,10 @@ public class ServerPing { return favicon; } + public List getMods() { + return mods; + } + @Override public String toString() { return "Builder{" + @@ -157,8 +178,10 @@ public class ServerPing { ", onlinePlayers=" + onlinePlayers + ", maximumPlayers=" + maximumPlayers + ", samplePlayers=" + samplePlayers + + ", mods=" + mods + ", description=" + description + ", favicon=" + favicon + + ", nullOutPlayers=" + nullOutPlayers + '}'; } } @@ -247,4 +270,25 @@ public class ServerPing { '}'; } } + + public static class Modinfo { + public static final Modinfo DEFAULT = new Modinfo(ImmutableList.of()); + + private final String type = "FML"; + private final List modList; + + public Modinfo(List modList) { + this.modList = ImmutableList.copyOf(modList); + } + } + + public static class Mod { + private final String id; + private final String version; + + public Mod(String id, String version) { + this.id = Preconditions.checkNotNull(id, "id"); + this.version = Preconditions.checkNotNull(version, "version"); + } + } } From a62238d073f9e41ce0523d16d458adb1b6ab732f Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 7 Sep 2018 18:18:38 -0400 Subject: [PATCH 20/49] Allow toggling announcing Forge support on and off --- .../api/proxy/server/ServerPing.java | 26 +++++-- .../proxy/config/VelocityConfiguration.java | 73 +++++++------------ .../client/HandshakeSessionHandler.java | 1 + .../client/StatusSessionHandler.java | 3 +- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java index 88fe4400..ec5d35a5 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java @@ -22,7 +22,7 @@ public class ServerPing { this(version, players, description, favicon, Modinfo.DEFAULT); } - public ServerPing(Version version, @Nullable Players players, Component description, @Nullable Favicon favicon, Modinfo modinfo) { + public ServerPing(Version version, @Nullable Players players, Component description, @Nullable Favicon favicon, @Nullable Modinfo modinfo) { this.version = Preconditions.checkNotNull(version, "version"); this.players = players; this.description = Preconditions.checkNotNull(description, "description"); @@ -46,6 +46,10 @@ public class ServerPing { return Optional.ofNullable(favicon); } + public Optional getModinfo() { + return Optional.ofNullable(modinfo); + } + @Override public String toString() { return "ServerPing{" + @@ -59,11 +63,16 @@ public class ServerPing { public Builder asBuilder() { Builder builder = new Builder(); builder.version = version; - builder.onlinePlayers = players.online; - builder.maximumPlayers = players.max; - builder.samplePlayers.addAll(players.sample); + if (players != null) { + builder.onlinePlayers = players.online; + builder.maximumPlayers = players.max; + builder.samplePlayers.addAll(players.sample); + } else { + builder.nullOutPlayers = true; + } builder.description = description; builder.favicon = favicon; + builder.nullOutModinfo = modinfo == null; return builder; } @@ -83,6 +92,7 @@ public class ServerPing { private Component description; private Favicon favicon; private boolean nullOutPlayers; + private boolean nullOutModinfo; private Builder() { @@ -123,6 +133,11 @@ public class ServerPing { return this; } + public Builder notModCompatible() { + this.nullOutModinfo = true; + return this; + } + public Builder nullPlayers() { this.nullOutPlayers = true; return this; @@ -140,7 +155,7 @@ public class ServerPing { public ServerPing build() { return new ServerPing(version, nullOutPlayers ? null : new Players(onlinePlayers, maximumPlayers, samplePlayers), description, favicon, - new Modinfo(mods)); + nullOutModinfo ? null : new Modinfo(mods)); } public Version getVersion() { @@ -182,6 +197,7 @@ public class ServerPing { ", description=" + description + ", favicon=" + favicon + ", nullOutPlayers=" + nullOutPlayers + + ", nullOutModinfo=" + nullOutModinfo + '}'; } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index 2a7e214a..470e064f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -1,5 +1,6 @@ package com.velocitypowered.proxy.config; +import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableMap; import com.moandjiezana.toml.Toml; import com.velocitypowered.api.util.Favicon; @@ -48,7 +49,7 @@ public class VelocityConfiguration extends AnnotatedConfig { @Comment({ "Should we forward IP addresses and other data to backend servers?", "Available options:", - "- \"none\": No forwarding will be done. All players will appear to be Should we forward IP addresses and other data to backend servers?connecting from the proxy", + "- \"none\": No forwarding will be done. All players will appear to be connecting from the proxy", " and will have offline-mode UUIDs.", "- \"legacy\": Forward player IPs and UUIDs in BungeeCord-compatible fashion. Use this if you run", " servers using Minecraft 1.12 or lower.", @@ -62,6 +63,10 @@ public class VelocityConfiguration extends AnnotatedConfig { @ConfigKey("forwarding-secret") private byte[] forwardingSecret = generateRandomString(12).getBytes(StandardCharsets.UTF_8); + @Comment("Announce whether or not your server supports Forge/FML. If you run a modded server, we suggest turning this on.") + @ConfigKey("announce-forge") + private boolean announceForge = false; + @Table("[servers]") private final Servers servers; @@ -83,12 +88,13 @@ public class VelocityConfiguration extends AnnotatedConfig { } private VelocityConfiguration(String bind, String motd, int showMaxPlayers, boolean onlineMode, - PlayerInfoForwarding playerInfoForwardingMode, byte[] forwardingSecret, Servers servers, - Advanced advanced, Query query) { + boolean announceForge, PlayerInfoForwarding playerInfoForwardingMode, byte[] forwardingSecret, + Servers servers, Advanced advanced, Query query) { this.bind = bind; this.motd = motd; this.showMaxPlayers = showMaxPlayers; this.onlineMode = onlineMode; + this.announceForge = announceForge; this.playerInfoForwardingMode = playerInfoForwardingMode; this.forwardingSecret = forwardingSecret; this.servers = servers; @@ -263,54 +269,26 @@ public class VelocityConfiguration extends AnnotatedConfig { return favicon; } - private void setBind(String bind) { - this.bind = bind; - } - - private void setMotd(String motd) { - this.motd = motd; - } - - private void setShowMaxPlayers(int showMaxPlayers) { - this.showMaxPlayers = showMaxPlayers; - } - - private void setOnlineMode(boolean onlineMode) { - this.onlineMode = onlineMode; - } - - private void setPlayerInfoForwardingMode(PlayerInfoForwarding playerInfoForwardingMode) { - this.playerInfoForwardingMode = playerInfoForwardingMode; - } - - private void setForwardingSecret(byte[] forwardingSecret) { - this.forwardingSecret = forwardingSecret; - } - - private void setMotdAsComponent(Component motdAsComponent) { - this.motdAsComponent = motdAsComponent; - } - - private void setFavicon(Favicon favicon) { - this.favicon = favicon; + public boolean isAnnounceForge() { + return announceForge; } @Override public String toString() { - - return "VelocityConfiguration{" - + "bind='" + bind + '\'' - + ", motd='" + motd + '\'' - + ", showMaxPlayers=" + showMaxPlayers - + ", onlineMode=" + onlineMode - + ", playerInfoForwardingMode=" + playerInfoForwardingMode - + ", forwardingSecret=" + ByteBufUtil.hexDump(forwardingSecret) - + ", servers=" + servers - + ", advanced=" + advanced - + ", query=" + query - + ", motdAsComponent=" + motdAsComponent - + ", favicon=" + favicon - + '}'; + return MoreObjects.toStringHelper(this) + .add("configVersion", configVersion) + .add("bind", bind) + .add("motd", motd) + .add("showMaxPlayers", showMaxPlayers) + .add("onlineMode", onlineMode) + .add("playerInfoForwardingMode", playerInfoForwardingMode) + .add("forwardingSecret", forwardingSecret) + .add("announceForge", announceForge) + .add("servers", servers) + .add("advanced", advanced) + .add("query", query) + .add("favicon", favicon) + .toString(); } public static VelocityConfiguration read(Path path) throws IOException { @@ -335,6 +313,7 @@ public class VelocityConfiguration extends AnnotatedConfig { toml.getString("motd", "&3A Velocity Server"), toml.getLong("show-max-players", 500L).intValue(), toml.getBoolean("online-mode", true), + toml.getBoolean("announce-forge", false), PlayerInfoForwarding.valueOf(toml.getString("player-info-forwarding-mode", "MODERN").toUpperCase()), forwardingSecret, servers, diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java index b6b2dafe..54899d2a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java @@ -108,6 +108,7 @@ public class HandshakeSessionHandler implements MinecraftSessionHandler { new ServerPing.Version(ProtocolConstants.MAXIMUM_GENERIC_VERSION, "Velocity " + ProtocolConstants.SUPPORTED_GENERIC_VERSION_STRING), new ServerPing.Players(server.getPlayerCount(), configuration.getShowMaxPlayers(), ImmutableList.of()), configuration.getMotdComponent(), + null, null ); ProxyPingEvent event = new ProxyPingEvent(new LegacyInboundConnection(connection), ping); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java index 863c32e5..ccc53a8a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java @@ -48,7 +48,8 @@ public class StatusSessionHandler implements MinecraftSessionHandler { new ServerPing.Version(shownVersion, "Velocity " + ProtocolConstants.SUPPORTED_GENERIC_VERSION_STRING), new ServerPing.Players(server.getPlayerCount(), configuration.getShowMaxPlayers(), ImmutableList.of()), configuration.getMotdComponent(), - configuration.getFavicon() + configuration.getFavicon(), + configuration.isAnnounceForge() ? ServerPing.Modinfo.DEFAULT : null ); ProxyPingEvent event = new ProxyPingEvent(inboundWrapper, initialPing); From 5dbf8f1736c440975a361a48e785dd34be909be0 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 7 Sep 2018 23:23:53 -0400 Subject: [PATCH 21/49] Include modlist in toBuilder() --- .../java/com/velocitypowered/api/proxy/server/ServerPing.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java index ec5d35a5..252ad77c 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java @@ -73,6 +73,9 @@ public class ServerPing { builder.description = description; builder.favicon = favicon; builder.nullOutModinfo = modinfo == null; + if (modinfo != null) { + builder.mods.addAll(modinfo.modList); + } return builder; } From f75a51638cf35ac66c9f657a9ab5db4b6fd9ca7f Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 7 Sep 2018 23:29:49 -0400 Subject: [PATCH 22/49] Enable TCP_NODELAY on Minecraft proxy connections too --- .../proxy/connection/backend/VelocityServerConnection.java | 1 + 1 file changed, 1 insertion(+) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index fe472afb..ceb53b9e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -55,6 +55,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, public CompletableFuture connect() { CompletableFuture result = new CompletableFuture<>(); server.initializeGenericBootstrap() + .option(ChannelOption.TCP_NODELAY, true) .handler(new ChannelInitializer() { @Override protected void initChannel(Channel ch) throws Exception { From 8d045c9140965ea9b0a54c40c0db6e0e10099e82 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 8 Sep 2018 00:32:53 -0400 Subject: [PATCH 23/49] Bump versions of Gradle, log4j, and Netty. --- build.gradle | 4 ++-- gradle/wrapper/gradle-wrapper.properties | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index ecccaa6b..437c41ed 100644 --- a/build.gradle +++ b/build.gradle @@ -13,8 +13,8 @@ allprojects { // dependency versions junitVersion = '5.3.0-M1' slf4jVersion = '1.7.25' - log4jVersion = '2.11.0' - nettyVersion = '4.1.28.Final' + log4jVersion = '2.11.1' + nettyVersion = '4.1.29.Final' guavaVersion = '25.1-jre' getCurrentBranchName = { diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 53fd890d..c77aa2dc 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-all.zip From 0b1f95147a9a07c4d30865ebfa0d6651d0f8d381 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 8 Sep 2018 01:00:21 -0400 Subject: [PATCH 24/49] Implment simple backpressure support In most cases this should only trigger on initial spawns and server switches. --- .../proxy/connection/MinecraftConnection.java | 7 +++++++ .../proxy/connection/MinecraftSessionHandler.java | 4 ++++ .../connection/client/ClientPlaySessionHandler.java | 10 ++++++++++ .../proxy/network/ConnectionManager.java | 9 ++++----- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java index 94293ebc..b4a68dfb 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -107,6 +107,13 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { } } + @Override + public void channelWritabilityChanged(ChannelHandlerContext ctx) throws Exception { + if (sessionHandler != null) { + sessionHandler.writabilityChanged(); + } + } + public void write(Object msg) { if (channel.isActive()) { channel.writeAndFlush(msg, channel.voidPromise()); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftSessionHandler.java index 4f13190d..146ec86d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftSessionHandler.java @@ -29,4 +29,8 @@ public interface MinecraftSessionHandler { default void exception(Throwable throwable) { } + + default void writabilityChanged() { + + } } 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 34305448..0692fc00 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 @@ -4,6 +4,7 @@ import com.velocitypowered.api.event.connection.DisconnectEvent; import com.velocitypowered.api.proxy.messages.ChannelSide; import com.velocitypowered.api.proxy.messages.MessageHandler; import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolConstants; import com.velocitypowered.proxy.protocol.packet.*; @@ -151,6 +152,15 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { .build()); } + @Override + public void writabilityChanged() { + VelocityServerConnection server = player.getConnectedServer(); + if (server != null) { + boolean writable = player.getConnection().getChannel().isWritable(); + server.getMinecraftConnection().getChannel().config().setAutoRead(writable); + } + } + public void handleBackendJoinGame(JoinGame joinGame) { resetPingData(); // reset ping data; if (!spawned) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java index fb08b63a..cc37d62b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java @@ -16,11 +16,7 @@ import com.velocitypowered.proxy.protocol.netty.MinecraftVarintFrameDecoder; import com.velocitypowered.proxy.protocol.netty.MinecraftVarintLengthEncoder; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFutureListener; -import io.netty.channel.ChannelInitializer; -import io.netty.channel.ChannelOption; -import io.netty.channel.EventLoopGroup; +import io.netty.channel.*; import io.netty.channel.epoll.Epoll; import io.netty.channel.epoll.EpollDatagramChannel; import io.netty.channel.epoll.EpollEventLoopGroup; @@ -48,6 +44,8 @@ import java.util.concurrent.TimeUnit; import static com.velocitypowered.proxy.network.Connections.*; public final class ConnectionManager { + private static final WriteBufferWaterMark SERVER_WRITE_MARK = new WriteBufferWaterMark(1 << 16, 1 << 18); + private static final Logger logger = LogManager.getLogger(ConnectionManager.class); private final Set endpoints = new HashSet<>(); @@ -72,6 +70,7 @@ public final class ConnectionManager { final ServerBootstrap bootstrap = new ServerBootstrap() .channel(this.transportType.serverSocketChannelClass) .group(this.bossGroup, this.workerGroup) + .childOption(ChannelOption.WRITE_BUFFER_WATER_MARK, SERVER_WRITE_MARK) .childHandler(new ChannelInitializer() { @Override protected void initChannel(final Channel ch) { From 54c3c37064d0ae1abfec2cc3dc85dbdc15ea181e Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 8 Sep 2018 02:14:52 -0400 Subject: [PATCH 25/49] Update note about Forge support [ci skip] --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index a5865623..493fdb34 100644 --- a/README.md +++ b/README.md @@ -44,5 +44,4 @@ is not currently suitable for production usage. For development and testing purposes, however, Velocity is fully-fledged and ready to go. Velocity supports Minecraft 1.8-1.13.1, and has full support for Paper and Sponge. -Forge support is currently not implemented, but Velocity will work with Forge's -vanilla fallback mode. \ No newline at end of file +Forge is fully supported but mod compatibility may vary. \ No newline at end of file From f2e3b5c7ec30e047dcd450bf9154c8e3553c518a Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 9 Sep 2018 14:20:46 -0400 Subject: [PATCH 26/49] Increase plugin channels limit --- .../proxy/connection/client/ClientPlaySessionHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0692fc00..8bc86848 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 @@ -25,7 +25,7 @@ import java.util.*; */ public class ClientPlaySessionHandler implements MinecraftSessionHandler { private static final Logger logger = LogManager.getLogger(ClientPlaySessionHandler.class); - private static final int MAX_PLUGIN_CHANNELS = 128; + private static final int MAX_PLUGIN_CHANNELS = 1024; private final ConnectedPlayer player; private long lastPingID = -1; From 46aa8efb354c2e4190b50ccc4d8e329212c65f7b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 9 Sep 2018 14:37:43 -0400 Subject: [PATCH 27/49] Improve reliability of varint decoder. --- .../netty/MinecraftVarintFrameDecoder.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java index 08125359..ec2f9ded 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java @@ -2,8 +2,10 @@ package com.velocitypowered.proxy.protocol.netty; import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.ByteToMessageDecoder; +import io.netty.handler.codec.CorruptedFrameException; import java.util.List; @@ -15,12 +17,26 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder { } in.markReaderIndex(); - int packetLength = ProtocolUtils.readVarInt(in); - if (in.readableBytes() < packetLength) { - in.resetReaderIndex(); - return; + + byte[] lenBuf = new byte[3]; + for (int i = 0; i < lenBuf.length; i++) { + lenBuf[i] = in.readByte(); + if (lenBuf[i] > 0) { + int packetLength = ProtocolUtils.readVarInt(Unpooled.wrappedBuffer(lenBuf)); + if (packetLength == 0) { + return; + } + + if (in.readableBytes() < packetLength) { + in.resetReaderIndex(); + return; + } + + out.add(in.readRetainedSlice(packetLength)); + return; + } } - out.add(in.readRetainedSlice(packetLength)); + throw new CorruptedFrameException("VarInt too big"); } } From 56a50c60b560f220dfdae33f35813af68bae7be4 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 9 Sep 2018 15:05:27 -0400 Subject: [PATCH 28/49] Add missing readable check --- .../proxy/protocol/netty/MinecraftVarintFrameDecoder.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java index ec2f9ded..136204fa 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java @@ -20,6 +20,11 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder { byte[] lenBuf = new byte[3]; for (int i = 0; i < lenBuf.length; i++) { + if (!in.isReadable()) { + in.resetReaderIndex(); + return; + } + lenBuf[i] = in.readByte(); if (lenBuf[i] > 0) { int packetLength = ProtocolUtils.readVarInt(Unpooled.wrappedBuffer(lenBuf)); From df637cd59871dee74decd3d851e99a525ab6ed88 Mon Sep 17 00:00:00 2001 From: Daniel Naylor Date: Mon, 10 Sep 2018 18:12:01 +0100 Subject: [PATCH 29/49] Ensure the reset packet is not sent when Forge isn't expecting it. Fixes #69 --- .../connection/backend/LoginSessionHandler.java | 6 ++++++ .../client/ClientPlaySessionHandler.java | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java index 1a7f5e2d..c65f1745 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java @@ -82,6 +82,12 @@ public class LoginSessionHandler implements MinecraftSessionHandler { if (existingConnection == null) { // Strap on the play session handler connection.getPlayer().getConnection().setSessionHandler(new ClientPlaySessionHandler(server, connection.getPlayer())); + + // This is for legacy Forge servers - during first connection the FML handshake will transition to complete regardless + // Thus, we need to ensure that a reset packet is ALWAYS sent on first switch. + // + // The call will handle if the player is not a Forge player appropriately. + connection.getPlayer().getConnection().setCanSendLegacyFMLResetPacket(true); } else { // The previous server connection should become obsolete. // Before we remove it, if the server we are departing is modded, we must always reset the client state. 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 8bc86848..b7cb0f91 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 @@ -214,7 +214,20 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { player.getConnection().flush(); player.getConnectedServer().getMinecraftConnection().flush(); player.getConnectedServer().setHasCompletedJoin(true); - player.getConnection().setCanSendLegacyFMLResetPacket(true); + if (player.getConnectedServer().isLegacyForge()) { + // We only need to indicate we can send a reset packet if we complete a handshake, that is, + // logged onto a Forge server. + // + // The special case is if we log onto a Vanilla server as our first server, FML will treat this + // as complete and **will** need a reset packet sending at some point. We will handle this + // during initial player connection if the player is detected to be forge. + // + // This is why we use an if statement rather than the result of VelocityServerConnection#isLegacyForge() + // because we don't want to set it false if this is a first connection to a Vanilla server. + // + // See LoginSessionHandler#handle for where the counterpart to this method is + player.getConnection().setCanSendLegacyFMLResetPacket(true); + } } public List getServerBossBars() { From 74bf246c3958e640a183aea653d839d636ae2554 Mon Sep 17 00:00:00 2001 From: Leymooo Date: Tue, 11 Sep 2018 16:15:54 +0300 Subject: [PATCH 30/49] Add PostLoginEvent. Resolve #72 --- .../api/event/connection/PostLoginEvent.java | 29 +++++++++++ .../client/LoginSessionHandler.java | 48 ++++++++++--------- 2 files changed, 55 insertions(+), 22 deletions(-) create mode 100644 api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java new file mode 100644 index 00000000..8cd3dea5 --- /dev/null +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java @@ -0,0 +1,29 @@ +package com.velocitypowered.api.event.connection; + +import com.google.common.base.Preconditions; +import com.velocitypowered.api.proxy.Player; +import org.checkerframework.checker.nullness.qual.NonNull; + +/** + * This event is fired once the player has been successfully authenticated and + * fully initialized and player will be connected to server after this event + */ +public class PostLoginEvent { + + private final Player player; + + public PostLoginEvent(@NonNull Player player) { + this.player = Preconditions.checkNotNull(player, "player"); + } + + public Player getPlayer() { + return player; + } + + @Override + public String toString() { + return "PostLoginEvent{" + + "player=" + player + + '}'; + } +} 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 ff728178..0ae515ff 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 @@ -2,6 +2,7 @@ package com.velocitypowered.proxy.connection.client; import com.google.common.base.Preconditions; import com.velocitypowered.api.event.connection.LoginEvent; +import com.velocitypowered.api.event.connection.PostLoginEvent; import com.velocitypowered.api.event.connection.PreLoginEvent; import com.velocitypowered.api.event.connection.PreLoginEvent.PreLoginComponentResult; import com.velocitypowered.api.event.permission.PermissionsSetupEvent; @@ -39,9 +40,10 @@ import java.util.Optional; import java.util.concurrent.ThreadLocalRandom; public class LoginSessionHandler implements MinecraftSessionHandler { + private static final Logger logger = LogManager.getLogger(LoginSessionHandler.class); - private static final String MOJANG_SERVER_AUTH_URL = - "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s&ip=%s"; + private static final String MOJANG_SERVER_AUTH_URL + = "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s&ip=%s"; private final VelocityServer server; private final MinecraftConnection inbound; @@ -193,26 +195,26 @@ public class LoginSessionHandler implements MinecraftSessionHandler { apiInbound.getVirtualHost().orElse(null)); return server.getEventManager().fire(new PermissionsSetupEvent(player, ConnectedPlayer.DEFAULT_PERMISSIONS)) - .thenCompose(event -> { - // wait for permissions to load, then set the players permission function - player.setPermissionFunction(event.createFunction(player)); - // then call & wait for the login event - return server.getEventManager().fire(new LoginEvent(player)); - }) - // then complete the connection - .thenAcceptAsync(event -> { - if (inbound.isClosed()) { - // The player was disconnected - return; - } - if (!event.getResult().isAllowed()) { - // The component is guaranteed to be provided if the connection was denied. - inbound.closeWith(Disconnect.create(event.getResult().getReason().get())); - return; - } + .thenCompose(event -> { + // wait for permissions to load, then set the players permission function + player.setPermissionFunction(event.createFunction(player)); + // then call & wait for the login event + return server.getEventManager().fire(new LoginEvent(player)); + }) + // then complete the connection + .thenAcceptAsync(event -> { + if (inbound.isClosed()) { + // The player was disconnected + return; + } + if (!event.getResult().isAllowed()) { + // The component is guaranteed to be provided if the connection was denied. + inbound.closeWith(Disconnect.create(event.getResult().getReason().get())); + return; + } - handleProxyLogin(player); - }, inbound.getChannel().eventLoop()); + handleProxyLogin(player); + }, inbound.getChannel().eventLoop()); }); } @@ -244,7 +246,9 @@ public class LoginSessionHandler implements MinecraftSessionHandler { logger.info("{} has connected", player); inbound.setSessionHandler(new InitialConnectSessionHandler(player)); - player.createConnectionRequest(toTry.get()).fireAndForget(); + server.getEventManager().fire(new PostLoginEvent(player)).thenRun(() -> { + player.createConnectionRequest(toTry.get()).fireAndForget(); + }); } @Override From 79bb43468f97399663902bb6eb64d9e380ee2547 Mon Sep 17 00:00:00 2001 From: Leymooo Date: Tue, 11 Sep 2018 17:42:24 +0300 Subject: [PATCH 31/49] remove @NonNull, revert reindentation --- .../api/event/connection/PostLoginEvent.java | 3 +- .../client/LoginSessionHandler.java | 42 +++++++++---------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java index 8cd3dea5..7ecc23bb 100644 --- a/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PostLoginEvent.java @@ -2,7 +2,6 @@ package com.velocitypowered.api.event.connection; import com.google.common.base.Preconditions; import com.velocitypowered.api.proxy.Player; -import org.checkerframework.checker.nullness.qual.NonNull; /** * This event is fired once the player has been successfully authenticated and @@ -12,7 +11,7 @@ public class PostLoginEvent { private final Player player; - public PostLoginEvent(@NonNull Player player) { + public PostLoginEvent(Player player) { this.player = Preconditions.checkNotNull(player, "player"); } 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 0ae515ff..fed5cb58 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 @@ -42,8 +42,8 @@ import java.util.concurrent.ThreadLocalRandom; public class LoginSessionHandler implements MinecraftSessionHandler { private static final Logger logger = LogManager.getLogger(LoginSessionHandler.class); - private static final String MOJANG_SERVER_AUTH_URL - = "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s&ip=%s"; + private static final String MOJANG_SERVER_AUTH_URL = + "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s&ip=%s"; private final VelocityServer server; private final MinecraftConnection inbound; @@ -195,26 +195,26 @@ public class LoginSessionHandler implements MinecraftSessionHandler { apiInbound.getVirtualHost().orElse(null)); return server.getEventManager().fire(new PermissionsSetupEvent(player, ConnectedPlayer.DEFAULT_PERMISSIONS)) - .thenCompose(event -> { - // wait for permissions to load, then set the players permission function - player.setPermissionFunction(event.createFunction(player)); - // then call & wait for the login event - return server.getEventManager().fire(new LoginEvent(player)); - }) - // then complete the connection - .thenAcceptAsync(event -> { - if (inbound.isClosed()) { - // The player was disconnected - return; - } - if (!event.getResult().isAllowed()) { - // The component is guaranteed to be provided if the connection was denied. - inbound.closeWith(Disconnect.create(event.getResult().getReason().get())); - return; - } + .thenCompose(event -> { + // wait for permissions to load, then set the players permission function + player.setPermissionFunction(event.createFunction(player)); + // then call & wait for the login event + return server.getEventManager().fire(new LoginEvent(player)); + }) + // then complete the connection + .thenAcceptAsync(event -> { + if (inbound.isClosed()) { + // The player was disconnected + return; + } + if (!event.getResult().isAllowed()) { + // The component is guaranteed to be provided if the connection was denied. + inbound.closeWith(Disconnect.create(event.getResult().getReason().get())); + return; + } - handleProxyLogin(player); - }, inbound.getChannel().eventLoop()); + handleProxyLogin(player); + }, inbound.getChannel().eventLoop()); }); } From bc86a12c57e8e36952f1927cd63f35b7024e4972 Mon Sep 17 00:00:00 2001 From: Leymooo Date: Wed, 12 Sep 2018 11:43:33 +0300 Subject: [PATCH 32/49] expand PreLoginComponentResult with force offline mode --- .../api/event/connection/PreLoginEvent.java | 25 ++++++++++++++++--- .../client/LoginSessionHandler.java | 2 +- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java index ef8edfb9..9d0620f8 100644 --- a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java @@ -56,31 +56,41 @@ public class PreLoginEvent implements ResultedEvent Date: Wed, 12 Sep 2018 10:21:28 +0100 Subject: [PATCH 33/49] Don't fire a FML reset packet on first login, set it as required for the second join after the first. Fixes #69 --- .../connection/backend/LoginSessionHandler.java | 6 ------ .../connection/client/ClientPlaySessionHandler.java | 12 +++++++++++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java index c65f1745..1a7f5e2d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java @@ -82,12 +82,6 @@ public class LoginSessionHandler implements MinecraftSessionHandler { if (existingConnection == null) { // Strap on the play session handler connection.getPlayer().getConnection().setSessionHandler(new ClientPlaySessionHandler(server, connection.getPlayer())); - - // This is for legacy Forge servers - during first connection the FML handshake will transition to complete regardless - // Thus, we need to ensure that a reset packet is ALWAYS sent on first switch. - // - // The call will handle if the player is not a Forge player appropriately. - connection.getPlayer().getConnection().setCanSendLegacyFMLResetPacket(true); } else { // The previous server connection should become obsolete. // Before we remove it, if the server we are departing is modded, we must always reset the client state. 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 b7cb0f91..d178fe7a 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 @@ -164,9 +164,19 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { public void handleBackendJoinGame(JoinGame joinGame) { resetPingData(); // reset ping data; if (!spawned) { - // nothing special to do here + // Nothing special to do with regards to spawning the player spawned = true; player.getConnection().delayedWrite(joinGame); + + // We have something special to do for legacy Forge servers - during first connection the FML handshake + // will transition to complete regardless. Thus, we need to ensure that a reset packet is ALWAYS sent on + // first switch. + // + // As we know that calling this branch only happens on first join, we set that if we are a Forge + // client that we must reset on the next switch. + // + // The call will handle if the player is not a Forge player appropriately. + player.getConnection().setCanSendLegacyFMLResetPacket(true); } else { // Ah, this is the meat and potatoes of the whole venture! // From 6196f94adf803eab442d9e51129c1236cf55f293 Mon Sep 17 00:00:00 2001 From: Leymooo Date: Wed, 12 Sep 2018 16:28:34 +0300 Subject: [PATCH 34/49] I think we dont need this comment --- .../api/event/connection/PreLoginEvent.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java index 9d0620f8..36d7b98c 100644 --- a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java @@ -52,7 +52,7 @@ public class PreLoginEvent implements ResultedEvent Date: Wed, 12 Sep 2018 22:41:51 -0700 Subject: [PATCH 35/49] ServerPreConnectEvent#getInfo -> getServer --- .../api/event/player/ServerPreConnectEvent.java | 12 ++++++------ .../proxy/connection/client/ConnectedPlayer.java | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java index 2d86c06f..a91f819c 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java @@ -50,11 +50,11 @@ public class ServerPreConnectEvent implements ResultedEvent getInfo() { - return Optional.ofNullable(info); + public Optional getServer() { + return Optional.ofNullable(server); } @Override @@ -71,7 +71,7 @@ public class ServerPreConnectEvent implements ResultedEvent Date: Thu, 13 Sep 2018 10:16:10 +0300 Subject: [PATCH 36/49] Refactor PreLoginComponentResult --- .../api/event/connection/PreLoginEvent.java | 73 ++++++++++++------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java index 36d7b98c..e4b4b146 100644 --- a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java @@ -3,8 +3,10 @@ package com.velocitypowered.api.event.connection; import com.google.common.base.Preconditions; import com.velocitypowered.api.event.ResultedEvent; import com.velocitypowered.api.proxy.InboundConnection; +import java.util.Optional; import net.kyori.text.Component; +import net.kyori.text.serializer.ComponentSerializers; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -54,33 +56,35 @@ public class PreLoginEvent implements ResultedEvent reason; + + private PreLoginComponentResult(Result result, @Nullable Component reason) { + this.result = result; + this.reason = Optional.ofNullable(reason); } - private PreLoginComponentResult(@Nullable Component reason) { - super(reason == null, reason); - // Don't care about this - this.onlineMode = false; - this.forceOfflineMode = false; + @Override + public boolean isAllowed() { + return result != Result.DISALLOWED; + } + + public Optional getReason() { + return reason; } public boolean isOnlineModeAllowed() { - return this.onlineMode; + return result == Result.FORCE_ONLINE; } public boolean isForceOfflineMode() { - return forceOfflineMode; + return result == Result.FORCE_OFFLINE; } @Override @@ -91,12 +95,18 @@ public class PreLoginEvent implements ResultedEvent Date: Thu, 13 Sep 2018 10:33:27 +0100 Subject: [PATCH 37/49] Only send FML/FML|MP plugin messages if the player has joined the server. See #78 --- .../proxy/connection/VelocityConstants.java | 2 ++ .../backend/VelocityServerConnection.java | 6 ++++++ .../connection/client/ClientPlaySessionHandler.java | 13 ++++++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java index b6956336..b524b31e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/VelocityConstants.java @@ -8,6 +8,8 @@ public class VelocityConstants { public static final String VELOCITY_IP_FORWARDING_CHANNEL = "velocity:player_info"; public static final String FORGE_LEGACY_HANDSHAKE_CHANNEL = "FML|HS"; + public static final String FORGE_LEGACY_CHANNEL = "FML"; + public static final String FORGE_MULTIPART_LEGACY_CHANNEL = "FML|MP"; public static final byte[] FORGE_LEGACY_HANDSHAKE_RESET_DATA = new byte[] { -2, 0 }; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index ceb53b9e..59b97be2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -127,6 +127,12 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, minecraftConnection.write(login); } + public void writeIfJoined(PluginMessage message) { + if (hasCompletedJoin) { + minecraftConnection.write(message); + } + } + public MinecraftConnection getMinecraftConnection() { return minecraftConnection; } 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 b7cb0f91..137b97f7 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 @@ -4,6 +4,7 @@ import com.velocitypowered.api.event.connection.DisconnectEvent; import com.velocitypowered.api.proxy.messages.ChannelSide; import com.velocitypowered.api.proxy.messages.MessageHandler; import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolConstants; @@ -275,8 +276,18 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(player, ChannelSide.FROM_CLIENT, packet); if (status == MessageHandler.ForwardStatus.FORWARD) { + String channel = packet.getChannel(); + // We're going to forward on the original packet. - player.getConnectedServer().getMinecraftConnection().write(packet); + // + // If we have Forge messages, we may need to drop them if the server switch has + // not completed yet. + if (channel.equals(VelocityConstants.FORGE_LEGACY_CHANNEL) + || channel.equals(VelocityConstants.FORGE_MULTIPART_LEGACY_CHANNEL)) { + player.getConnectedServer().writeIfJoined(packet); + } else { + player.getConnectedServer().getMinecraftConnection().write(packet); + } } } From 961757b481107285655a30daf114d8827ffa4fcf Mon Sep 17 00:00:00 2001 From: Leymooo Date: Thu, 13 Sep 2018 23:00:12 +0300 Subject: [PATCH 38/49] fix compile --- .../com/velocitypowered/api/event/connection/PreLoginEvent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java index e4b4b146..04fdc762 100644 --- a/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/connection/PreLoginEvent.java @@ -56,7 +56,7 @@ public class PreLoginEvent implements ResultedEvent Date: Thu, 13 Sep 2018 22:41:21 -0600 Subject: [PATCH 39/49] Add CONNECT_TIMEOUT_MILLIS to fix timeout errors. --- .../proxy/connection/backend/VelocityServerConnection.java | 1 + .../com/velocitypowered/proxy/network/ConnectionManager.java | 1 + 2 files changed, 2 insertions(+) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index 59b97be2..7a52248f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -56,6 +56,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, CompletableFuture result = new CompletableFuture<>(); server.initializeGenericBootstrap() .option(ChannelOption.TCP_NODELAY, true) + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, SERVER_READ_TIMEOUT_SECONDS * 1000) .handler(new ChannelInitializer() { @Override protected void initChannel(Channel ch) throws Exception { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java index cc37d62b..ff98ae02 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java @@ -89,6 +89,7 @@ public final class ConnectionManager { ch.pipeline().addLast(Connections.HANDLER, connection); } }) + .childOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, CLIENT_READ_TIMEOUT_SECONDS * 1000) .childOption(ChannelOption.TCP_NODELAY, true) .childOption(ChannelOption.IP_TOS, 0x18) .localAddress(address); From cf46659d9e6ebd2431f8574e50571f9ede2bf176 Mon Sep 17 00:00:00 2001 From: PurpleIsEverything Date: Thu, 13 Sep 2018 22:53:23 -0600 Subject: [PATCH 40/49] Change to a 5 second connection timeout. --- .../proxy/connection/backend/VelocityServerConnection.java | 3 ++- .../com/velocitypowered/proxy/network/ConnectionManager.java | 2 +- .../java/com/velocitypowered/proxy/network/Connections.java | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index 7a52248f..ed6b3b6a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -33,6 +33,7 @@ import static com.velocitypowered.proxy.network.Connections.HANDLER; import static com.velocitypowered.proxy.network.Connections.MINECRAFT_DECODER; import static com.velocitypowered.proxy.network.Connections.MINECRAFT_ENCODER; import static com.velocitypowered.proxy.network.Connections.READ_TIMEOUT; +import static com.velocitypowered.proxy.network.Connections.CONNECTION_TIMEOUT_SECONDS; import static com.velocitypowered.proxy.network.Connections.SERVER_READ_TIMEOUT_SECONDS; public class VelocityServerConnection implements MinecraftConnectionAssociation, ServerConnection { @@ -56,7 +57,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, CompletableFuture result = new CompletableFuture<>(); server.initializeGenericBootstrap() .option(ChannelOption.TCP_NODELAY, true) - .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, SERVER_READ_TIMEOUT_SECONDS * 1000) + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECTION_TIMEOUT_SECONDS * 1000) .handler(new ChannelInitializer() { @Override protected void initChannel(Channel ch) throws Exception { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java index ff98ae02..e1ffe129 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java @@ -89,7 +89,7 @@ public final class ConnectionManager { ch.pipeline().addLast(Connections.HANDLER, connection); } }) - .childOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, CLIENT_READ_TIMEOUT_SECONDS * 1000) + .childOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECTION_TIMEOUT_SECONDS * 1000) .childOption(ChannelOption.TCP_NODELAY, true) .childOption(ChannelOption.IP_TOS, 0x18) .localAddress(address); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java b/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java index 518a3acb..59869eed 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java @@ -16,4 +16,5 @@ public interface Connections { int CLIENT_READ_TIMEOUT_SECONDS = 30; // client -> proxy int SERVER_READ_TIMEOUT_SECONDS = 30; // proxy -> server + int CONNECTION_TIMEOUT_SECONDS = 5; } From 0469aaa03a9c2016e9f8570726089e78924fef30 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 14 Sep 2018 01:00:56 -0400 Subject: [PATCH 41/49] Fix several invalid connection closure issues. --- .../proxy/connection/backend/BackendPlaySessionHandler.java | 4 ++-- .../proxy/connection/backend/VelocityServerConnection.java | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) 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 1a011a2e..0d023692 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 @@ -32,7 +32,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { if (!connection.getPlayer().isActive()) { // Connection was left open accidentally. Close it so as to avoid "You logged in from another location" // errors. - connection.getMinecraftConnection().close(); + connection.disconnect(); return; } @@ -99,7 +99,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { if (!connection.getPlayer().isActive()) { // Connection was left open accidentally. Close it so as to avoid "You logged in from another location" // errors. - connection.getMinecraftConnection().close(); + connection.disconnect(); return; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index ed6b3b6a..0d0a4be0 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -149,8 +149,10 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, } public void disconnect() { - minecraftConnection.close(); - minecraftConnection = null; + if (minecraftConnection != null) { + minecraftConnection.close(); + minecraftConnection = null; + } } @Override From 496c579e469e5ba13e26c98bf8ae8bc7ee2a5113 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 14 Sep 2018 13:56:38 -0400 Subject: [PATCH 42/49] Drop non-FML handshake packets if the game start process is not done. --- .../proxy/connection/client/ClientPlaySessionHandler.java | 7 +++++-- 1 file changed, 5 insertions(+), 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 6bc44a2b..353c2d0c 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 @@ -278,8 +278,11 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } if (player.getConnectedServer().isLegacyForge() && !player.getConnectedServer().hasCompletedJoin()) { - // Ensure that the messages are forwarded - player.getConnectedServer().getMinecraftConnection().write(packet); + // Ensure that the FML handshake is forwarded. Do not try to forward other client-side plugin messages, as + // some mods are poorly coded and will crash if mod packets are sent during the handshake/join game process. + if (packet.getChannel().equals(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) { + player.getConnectedServer().getMinecraftConnection().write(packet); + } return; } From 9776675b7067307a925e3ce401d33933d5b65db5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 14 Sep 2018 14:16:32 -0400 Subject: [PATCH 43/49] Queue mod plugin messages instead. --- .../client/ClientPlaySessionHandler.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 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 353c2d0c..295b4b13 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 @@ -34,6 +34,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { private boolean spawned = false; private final List serverBossBars = new ArrayList<>(); private final Set clientPluginMsgChannels = new HashSet<>(); + private final Queue loginPluginMessages = new ArrayDeque<>(); private final VelocityServer server; public ClientPlaySessionHandler(VelocityServer server, ConnectedPlayer player) { @@ -163,7 +164,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } public void handleBackendJoinGame(JoinGame joinGame) { - resetPingData(); // reset ping data; + resetPingData(); // reset ping data if (!spawned) { // Nothing special to do with regards to spawning the player spawned = true; @@ -221,6 +222,12 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { channel, toRegister)); } + // If we had plugin messages queued during login/FML handshake, send them now. + PluginMessage pm; + while ((pm = loginPluginMessages.poll()) != null) { + player.getConnectedServer().getMinecraftConnection().delayedWrite(pm); + } + // Flush everything player.getConnection().flush(); player.getConnectedServer().getMinecraftConnection().flush(); @@ -278,10 +285,14 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } if (player.getConnectedServer().isLegacyForge() && !player.getConnectedServer().hasCompletedJoin()) { - // Ensure that the FML handshake is forwarded. Do not try to forward other client-side plugin messages, as - // some mods are poorly coded and will crash if mod packets are sent during the handshake/join game process. if (packet.getChannel().equals(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) { + // Always forward the FML handshake to the remote server. player.getConnectedServer().getMinecraftConnection().write(packet); + } else { + // The client is trying to send messages too early. This is primarily caused by mods, but it's further + // aggravated by Velocity. To work around these issues, we will queue any non-FML handshake messages to + // be sent once the JoinGame packet has been received by the proxy. + loginPluginMessages.add(packet); } return; } From e9568e1b6c27764a320bd6d2d708e4a017251503 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 14 Sep 2018 15:26:51 -0400 Subject: [PATCH 44/49] Do not write plugin messages from the server if the player hasn't joined --- .../connection/client/ClientPlaySessionHandler.java | 13 +------------ 1 file changed, 1 insertion(+), 12 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 295b4b13..f550865d 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 @@ -300,18 +300,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(player, ChannelSide.FROM_CLIENT, packet); if (status == MessageHandler.ForwardStatus.FORWARD) { - String channel = packet.getChannel(); - - // We're going to forward on the original packet. - // - // If we have Forge messages, we may need to drop them if the server switch has - // not completed yet. - if (channel.equals(VelocityConstants.FORGE_LEGACY_CHANNEL) - || channel.equals(VelocityConstants.FORGE_MULTIPART_LEGACY_CHANNEL)) { - player.getConnectedServer().writeIfJoined(packet); - } else { - player.getConnectedServer().getMinecraftConnection().write(packet); - } + player.getConnectedServer().writeIfJoined(packet); } } From 2b1d55a0fcfae68661277f0311fe1f2d0990d716 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 15 Sep 2018 01:16:26 -0400 Subject: [PATCH 45/49] Expose original server in ServerPreConnectEvent --- .../api/event/player/ServerPreConnectEvent.java | 11 +++++++++-- .../proxy/connection/client/ConnectedPlayer.java | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java index a91f819c..c0ad8873 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/player/ServerPreConnectEvent.java @@ -14,11 +14,13 @@ import java.util.Optional; */ public class ServerPreConnectEvent implements ResultedEvent { private final Player player; + private final ServerInfo originalServer; private ServerResult result; - public ServerPreConnectEvent(Player player, ServerResult result) { + public ServerPreConnectEvent(Player player, ServerInfo originalServer) { this.player = Preconditions.checkNotNull(player, "player"); - this.result = Preconditions.checkNotNull(result, "result"); + this.originalServer = Preconditions.checkNotNull(originalServer, "originalServer"); + this.result = ServerResult.allowed(originalServer); } public Player getPlayer() { @@ -35,10 +37,15 @@ public class ServerPreConnectEvent implements ResultedEvent> CONNECTION_NOTIFIER = @@ -56,13 +54,11 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, public CompletableFuture connect() { CompletableFuture result = new CompletableFuture<>(); server.initializeGenericBootstrap() - .option(ChannelOption.TCP_NODELAY, true) - .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECTION_TIMEOUT_SECONDS * 1000) .handler(new ChannelInitializer() { @Override protected void initChannel(Channel ch) throws Exception { ch.pipeline() - .addLast(READ_TIMEOUT, new ReadTimeoutHandler(SERVER_READ_TIMEOUT_SECONDS, TimeUnit.SECONDS)) + .addLast(READ_TIMEOUT, new ReadTimeoutHandler(server.getConfiguration().getReadTimeout(), TimeUnit.SECONDS)) .addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder()) .addLast(FRAME_ENCODER, MinecraftVarintLengthEncoder.INSTANCE) .addLast(MINECRAFT_DECODER, new MinecraftDecoder(ProtocolConstants.Direction.CLIENTBOUND)) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java index e1ffe129..9a115a60 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/ConnectionManager.java @@ -75,7 +75,7 @@ public final class ConnectionManager { @Override protected void initChannel(final Channel ch) { ch.pipeline() - .addLast(READ_TIMEOUT, new ReadTimeoutHandler(CLIENT_READ_TIMEOUT_SECONDS, TimeUnit.SECONDS)) + .addLast(READ_TIMEOUT, new ReadTimeoutHandler(server.getConfiguration().getReadTimeout(), TimeUnit.SECONDS)) .addLast(LEGACY_PING_DECODER, new LegacyPingDecoder()) .addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder()) .addLast(LEGACY_PING_ENCODER, LegacyPingEncoder.INSTANCE) @@ -89,7 +89,6 @@ public final class ConnectionManager { ch.pipeline().addLast(Connections.HANDLER, connection); } }) - .childOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECTION_TIMEOUT_SECONDS * 1000) .childOption(ChannelOption.TCP_NODELAY, true) .childOption(ChannelOption.IP_TOS, 0x18) .localAddress(address); @@ -126,7 +125,9 @@ public final class ConnectionManager { public Bootstrap createWorker() { return new Bootstrap() .channel(this.transportType.socketChannelClass) - .group(this.workerGroup); + .group(this.workerGroup) + .option(ChannelOption.TCP_NODELAY, true) + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, server.getConfiguration().getConnectTimeout()); } public void shutdown() { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java b/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java index 59869eed..fb248dc6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/Connections.java @@ -13,8 +13,4 @@ public interface Connections { String MINECRAFT_DECODER = "minecraft-decoder"; String MINECRAFT_ENCODER = "minecraft-encoder"; String READ_TIMEOUT = "read-timeout"; - - int CLIENT_READ_TIMEOUT_SECONDS = 30; // client -> proxy - int SERVER_READ_TIMEOUT_SECONDS = 30; // proxy -> server - int CONNECTION_TIMEOUT_SECONDS = 5; } From 84947564e432b6b3089df810c24abc54152aa39b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 15 Sep 2018 01:46:28 -0400 Subject: [PATCH 47/49] Handle unexpected disconnects without a reason. --- .../backend/BackendPlaySessionHandler.java | 14 ++++++++++++++ .../backend/VelocityServerConnection.java | 6 ++++++ .../proxy/connection/client/ConnectedPlayer.java | 15 ++++++++++----- .../proxy/connection/util/ConnectionMessages.java | 1 + 4 files changed, 31 insertions(+), 5 deletions(-) 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 0d023692..e62eb461 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 @@ -6,6 +6,7 @@ import com.velocitypowered.api.proxy.messages.MessageHandler; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.VelocityConstants; import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; +import com.velocitypowered.proxy.connection.util.ConnectionMessages; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolConstants; import com.velocitypowered.proxy.protocol.packet.*; @@ -44,6 +45,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { connection.getPlayer().getConnection().write(packet); } else if (packet instanceof Disconnect) { Disconnect original = (Disconnect) packet; + connection.disconnect(); connection.getPlayer().handleConnectionException(connection.getServerInfo(), original); } else if (packet instanceof JoinGame) { playerHandler.handleBackendJoinGame((JoinGame) packet); @@ -113,6 +115,18 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { connection.getPlayer().handleConnectionException(connection.getServerInfo(), throwable); } + public VelocityServer getServer() { + return server; + } + + @Override + public void disconnected() { + if (connection.isGracefulDisconnect()) { + return; + } + connection.getPlayer().handleConnectionException(connection.getServerInfo(), Disconnect.create(ConnectionMessages.UNEXPECTED_DISCONNECT)); + } + private boolean canForwardPluginMessage(PluginMessage message) { ClientPlaySessionHandler playerHandler = (ClientPlaySessionHandler) connection.getPlayer().getConnection().getSessionHandler(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java index 2362a27d..496a4a68 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/VelocityServerConnection.java @@ -44,6 +44,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, private MinecraftConnection minecraftConnection; private boolean legacyForge = false; private boolean hasCompletedJoin = false; + private boolean gracefulDisconnect = false; public VelocityServerConnection(ServerInfo target, ConnectedPlayer proxyPlayer, VelocityServer server) { this.serverInfo = target; @@ -148,6 +149,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, if (minecraftConnection != null) { minecraftConnection.close(); minecraftConnection = null; + gracefulDisconnect = true; } } @@ -181,4 +183,8 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, public void setHasCompletedJoin(boolean hasCompletedJoin) { this.hasCompletedJoin = hasCompletedJoin; } + + public boolean isGracefulDisconnect() { + return gracefulDisconnect; + } } 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 2d3bd80d..b56b7475 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 @@ -205,14 +205,19 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { String plainTextReason = PASS_THRU_TRANSLATE.serialize(disconnectReason); if (connectedServer != null && connectedServer.getServerInfo().equals(info)) { logger.error("{}: kicked from server {}: {}", this, info.getName(), plainTextReason); + handleConnectionException(info, disconnectReason, TextComponent.builder() + .content("Kicked from " + info.getName() + ": ") + .color(TextColor.RED) + .append(disconnectReason) + .build()); } else { logger.error("{}: disconnected while connecting to {}: {}", this, info.getName(), plainTextReason); + handleConnectionException(info, disconnectReason, TextComponent.builder() + .content("Unable to connect to " + info.getName() + ": ") + .color(TextColor.RED) + .append(disconnectReason) + .build()); } - handleConnectionException(info, disconnectReason, TextComponent.builder() - .content("Unable to connect to " + info.getName() + ": ") - .color(TextColor.RED) - .append(disconnectReason) - .build()); } private void handleConnectionException(ServerInfo info, @Nullable Component kickReason, Component friendlyReason) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionMessages.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionMessages.java index b2ff70a4..727750d6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionMessages.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionMessages.java @@ -7,6 +7,7 @@ public class ConnectionMessages { public static final TextComponent ALREADY_CONNECTED = TextComponent.of("You are already connected to this server!", TextColor.RED); public static final TextComponent IN_PROGRESS = TextComponent.of("You are already connecting to a server!", TextColor.RED); public static final TextComponent INTERNAL_SERVER_CONNECTION_ERROR = TextComponent.of("Internal server connection error"); + public static final TextComponent UNEXPECTED_DISCONNECT = TextComponent.of("Unexpectedly disconnected from server - crash?"); private ConnectionMessages() { throw new AssertionError(); From ab568405dd1394045605005baf3960209bb42a77 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 15 Sep 2018 02:22:52 -0400 Subject: [PATCH 48/49] Cleaned up client plugin message logic. --- .../client/ClientPlaySessionHandler.java | 33 +++++++------------ .../protocol/util/PluginMessageUtil.java | 17 +++++++--- 2 files changed, 24 insertions(+), 26 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 f550865d..55b023af 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 @@ -252,8 +252,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { return serverBossBars; } - public void handleClientPluginMessage(PluginMessage packet) { - if (packet.getChannel().equals("REGISTER") || packet.getChannel().equals("minecraft:register")) { + private void handleClientPluginMessage(PluginMessage packet) { + if (PluginMessageUtil.isMCRegister(packet)) { List actuallyRegistered = new ArrayList<>(); List channels = PluginMessageUtil.getChannels(packet); for (String channel : channels) { @@ -270,21 +270,13 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { PluginMessage newRegisterPacket = PluginMessageUtil.constructChannelsPacket(packet.getChannel(), actuallyRegistered); player.getConnectedServer().getMinecraftConnection().write(newRegisterPacket); } - - return; - } - - if (packet.getChannel().equals("UNREGISTER") || packet.getChannel().equals("minecraft:unregister")) { + } else if (PluginMessageUtil.isMCUnregister(packet)) { List channels = PluginMessageUtil.getChannels(packet); clientPluginMsgChannels.removeAll(channels); - } - - if (PluginMessageUtil.isMCBrand(packet)) { + player.getConnectedServer().getMinecraftConnection().write(packet); + } else if (PluginMessageUtil.isMCBrand(packet)) { player.getConnectedServer().getMinecraftConnection().write(PluginMessageUtil.rewriteMCBrand(packet)); - return; - } - - if (player.getConnectedServer().isLegacyForge() && !player.getConnectedServer().hasCompletedJoin()) { + } else if (player.getConnectedServer().isLegacyForge() && !player.getConnectedServer().hasCompletedJoin()) { if (packet.getChannel().equals(VelocityConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) { // Always forward the FML handshake to the remote server. player.getConnectedServer().getMinecraftConnection().write(packet); @@ -294,13 +286,12 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // be sent once the JoinGame packet has been received by the proxy. loginPluginMessages.add(packet); } - return; - } - - MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(player, - ChannelSide.FROM_CLIENT, packet); - if (status == MessageHandler.ForwardStatus.FORWARD) { - player.getConnectedServer().writeIfJoined(packet); + } else { + MessageHandler.ForwardStatus status = server.getChannelRegistrar().handlePluginMessage(player, + ChannelSide.FROM_CLIENT, packet); + if (status == MessageHandler.ForwardStatus.FORWARD) { + player.getConnectedServer().writeIfJoined(packet); + } } } 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 07505848..e3fdeab3 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 @@ -20,13 +20,20 @@ public enum PluginMessageUtil { return message.getChannel().equals("MC|Brand") || message.getChannel().equals("minecraft:brand"); } + public static boolean isMCRegister(PluginMessage message) { + Preconditions.checkNotNull(message, "message"); + return message.getChannel().equals("REGISTER") || message.getChannel().equals("minecraft:register"); + } + + public static boolean isMCUnregister(PluginMessage message) { + Preconditions.checkNotNull(message, "message"); + return message.getChannel().equals("UNREGISTER") || message.getChannel().equals("minecraft:unregister"); + } + public static List getChannels(PluginMessage message) { Preconditions.checkNotNull(message, "message"); - Preconditions.checkArgument(message.getChannel().equals("REGISTER") || - message.getChannel().equals("UNREGISTER") || - message.getChannel().equals("minecraft:register") || - message.getChannel().equals("minecraft:unregister"), - "Unknown channel type " + message.getChannel()); + Preconditions.checkArgument(isMCRegister(message) || isMCUnregister(message),"Unknown channel type %s", + message.getChannel()); String channels = new String(message.getData(), StandardCharsets.UTF_8); return ImmutableList.copyOf(channels.split("\0")); } From 88b7407aaf0dabdf2a7c6bb82908b4290989dd6e Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 15 Sep 2018 13:37:30 -0400 Subject: [PATCH 49/49] Fix rare NPE during server transition. Fixes #87 --- .../client/ClientPlaySessionHandler.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 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 55b023af..c3bf3845 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 @@ -55,6 +55,12 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { @Override public void handle(MinecraftPacket packet) { + VelocityServerConnection serverConnection = player.getConnectedServer(); + if (serverConnection == null) { + // No server connection yet, probably transitioning. + return; + } + if (packet instanceof KeepAlive) { KeepAlive keepAlive = (KeepAlive) packet; if (keepAlive.getRandomId() != lastPingID) { @@ -64,7 +70,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } player.setPing(System.currentTimeMillis() - lastPingSent); resetPingData(); - player.getConnectedServer().getMinecraftConnection().write(packet); + serverConnection.getMinecraftConnection().write(packet); return; } @@ -112,7 +118,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { player.getConnection().write(response); } else { - player.getConnectedServer().getMinecraftConnection().write(packet); + serverConnection.getMinecraftConnection().write(packet); } } catch (Exception e) { logger.error("Unable to provide tab list completions for " + player.getUsername() + " for command '" + req.getCommand() + "'", e); @@ -127,15 +133,21 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } // If we don't want to handle this packet, just forward it on. - if (player.getConnectedServer().hasCompletedJoin()) { - player.getConnectedServer().getMinecraftConnection().write(packet); + if (serverConnection.hasCompletedJoin()) { + serverConnection.getMinecraftConnection().write(packet); } } @Override public void handleUnknown(ByteBuf buf) { - if (player.getConnectedServer().hasCompletedJoin()) { - player.getConnectedServer().getMinecraftConnection().write(buf.retain()); + VelocityServerConnection serverConnection = player.getConnectedServer(); + if (serverConnection == null) { + // No server connection yet, probably transitioning. + return; + } + + if (serverConnection.hasCompletedJoin()) { + serverConnection.getMinecraftConnection().write(buf.retain()); } }