From 1661cece2d8b89918df89fd008e54cc51a8c8ce7 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Wed, 24 Apr 2019 14:36:49 -0400 Subject: [PATCH] An Easter basket bearing gifts! (#191) * Delay switch to new server until after JoinGame is sent. Unfortunately, in some cases (especially vanilla Minecraft) some login disconnects are sent after ServerLoginSuccess but before JoinGame. We've been using ServerLoginSuccess but it has caused too many problems. Now Velocity will switch to a stripped-down version of the play session handler until JoinGame is received. This handler does very little by itself: it simply forwards plugin messages (for Forge) and waits for the JoinGame packet from the server. This is an initial version: only vanilla Minecraft 1.12.2 was tested. However this is the way Waterfall without entity rewriting does server switches (which, in turn, is inherited from BungeeCord). * Move to Gradle 5 and Error Prone. --- api/build.gradle | 3 +- .../plugin/ap/PluginAnnotationProcessor.java | 3 +- .../ap/SerializedPluginDescription.java | 4 +- .../com/velocitypowered/api/proxy/Player.java | 1 + .../api/proxy/server/QueryResponse.java | 11 +- gradle/checkerframework.gradle | 70 ------- gradle/errorprone.gradle | 4 + gradle/wrapper/gradle-wrapper.properties | 2 +- native/build.gradle | 3 +- .../natives/util/NativeConstraints.java | 4 +- proxy/build.gradle | 7 +- .../velocitypowered/proxy/VelocityServer.java | 1 + .../proxy/command/VelocityCommandManager.java | 6 +- .../proxy/config/VelocityConfiguration.java | 26 ++- .../proxy/connection/MinecraftConnection.java | 20 ++ .../backend/BackendConnectionPhases.java | 12 ++ .../backend/BackendPlaySessionHandler.java | 10 +- .../backend/LoginSessionHandler.java | 54 ++---- .../backend/TransitionSessionHandler.java | 177 ++++++++++++++++++ .../backend/VelocityServerConnection.java | 17 +- .../client/ClientConnectionPhase.java | 8 +- .../client/ClientPlaySessionHandler.java | 63 ++++--- .../connection/client/ConnectedPlayer.java | 69 +++++-- .../client/InitialConnectSessionHandler.java | 13 +- .../LegacyForgeHandshakeClientPhase.java | 42 ++--- .../util/ConnectionRequestResults.java | 93 +++++---- .../proxy/network/http/NettyHttpClient.java | 36 ---- .../plugin/loader/java/JavaPluginLoader.java | 3 +- .../proxy/protocol/StateRegistry.java | 2 +- .../proxy/protocol/netty/GS4QueryHandler.java | 6 +- .../protocol/packet/LegacyDisconnect.java | 2 +- .../proxy/protocol/packet/PlayerListItem.java | 4 +- .../proxy/protocol/packet/PluginMessage.java | 2 +- .../util/PluginDependencyUtilsTest.java | 2 +- settings.gradle | 4 +- 35 files changed, 475 insertions(+), 309 deletions(-) delete mode 100644 gradle/checkerframework.gradle create mode 100644 gradle/errorprone.gradle create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java diff --git a/api/build.gradle b/api/build.gradle index 335427110..7fb454545 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -2,10 +2,11 @@ plugins { id 'java' id 'maven-publish' id 'checkstyle' + id "net.ltgt.errorprone" version "0.8" } -apply from: '../gradle/checkerframework.gradle' apply from: '../gradle/checkstyle.gradle' +apply from: '../gradle/errorprone.gradle' apply plugin: 'com.github.johnrengelman.shadow' sourceSets { diff --git a/api/src/ap/java/com/velocitypowered/api/plugin/ap/PluginAnnotationProcessor.java b/api/src/ap/java/com/velocitypowered/api/plugin/ap/PluginAnnotationProcessor.java index 661d3a015..eef691ba4 100644 --- a/api/src/ap/java/com/velocitypowered/api/plugin/ap/PluginAnnotationProcessor.java +++ b/api/src/ap/java/com/velocitypowered/api/plugin/ap/PluginAnnotationProcessor.java @@ -38,7 +38,8 @@ public class PluginAnnotationProcessor extends AbstractProcessor { } @Override - public boolean process(Set annotations, RoundEnvironment roundEnv) { + public synchronized boolean process(Set annotations, + RoundEnvironment roundEnv) { if (roundEnv.processingOver()) { return false; } diff --git a/api/src/ap/java/com/velocitypowered/api/plugin/ap/SerializedPluginDescription.java b/api/src/ap/java/com/velocitypowered/api/plugin/ap/SerializedPluginDescription.java index 5d370f827..9e0cf7d5a 100644 --- a/api/src/ap/java/com/velocitypowered/api/plugin/ap/SerializedPluginDescription.java +++ b/api/src/ap/java/com/velocitypowered/api/plugin/ap/SerializedPluginDescription.java @@ -12,7 +12,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import org.checkerframework.checker.nullness.qual.Nullable; -public class SerializedPluginDescription { +public final class SerializedPluginDescription { public static final Pattern ID_PATTERN = Pattern.compile("[a-z][a-z0-9-_]{0,63}"); @@ -123,7 +123,7 @@ public class SerializedPluginDescription { + '}'; } - public static class Dependency { + public static final class Dependency { private final String id; private final boolean optional; diff --git a/api/src/main/java/com/velocitypowered/api/proxy/Player.java b/api/src/main/java/com/velocitypowered/api/proxy/Player.java index 21e22221f..54fcc8d99 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/Player.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/Player.java @@ -70,6 +70,7 @@ public interface Player extends CommandSource, InboundConnection, ChannelMessage * * @param component the chat message to send */ + @Override default void sendMessage(Component component) { sendMessage(component, MessagePosition.CHAT); } diff --git a/api/src/main/java/com/velocitypowered/api/proxy/server/QueryResponse.java b/api/src/main/java/com/velocitypowered/api/proxy/server/QueryResponse.java index 09993c366..fab5f2fb2 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/server/QueryResponse.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/server/QueryResponse.java @@ -3,6 +3,7 @@ package com.velocitypowered.api.proxy.server; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.velocitypowered.api.proxy.config.ProxyConfig; import java.util.ArrayList; @@ -26,14 +27,14 @@ public final class QueryResponse { private final int maxPlayers; private final String proxyHost; private final int proxyPort; - private final Collection players; + private final ImmutableCollection players; private final String proxyVersion; - private final Collection plugins; + private final ImmutableCollection plugins; @VisibleForTesting QueryResponse(String hostname, String gameVersion, String map, int currentPlayers, - int maxPlayers, String proxyHost, int proxyPort, Collection players, - String proxyVersion, Collection plugins) { + int maxPlayers, String proxyHost, int proxyPort, ImmutableCollection players, + String proxyVersion, ImmutableCollection plugins) { this.hostname = hostname; this.gameVersion = gameVersion; this.map = map; @@ -403,7 +404,7 @@ public final class QueryResponse { /** * Represents a plugin in the query response. */ - public static class PluginInformation { + public static final class PluginInformation { private final String name; private final @Nullable String version; diff --git a/gradle/checkerframework.gradle b/gradle/checkerframework.gradle deleted file mode 100644 index a2643d10f..000000000 --- a/gradle/checkerframework.gradle +++ /dev/null @@ -1,70 +0,0 @@ -/////////////////////////////////////////////////////////////////////////// -/// Checker Framework pluggable type-checking -/// - -repositories { - mavenCentral() -} - -configurations { - checkerFrameworkCheckerJar { - description = 'the Checker Framework, including the Type Annotations compiler' - } - - checkerFrameworkAnnotatedJDK { - description = 'a copy of JDK classes with Checker Framework type qualifers inserted' - } -} - -// By default, use Checker Framework from Maven Central. -// Pass -PcfLocal to use a locally-built version of the Checker Framework. -dependencies { - if (!rootProject.hasProperty('cfLocal')) { - checkerFrameworkAnnotatedJDK "org.checkerframework:jdk8:${checkerFrameworkVersion}" - checkerFrameworkCheckerJar "org.checkerframework:checker:${checkerFrameworkVersion}" - implementation "org.checkerframework:checker-qual:${checkerFrameworkVersion}" - } else if (System.getenv("CHECKERFRAMEWORK") == null) { - throw new GradleException("Environment variable CHECKERFRAMEWORK is not set") - } else if (!file(System.getenv("CHECKERFRAMEWORK")).exists()) { - throw new GradleException("Environment variable CHECKERFRAMEWORK is set to non-existent directory " + System.getenv("CHECKERFRAMEWORK")); - } else { - ext.checkerframeworkdist = "$System.env.CHECKERFRAMEWORK/checker/dist" - checkerFrameworkAnnotatedJDK fileTree(dir: "${ext.checkerframeworkdist}", include: "jdk8.jar") - checkerFrameworkCheckerJar fileTree(dir: "${ext.checkerframeworkdist}", include: 'checker.jar') - implementation fileTree(dir: "${ext.checkerframeworkdist}", include: 'checker-qual.jar') - } -} - -// // To type-check all projects. -// allprojects { -// tasks.withType(JavaCompile).all { JavaCompile compile -> -// compile.doFirst { -// compile.options.compilerArgs = [ -// '-processor', 'org.checkerframework.checker.formatter.FormatterChecker,org.checkerframework.checker.index.IndexChecker,org.checkerframework.checker.lock.LockChecker,org.checkerframework.checker.nullness.NullnessChecker,org.checkerframework.checker.signature.SignatureChecker', -// '-Xmaxerrs', '10000', -// '-Awarns', // -Awarns turns Checker Framework errors into warnings -// '-AcheckPurityAnnotations', -// '-processorpath', "${configurations.checkerFrameworkCheckerJar.asPath}", -// "-Xbootclasspath/p:${configurations.checkerFrameworkAnnotatedJDK.asPath}", -// "-Astubs=$System.env.CHECKERFRAMEWORK/checker/resources/javadoc.astub" // TODO: does not work when downloading from Maven Central -// ] -// } -// } -// } - -// To typecheck only the current project's main source set (in a multi-project -// build), use this instead: -compileJava { - doFirst { - options.compilerArgs = [ - '-processor', 'org.checkerframework.checker.nullness.NullnessChecker', - '-processor', 'org.checkerframework.checker.optional.OptionalChecker', - '-Xmaxerrs', '10000', - '-Xmaxwarns', '10000', - // '-Awarns', // -Awarns turns Checker Framework errors into warnings - //'-AcheckPurityAnnotations', // Disabled for Velocity, wish we could do better - '-processorpath', "${configurations.checkerFrameworkCheckerJar.asPath}", - "-Xbootclasspath/p:${configurations.checkerFrameworkAnnotatedJDK.asPath}" - ] - } -} diff --git a/gradle/errorprone.gradle b/gradle/errorprone.gradle new file mode 100644 index 000000000..4a1334c1f --- /dev/null +++ b/gradle/errorprone.gradle @@ -0,0 +1,4 @@ +dependencies { + errorprone("com.google.errorprone:error_prone_core:2.3.3") + errorproneJavac("com.google.errorprone:javac:9+181-r4173-1") +} \ No newline at end of file diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index c77aa2dc3..54c60fef7 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.10-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-5.4-all.zip diff --git a/native/build.gradle b/native/build.gradle index 00b298325..107b5f246 100644 --- a/native/build.gradle +++ b/native/build.gradle @@ -1,10 +1,11 @@ plugins { id 'java' id 'checkstyle' + id "net.ltgt.errorprone" version "0.8" } -apply from: '../gradle/checkerframework.gradle' apply from: '../gradle/checkstyle.gradle' +apply from: '../gradle/errorprone.gradle' dependencies { compile "com.google.guava:guava:${guavaVersion}" diff --git a/native/src/main/java/com/velocitypowered/natives/util/NativeConstraints.java b/native/src/main/java/com/velocitypowered/natives/util/NativeConstraints.java index 0657577a2..bf5b73002 100644 --- a/native/src/main/java/com/velocitypowered/natives/util/NativeConstraints.java +++ b/native/src/main/java/com/velocitypowered/natives/util/NativeConstraints.java @@ -21,13 +21,13 @@ public class NativeConstraints { return NATIVES_ENABLED && CAN_GET_MEMORYADDRESS && System.getProperty("os.name", "").equalsIgnoreCase("Mac OS X") - && System.getProperty("os.arch").equals("x86_64"); + && System.getProperty("os.arch", "").equals("x86_64"); }; static final BooleanSupplier LINUX = () -> { return NATIVES_ENABLED && CAN_GET_MEMORYADDRESS && System.getProperty("os.name", "").equalsIgnoreCase("Linux") - && System.getProperty("os.arch").equals("amd64"); + && System.getProperty("os.arch", "").equals("amd64"); }; } diff --git a/proxy/build.gradle b/proxy/build.gradle index 838e740ac..7c1bcaa14 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -1,10 +1,13 @@ +import com.github.jengelman.gradle.plugins.shadow.transformers.Log4j2PluginsCacheFileTransformer + plugins { id 'java' id 'checkstyle' + id "net.ltgt.errorprone" version "0.8" } -apply from: '../gradle/checkerframework.gradle' apply from: '../gradle/checkstyle.gradle' +apply from: '../gradle/errorprone.gradle' apply plugin: 'com.github.johnrengelman.shadow' jar { @@ -20,7 +23,7 @@ jar { } shadowJar { - transform(com.github.jengelman.gradle.plugins.shadow.transformers.Log4j2PluginsCacheFileTransformer) + transform(Log4j2PluginsCacheFileTransformer) } tasks.withType(Checkstyle) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 4e73dd073..29bfd1418 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -109,6 +109,7 @@ public class VelocityServer implements ProxyServer { return ensureInitialized(serverKeyPair); } + @Override public VelocityConfiguration getConfiguration() { return ensureInitialized(this.configuration); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java index 1e7e7d936..dcd47339c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -93,14 +93,14 @@ public class VelocityCommandManager implements CommandManager { String alias = split[0]; if (split.length == 1) { // Offer to fill in commands. - List availableCommands = new ArrayList<>(); + ImmutableList.Builder availableCommands = ImmutableList.builder(); for (Map.Entry entry : commands.entrySet()) { if (entry.getKey().regionMatches(true, 0, alias, 0, alias.length()) && entry.getValue().hasPermission(source, new String[0])) { availableCommands.add("/" + entry.getKey()); } } - return availableCommands; + return availableCommands.build(); } Command command = commands.get(alias.toLowerCase(Locale.ENGLISH)); @@ -116,7 +116,7 @@ public class VelocityCommandManager implements CommandManager { return ImmutableList.of(); } - return command.suggest(source, actualArgs); + return ImmutableList.copyOf(command.suggest(source, actualArgs)); } catch (Exception e) { throw new RuntimeException( "Unable to invoke suggestions for command " + alias + " for " + source, e); 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 558d5a955..aa3410cee 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -245,14 +245,17 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi return AddressUtil.parseAddress(bind); } + @Override public boolean isQueryEnabled() { return query.isQueryEnabled(); } + @Override public int getQueryPort() { return query.getQueryPort(); } + @Override public String getQueryMap() { return query.getQueryMap(); } @@ -271,6 +274,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi * * @return the MOTD */ + @Override public Component getMotdComponent() { if (motdAsComponent == null) { if (motd.startsWith("{")) { @@ -282,10 +286,12 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi return motdAsComponent; } + @Override public int getShowMaxPlayers() { return showMaxPlayers; } + @Override public boolean isOnlineMode() { return onlineMode; } @@ -298,42 +304,52 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi return forwardingSecret; } + @Override public Map getServers() { return servers.getServers(); } + @Override public List getAttemptConnectionOrder() { return servers.getAttemptConnectionOrder(); } + @Override public Map> getForcedHosts() { return forcedHosts.getForcedHosts(); } + @Override public int getCompressionThreshold() { return advanced.getCompressionThreshold(); } + @Override public int getCompressionLevel() { return advanced.getCompressionLevel(); } + @Override public int getLoginRatelimit() { return advanced.getLoginRatelimit(); } + @Override public Optional getFavicon() { return Optional.ofNullable(favicon); } + @Override public boolean isAnnounceForge() { return announceForge; } + @Override public int getConnectTimeout() { return advanced.getConnectionTimeout(); } + @Override public int getReadTimeout() { return advanced.getReadTimeout(); } @@ -402,15 +418,9 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi advanced, query ); - upgradeConfig(configuration, toml); return configuration; } - private static void upgradeConfig(VelocityConfiguration configuration, Toml toml) { - // Will be implemented once there has been a backwards-incompatible change in the config file - // format. - } - private static String generateRandomString(int length) { String chars = "AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz1234567890"; StringBuilder builder = new StringBuilder(); @@ -547,7 +557,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi "compress all packets, and setting it to -1 will disable compression entirely." }) @ConfigKey("compression-threshold") - private int compressionThreshold = 1024; + private int compressionThreshold = 256; @Comment({"How much compression should be done (from 0-9). The default is -1, which uses the", "default level of 6."}) @@ -579,7 +589,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi private Advanced(Toml toml) { if (toml != null) { - this.compressionThreshold = toml.getLong("compression-threshold", 1024L).intValue(); + this.compressionThreshold = toml.getLong("compression-threshold", 256L).intValue(); this.compressionLevel = toml.getLong("compression-level", -1L).intValue(); this.loginRatelimit = toml.getLong("login-ratelimit", 3000L).intValue(); this.connectionTimeout = toml.getLong("connection-timeout", 5000L).intValue(); 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 f210ac402..beefa6b05 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -224,6 +224,26 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { return state; } + public boolean isAutoReading() { + return channel.config().isAutoRead(); + } + + /** + * Determines whether or not the channel should continue reading data automaticaly. + * @param autoReading whether or not we should read data automatically + */ + public void setAutoReading(boolean autoReading) { + channel.config().setAutoRead(autoReading); + if (autoReading) { + // For some reason, the channel may not completely read its queued contents once autoread + // is turned back on, even though toggling autoreading on should handle things automatically. + // We will issue an explicit read after turning on autoread. + // + // Much thanks to @creeper123123321. + channel.read(); + } + } + /** * Changes the state of the Minecraft connection. * @param state the new state diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendConnectionPhases.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendConnectionPhases.java index 8780f8ef3..09d17053d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendConnectionPhases.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendConnectionPhases.java @@ -36,6 +36,18 @@ public final class BackendConnectionPhases { } }; + /** + * A special backend phase used to indicate that this connection is about to become + * obsolete (transfer to a new server, for instance) and that Forge messages ought to be + * forwarded on to an in-flight connection instead. + */ + public static final BackendConnectionPhase IN_TRANSITION = new BackendConnectionPhase() { + @Override + public boolean consideredComplete() { + return true; + } + }; + private BackendConnectionPhases() { throw new AssertionError(); } 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 1a3b3b817..dad35321a 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,13 +70,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { @Override public boolean handle(Disconnect packet) { serverConn.disconnect(); - serverConn.getPlayer().handleConnectionException(serverConn.getServer(), packet); - return true; - } - - @Override - public boolean handle(JoinGame packet) { - playerSessionHandler.handleBackendJoinGame(packet); + serverConn.getPlayer().handleConnectionException(serverConn.getServer(), packet, true); return true; } @@ -169,7 +163,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { @Override public void exception(Throwable throwable) { exceptionTriggered = true; - serverConn.getPlayer().handleConnectionException(serverConn.getServer(), throwable); + serverConn.getPlayer().handleConnectionException(serverConn.getServer(), throwable, true); } public VelocityServer getServer() { 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 3f92556ff..14b7ad22c 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 @@ -1,6 +1,5 @@ package com.velocitypowered.proxy.connection.backend; -import com.velocitypowered.api.event.player.ServerConnectedEvent; import com.velocitypowered.api.proxy.ConnectionRequestBuilder; import com.velocitypowered.api.util.GameProfile; import com.velocitypowered.proxy.VelocityServer; @@ -9,8 +8,8 @@ import com.velocitypowered.proxy.config.VelocityConfiguration; import com.velocitypowered.proxy.connection.MinecraftConnection; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.connection.VelocityConstants; -import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; import com.velocitypowered.proxy.connection.util.ConnectionRequestResults; +import com.velocitypowered.proxy.connection.util.ConnectionRequestResults.Impl; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.StateRegistry; import com.velocitypowered.proxy.protocol.packet.Disconnect; @@ -37,24 +36,16 @@ public class LoginSessionHandler implements MinecraftSessionHandler { private final VelocityServer server; private final VelocityServerConnection serverConn; - private final CompletableFuture resultFuture; + private final CompletableFuture resultFuture; private boolean informationForwarded; LoginSessionHandler(VelocityServer server, VelocityServerConnection serverConn, - CompletableFuture resultFuture) { + CompletableFuture resultFuture) { this.server = server; this.serverConn = serverConn; this.resultFuture = resultFuture; } - private MinecraftConnection ensureMinecraftConnection() { - MinecraftConnection mc = serverConn.getConnection(); - if (mc == null) { - throw new IllegalStateException("Not connected to backend server!"); - } - return mc; - } - @Override public boolean handle(EncryptionRequest packet) { throw new IllegalStateException("Backend server is online-mode!"); @@ -62,7 +53,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { @Override public boolean handle(LoginPluginMessage packet) { - MinecraftConnection mc = ensureMinecraftConnection(); + MinecraftConnection mc = serverConn.ensureConnected(); VelocityConfiguration configuration = server.getConfiguration(); if (configuration.getPlayerInfoForwardingMode() == PlayerInfoForwarding.MODERN && packet .getChannel() @@ -95,7 +86,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { @Override public boolean handle(SetCompression packet) { - ensureMinecraftConnection().setCompressionThreshold(packet.getThreshold()); + serverConn.ensureConnected().setCompressionThreshold(packet.getThreshold()); return true; } @@ -109,36 +100,15 @@ public class LoginSessionHandler implements MinecraftSessionHandler { return true; } - // The player has been logged on to the backend server. - MinecraftConnection smc = ensureMinecraftConnection(); + // The player has been logged on to the backend server, but we're not done yet. There could be + // other problems that could arise before we get a JoinGame packet from the server. + + // Move into the PLAY phase. + MinecraftConnection smc = serverConn.ensureConnected(); smc.setState(StateRegistry.PLAY); - VelocityServerConnection existingConnection = serverConn.getPlayer().getConnectedServer(); - if (existingConnection == null) { - // Strap on the play session handler - serverConn.getPlayer().getMinecraftConnection() - .setSessionHandler(new ClientPlaySessionHandler(server, serverConn.getPlayer())); - } else { - // For Legacy Forge - existingConnection.getPhase().onDepartForNewServer(serverConn, serverConn.getPlayer()); - // Shut down the existing server connection. - serverConn.getPlayer().setConnectedServer(null); - existingConnection.disconnect(); - - // Send keep alive to try to avoid timeouts - serverConn.getPlayer().sendKeepAlive(); - } - - smc.getChannel().config().setAutoRead(false); - server.getEventManager() - .fire(new ServerConnectedEvent(serverConn.getPlayer(), serverConn.getServer())) - .whenCompleteAsync((x, error) -> { - resultFuture.complete(ConnectionRequestResults.successful(serverConn.getServer())); - smc.setSessionHandler(new BackendPlaySessionHandler(server, serverConn)); - serverConn.getPlayer().setConnectedServer(serverConn); - smc.getChannel().config().setAutoRead(true); - smc.getChannel().read(); - }, smc.eventLoop()); + // Switch to the transition handler. + smc.setSessionHandler(new TransitionSessionHandler(server, serverConn, resultFuture)); return true; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java new file mode 100644 index 000000000..82ba53419 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java @@ -0,0 +1,177 @@ +package com.velocitypowered.proxy.connection.backend; + +import static com.velocitypowered.proxy.connection.backend.BackendConnectionPhases.IN_TRANSITION; +import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeHandshakeBackendPhase.HELLO; + +import com.velocitypowered.api.event.player.ServerConnectedEvent; +import com.velocitypowered.api.network.ProtocolVersion; +import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.connection.ConnectionTypes; +import com.velocitypowered.proxy.connection.MinecraftConnection; +import com.velocitypowered.proxy.connection.MinecraftSessionHandler; +import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; +import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants; +import com.velocitypowered.proxy.connection.util.ConnectionRequestResults; +import com.velocitypowered.proxy.connection.util.ConnectionRequestResults.Impl; +import com.velocitypowered.proxy.protocol.packet.Disconnect; +import com.velocitypowered.proxy.protocol.packet.JoinGame; +import com.velocitypowered.proxy.protocol.packet.KeepAlive; +import com.velocitypowered.proxy.protocol.packet.PluginMessage; +import java.io.IOException; +import java.util.concurrent.CompletableFuture; + +/** + * A special session handler that catches "last minute" disconnects. + */ +public class TransitionSessionHandler implements MinecraftSessionHandler { + + private final VelocityServer server; + private final VelocityServerConnection serverConn; + private final CompletableFuture resultFuture; + + /** + * Creates the new transition handler. + * @param server the Velocity server instance + * @param serverConn the server connection + * @param resultFuture the result future + */ + TransitionSessionHandler(VelocityServer server, + VelocityServerConnection serverConn, + CompletableFuture resultFuture) { + this.server = server; + this.serverConn = serverConn; + this.resultFuture = resultFuture; + } + + @Override + public boolean beforeHandle() { + if (!serverConn.isActive()) { + // Obsolete connection + serverConn.disconnect(); + return true; + } + return false; + } + + @Override + public boolean handle(KeepAlive packet) { + serverConn.ensureConnected().write(packet); + return true; + } + + @Override + public boolean handle(JoinGame packet) { + MinecraftConnection smc = serverConn.ensureConnected(); + VelocityServerConnection existingConnection = serverConn.getPlayer().getConnectedServer(); + + if (existingConnection != null) { + // Shut down the existing server connection. + serverConn.getPlayer().setConnectedServer(null); + existingConnection.disconnect(); + + // Send keep alive to try to avoid timeouts + serverConn.getPlayer().sendKeepAlive(); + } + + // The goods are in hand! We got JoinGame. Let's transition completely to the new state. + smc.setAutoReading(false); + server.getEventManager() + .fire(new ServerConnectedEvent(serverConn.getPlayer(), serverConn.getServer())) + .whenCompleteAsync((x, error) -> { + // Strap on the ClientPlaySessionHandler if required. + ClientPlaySessionHandler playHandler; + if (serverConn.getPlayer().getMinecraftConnection().getSessionHandler() + instanceof ClientPlaySessionHandler) { + playHandler = (ClientPlaySessionHandler) serverConn.getPlayer().getMinecraftConnection() + .getSessionHandler(); + } else { + playHandler = new ClientPlaySessionHandler(server, serverConn.getPlayer()); + serverConn.getPlayer().getMinecraftConnection().setSessionHandler(playHandler); + } + playHandler.handleBackendJoinGame(packet, serverConn); + + // Strap on the correct session handler for the server. We will have nothing more to do + // with this connection once this task finishes up. + smc.setSessionHandler(new BackendPlaySessionHandler(server, serverConn)); + + // Clean up disabling auto-read while the connected event was being processed. + smc.setAutoReading(true); + + // Now set the connected server. + serverConn.getPlayer().setConnectedServer(serverConn); + + // We're done! :) + resultFuture.complete(ConnectionRequestResults.successful(serverConn.getServer())); + }, smc.eventLoop()); + + return true; + } + + @Override + public boolean handle(Disconnect packet) { + final MinecraftConnection connection = serverConn.ensureConnected(); + serverConn.disconnect(); + + // If we were in the middle of the Forge handshake, it is not safe to proceed. We must kick + // the client. + if (connection.getType() == ConnectionTypes.LEGACY_FORGE + && !serverConn.getPhase().consideredComplete()) { + resultFuture.complete(ConnectionRequestResults.forUnsafeDisconnect(packet, + serverConn.getServer())); + } else { + resultFuture.complete(ConnectionRequestResults.forDisconnect(packet, serverConn.getServer())); + } + + return true; + } + + @Override + public boolean handle(PluginMessage packet) { + if (!canForwardPluginMessage(packet)) { + return true; + } + + // We always need to handle plugin messages, for Forge compatibility. + if (serverConn.getPhase().handle(serverConn, serverConn.getPlayer(), packet)) { + // Handled, but check the server connection phase. + if (serverConn.getPhase() == HELLO) { + VelocityServerConnection existingConnection = serverConn.getPlayer().getConnectedServer(); + if (existingConnection != null && existingConnection.getPhase() != IN_TRANSITION) { + // Indicate that this connection is "in transition" + existingConnection.setConnectionPhase(IN_TRANSITION); + + // Tell the player that we're leaving and we just aren't coming back. + existingConnection.getPhase().onDepartForNewServer(existingConnection, + serverConn.getPlayer()); + } + } + return true; + } + + serverConn.getPlayer().getMinecraftConnection().write(packet); + return true; + } + + @Override + public void disconnected() { + resultFuture + .completeExceptionally(new IOException("Unexpectedly disconnected from remote server")); + } + + private boolean canForwardPluginMessage(PluginMessage message) { + MinecraftConnection mc = serverConn.getConnection(); + if (mc == null) { + return false; + } + boolean minecraftOrFmlMessage; + if (mc.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_12_2) <= 0) { + String channel = message.getChannel(); + minecraftOrFmlMessage = channel.startsWith("MC|") || channel + .startsWith(LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL); + } else { + minecraftOrFmlMessage = message.getChannel().startsWith("minecraft:"); + } + return minecraftOrFmlMessage + || server.getChannelRegistrar().registered(message.getChannel()); + } +} 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 90dd6f90c..285912c8e 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 @@ -22,6 +22,7 @@ import com.velocitypowered.proxy.connection.ConnectionTypes; import com.velocitypowered.proxy.connection.MinecraftConnection; import com.velocitypowered.proxy.connection.MinecraftConnectionAssociation; import com.velocitypowered.proxy.connection.client.ConnectedPlayer; +import com.velocitypowered.proxy.connection.util.ConnectionRequestResults.Impl; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.StateRegistry; import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder; @@ -71,8 +72,8 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, * @return a {@link com.velocitypowered.api.proxy.ConnectionRequestBuilder.Result} representing * whether or not the connect succeeded */ - public CompletableFuture connect() { - CompletableFuture result = new CompletableFuture<>(); + public CompletableFuture connect() { + CompletableFuture result = new CompletableFuture<>(); // Note: we use the event loop for the connection the player is on. This reduces context // switches. server.initializeGenericBootstrap(proxyPlayer.getMinecraftConnection().eventLoop()) @@ -164,6 +165,18 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, return connection; } + /** + * Ensures the connection is still active and throws an exception if it is not. + * @return the active connection + * @throws IllegalStateException if the connection is inactive + */ + public MinecraftConnection ensureConnected() { + if (connection == null) { + throw new IllegalStateException("Not connected to server!"); + } + return connection; + } + @Override public VelocityRegisteredServer getServer() { return registeredServer; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConnectionPhase.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConnectionPhase.java index b5177e73e..ba45251a7 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConnectionPhase.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConnectionPhase.java @@ -1,5 +1,6 @@ package com.velocitypowered.proxy.connection.client; +import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeHandshakeClientPhase; import com.velocitypowered.proxy.protocol.packet.PluginMessage; @@ -16,14 +17,13 @@ public interface ClientConnectionPhase { * this phase. * * @param player The player - * @param handler The {@link ClientPlaySessionHandler} that is handling - * packets * @param message The message to handle + * @param server The backend connection to use * @return true if handled, false otherwise. */ default boolean handle(ConnectedPlayer player, - ClientPlaySessionHandler handler, - PluginMessage message) { + PluginMessage message, + VelocityServerConnection server) { return false; } 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 86bd810ce..74b1d6730 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 @@ -10,6 +10,7 @@ import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.MinecraftConnection; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; +import com.velocitypowered.proxy.connection.backend.BackendConnectionPhases; import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.StateRegistry; @@ -160,7 +161,6 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { return false; } - List suggestions = server.getCommandManager().offerSuggestions(player, command); if (suggestions.isEmpty()) { return false; @@ -226,23 +226,34 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { backendConn.write(packet); } else if (PluginMessageUtil.isMcBrand(packet)) { backendConn.write(PluginMessageUtil.rewriteMinecraftBrand(packet, server.getVersion())); - } else if (!player.getPhase().handle(player, this, packet)) { - if (!player.getPhase().consideredComplete() || !serverConn.getPhase() - .consideredComplete()) { - // 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 FML handshake has completed or the - // JoinGame packet has been received by the proxy, whichever comes first. - loginPluginMessages.add(packet); - } else { - ChannelIdentifier id = server.getChannelRegistrar().getFromId(packet.getChannel()); - if (id == null) { - backendConn.write(packet); + } else { + if (serverConn.getPhase() == BackendConnectionPhases.IN_TRANSITION) { + // We must bypass the currently-connected server when forwarding Forge packets. + VelocityServerConnection inFlight = player.getConnectionInFlight(); + if (inFlight != null) { + player.getPhase().handle(player, packet, inFlight); + } + return true; + } + + if (!player.getPhase().handle(player, packet, serverConn)) { + if (!player.getPhase().consideredComplete() || !serverConn.getPhase() + .consideredComplete()) { + // The client is trying to send messages too early. This is primarily caused by mods, + // but further aggravated by Velocity. To work around these issues, we will queue any + // non-FML handshake messages to be sent once the FML handshake has completed or the + // JoinGame packet has been received by the proxy, whichever comes first. + loginPluginMessages.add(packet); } else { - PluginMessageEvent event = new PluginMessageEvent(player, serverConn, id, - packet.getData()); - server.getEventManager().fire(event).thenAcceptAsync(pme -> backendConn.write(packet), - backendConn.eventLoop()); + ChannelIdentifier id = server.getChannelRegistrar().getFromId(packet.getChannel()); + if (id == null) { + backendConn.write(packet); + } else { + PluginMessageEvent event = new PluginMessageEvent(player, serverConn, id, + packet.getData()); + server.getEventManager().fire(event).thenAcceptAsync(pme -> backendConn.write(packet), + backendConn.eventLoop()); + } } } } @@ -304,7 +315,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { boolean writable = player.getMinecraftConnection().getChannel().isWritable(); MinecraftConnection smc = serverConn.getConnection(); if (smc != null) { - smc.getChannel().config().setAutoRead(writable); + smc.setAutoReading(writable); } } } @@ -313,18 +324,10 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { * Handles the {@code JoinGame} packet. This function is responsible for handling the client-side * switching servers in Velocity. * @param joinGame the join game packet + * @param destination the new server we are connecting to */ - public void handleBackendJoinGame(JoinGame joinGame) { - VelocityServerConnection serverConn = player.getConnectedServer(); - if (serverConn == null) { - throw new IllegalStateException( - "No server connection for " + player + ", but JoinGame packet received"); - } - MinecraftConnection serverMc = serverConn.getConnection(); - if (serverMc == null) { - throw new IllegalStateException( - "Server connection for " + player + " is disconnected, but JoinGame packet received"); - } + public void handleBackendJoinGame(JoinGame joinGame, VelocityServerConnection destination) { + final MinecraftConnection serverMc = destination.ensureConnected(); if (!spawned) { // Nothing special to do with regards to spawning the player @@ -394,7 +397,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // Flush everything player.getMinecraftConnection().flush(); serverMc.flush(); - serverConn.completeJoin(); + destination.completeJoin(); } 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 3d5fadd04..937faabb5 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 @@ -33,6 +33,7 @@ import com.velocitypowered.proxy.connection.MinecraftConnectionAssociation; import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; import com.velocitypowered.proxy.connection.util.ConnectionMessages; import com.velocitypowered.proxy.connection.util.ConnectionRequestResults; +import com.velocitypowered.proxy.connection.util.ConnectionRequestResults.Impl; import com.velocitypowered.proxy.protocol.StateRegistry; import com.velocitypowered.proxy.protocol.packet.Chat; import com.velocitypowered.proxy.protocol.packet.ClientSettings; @@ -132,6 +133,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { this.ping = ping; } + @Override public PlayerSettings getPlayerSettings() { return settings == null ? ClientSettingsWrapper.DEFAULT : this.settings; } @@ -142,6 +144,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { server.getEventManager().fireAndForget(new PlayerSettingsChangedEvent(this, cs)); } + @Override public Optional getModInfo() { return Optional.ofNullable(modInfo); } @@ -293,6 +296,10 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { return connectedServer; } + public @Nullable VelocityServerConnection getConnectionInFlight() { + return connectionInFlight; + } + public void resetInFlightConnection() { connectionInFlight = null; } @@ -301,8 +308,15 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { * Handles unexpected disconnects. * @param server the server we disconnected from * @param throwable the exception + * @param safe whether or not we can safely reconnect to a new server */ - public void handleConnectionException(RegisteredServer server, Throwable throwable) { + public void handleConnectionException(RegisteredServer server, Throwable throwable, + boolean safe) { + if (!isActive()) { + // If the connection is no longer active, it makes no sense to try and recover it. + return; + } + if (throwable == null) { throw new NullPointerException("throwable"); } @@ -317,22 +331,29 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { String userMessage; if (connectedServer != null && connectedServer.getServerInfo().equals(server.getServerInfo())) { userMessage = "Your connection to " + server.getServerInfo().getName() + " encountered an " - + " error."; + + "error."; } else { logger.error("{}: unable to connect to server {}", this, server.getServerInfo().getName(), wrapped); userMessage = "Unable to connect to " + server.getServerInfo().getName() + ". Try again " + "later."; } - handleConnectionException(server, null, TextComponent.of(userMessage, TextColor.RED)); + handleConnectionException(server, null, TextComponent.of(userMessage, TextColor.RED), safe); } /** * Handles unexpected disconnects. * @param server the server we disconnected from * @param disconnect the disconnect packet + * @param safe whether or not we can safely reconnect to a new server */ - public void handleConnectionException(RegisteredServer server, Disconnect disconnect) { + public void handleConnectionException(RegisteredServer server, Disconnect disconnect, + boolean safe) { + if (!isActive()) { + // If the connection is no longer active, it makes no sense to try and recover it. + return; + } + Component disconnectReason = ComponentSerializers.JSON.deserialize(disconnect.getReason()); String plainTextReason = PASS_THRU_TRANSLATE.serialize(disconnectReason); if (connectedServer != null && connectedServer.getServerInfo().equals(server.getServerInfo())) { @@ -342,7 +363,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { .content("Kicked from " + server.getServerInfo().getName() + ": ") .color(TextColor.RED) .append(disconnectReason) - .build()); + .build(), safe); } else { logger.error("{}: disconnected while connecting to {}: {}", this, server.getServerInfo().getName(), plainTextReason); @@ -350,12 +371,25 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { .content("Can't connect to server " + server.getServerInfo().getName() + ": ") .color(TextColor.RED) .append(disconnectReason) - .build()); + .build(), safe); } } private void handleConnectionException(RegisteredServer rs, - @Nullable Component kickReason, Component friendlyReason) { + @Nullable Component kickReason, Component friendlyReason, boolean safe) { + if (!isActive()) { + // If the connection is no longer active, it makes no sense to try and recover it. + return; + } + + if (!safe) { + // /!\ IT IS UNSAFE TO CONTINUE /!\ + // + // This is usually triggered by a failed Forge handshake. + disconnect(friendlyReason); + return; + } + if (connectedServer == null) { // The player isn't yet connected to a server. Optional nextServer = getNextServerToTry(rs); @@ -620,8 +654,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { return Optional.empty(); } - @Override - public CompletableFuture connect() { + private CompletableFuture internalConnect() { Optional initialCheck = checkServer(toConnect); if (initialCheck.isPresent()) { return CompletableFuture @@ -655,14 +688,26 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { }); } + @Override + public CompletableFuture connect() { + return this.internalConnect() + .whenCompleteAsync((status, throwable) -> { + if (status != null && !status.isSafe()) { + // If it's not safe to continue the connection we need to shut it down. + handleConnectionException(status.getAttemptedConnection(), throwable, true); + } + }) + .thenApply(x -> x); + } + @Override public CompletableFuture connectWithIndication() { - return connect() + return internalConnect() .whenCompleteAsync((status, throwable) -> { if (throwable != null) { // TODO: The exception handling from this is not very good. Find a better way. handleConnectionException(status != null ? status.getAttemptedConnection() - : toConnect, throwable); + : toConnect, throwable, true); return; } @@ -678,7 +723,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { break; case SERVER_DISCONNECTED: handleConnectionException(toConnect, Disconnect.create(status.getReason() - .orElse(ConnectionMessages.INTERNAL_SERVER_CONNECTION_ERROR))); + .orElse(ConnectionMessages.INTERNAL_SERVER_CONNECTION_ERROR)), status.isSafe()); break; default: // The only remaining value is successful (no need to do anything!) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java index 58068d697..998e01cc5 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialConnectSessionHandler.java @@ -1,7 +1,8 @@ package com.velocitypowered.proxy.connection.client; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; -import com.velocitypowered.proxy.protocol.MinecraftPacket; +import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; +import com.velocitypowered.proxy.protocol.packet.PluginMessage; public class InitialConnectSessionHandler implements MinecraftSessionHandler { @@ -12,8 +13,14 @@ public class InitialConnectSessionHandler implements MinecraftSessionHandler { } @Override - public void handleGeneric(MinecraftPacket packet) { - // No-op: will never handle packets + public boolean handle(PluginMessage packet) { + VelocityServerConnection serverConn = player.getConnectionInFlight(); + if (serverConn != null) { + if (!player.getPhase().handle(player, packet, serverConn)) { + serverConn.ensureConnected().write(packet); + } + } + return true; } @Override diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeHandshakeClientPhase.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeHandshakeClientPhase.java index 3ba7859a4..fd4577181 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeHandshakeClientPhase.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeHandshakeClientPhase.java @@ -38,9 +38,8 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { @Override boolean onHandle(ConnectedPlayer player, - ClientPlaySessionHandler handler, - PluginMessage message, - MinecraftConnection backendConn) { + PluginMessage message, + MinecraftConnection backendConn) { // If we stay in this phase, we do nothing because it means the packet wasn't handled. // Returning false indicates this return false; @@ -73,9 +72,8 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { @Override boolean onHandle(ConnectedPlayer player, - ClientPlaySessionHandler handler, - PluginMessage message, - MinecraftConnection backendConn) { + PluginMessage message, + MinecraftConnection backendConn) { // Read the mod list if we haven't already. if (!player.getModInfo().isPresent()) { List mods = LegacyForgeUtil.readModList(message); @@ -84,7 +82,7 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { } } - return super.onHandle(player, handler, message, backendConn); + return super.onHandle(player, message, backendConn); } }, @@ -148,14 +146,16 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { @Override boolean onHandle(ConnectedPlayer player, - ClientPlaySessionHandler handler, - PluginMessage message, - MinecraftConnection backendConn) { - super.onHandle(player, handler, message, backendConn); + PluginMessage message, + MinecraftConnection backendConn) { + super.onHandle(player, message, backendConn); // just in case the timing is awful player.sendKeepAlive(); - handler.flushQueuedMessages(); + + if (backendConn.getSessionHandler() instanceof ClientPlaySessionHandler) { + ((ClientPlaySessionHandler) backendConn.getSessionHandler()).flushQueuedMessages(); + } return true; } @@ -178,11 +178,10 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { @Override public final boolean handle(ConnectedPlayer player, - ClientPlaySessionHandler handler, - PluginMessage message) { - VelocityServerConnection serverConn = player.getConnectedServer(); - if (serverConn != null) { - MinecraftConnection backendConn = serverConn.getConnection(); + PluginMessage message, + VelocityServerConnection server) { + if (server != null) { + MinecraftConnection backendConn = server.getConnection(); if (backendConn != null && message.getChannel().equals(LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) { // Get the phase and check if we need to start the next phase. @@ -192,7 +191,7 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { player.setPhase(newPhase); // Perform phase handling - return newPhase.onHandle(player, handler, message, backendConn); + return newPhase.onHandle(player, message, backendConn); } } @@ -204,17 +203,14 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase { * Handles the phase tasks. * * @param player The player - * @param handler The {@link ClientPlaySessionHandler} that is handling - * packets * @param message The message to handle * @param backendConn The backend connection to write to, if required. * * @return true if handled, false otherwise. */ boolean onHandle(ConnectedPlayer player, - ClientPlaySessionHandler handler, - PluginMessage message, - MinecraftConnection backendConn) { + PluginMessage message, + MinecraftConnection backendConn) { // Send the packet on to the server. backendConn.write(message); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionRequestResults.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionRequestResults.java index d6768b8c2..60bdcc995 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionRequestResults.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionRequestResults.java @@ -6,6 +6,7 @@ import com.velocitypowered.api.proxy.ConnectionRequestBuilder.Status; import com.velocitypowered.api.proxy.server.RegisteredServer; import com.velocitypowered.proxy.protocol.packet.Disconnect; import java.util.Optional; +import javax.annotation.Nullable; import net.kyori.text.Component; import net.kyori.text.serializer.ComponentSerializers; @@ -15,7 +16,7 @@ public class ConnectionRequestResults { throw new AssertionError(); } - public static Result successful(RegisteredServer server) { + public static Impl successful(RegisteredServer server) { return plainResult(Status.SUCCESS, server); } @@ -25,31 +26,10 @@ public class ConnectionRequestResults { * @param server the server to use * @return the result */ - public static ConnectionRequestBuilder.Result plainResult( + public static Impl plainResult( ConnectionRequestBuilder.Status status, RegisteredServer server) { - return new ConnectionRequestBuilder.Result() { - @Override - public ConnectionRequestBuilder.Status getStatus() { - return status; - } - - @Override - public Optional getReason() { - return Optional.empty(); - } - - @Override - public RegisteredServer getAttemptedConnection() { - return server; - } - }; - } - - public static ConnectionRequestBuilder.Result forDisconnect(Disconnect disconnect, - RegisteredServer server) { - Component deserialized = ComponentSerializers.JSON.deserialize(disconnect.getReason()); - return forDisconnect(deserialized, server); + return new Impl(status, null, server, true); } /** @@ -58,23 +38,56 @@ public class ConnectionRequestResults { * @param server the server to use * @return the result */ - public static ConnectionRequestBuilder.Result forDisconnect(Component component, - RegisteredServer server) { - return new ConnectionRequestBuilder.Result() { - @Override - public ConnectionRequestBuilder.Status getStatus() { - return ConnectionRequestBuilder.Status.SERVER_DISCONNECTED; - } + public static Impl forDisconnect(Component component, RegisteredServer server) { + return new Impl(Status.SERVER_DISCONNECTED, component, server, true); + } - @Override - public Optional getReason() { - return Optional.of(component); - } + public static Impl forDisconnect(Disconnect disconnect, RegisteredServer server) { + Component deserialized = ComponentSerializers.JSON.deserialize(disconnect.getReason()); + return forDisconnect(deserialized, server); + } - @Override - public RegisteredServer getAttemptedConnection() { - return server; - } - }; + public static Impl forUnsafeDisconnect(Disconnect disconnect, RegisteredServer server) { + Component deserialized = ComponentSerializers.JSON.deserialize(disconnect.getReason()); + return new Impl(Status.SERVER_DISCONNECTED, deserialized, server, false); + } + + public static class Impl implements ConnectionRequestBuilder.Result { + + private final Status status; + private final @Nullable Component component; + private final RegisteredServer attemptedConnection; + private final boolean safe; + + Impl(Status status, @Nullable Component component, + RegisteredServer attemptedConnection, boolean safe) { + this.status = status; + this.component = component; + this.attemptedConnection = attemptedConnection; + this.safe = safe; + } + + @Override + public Status getStatus() { + return status; + } + + @Override + public Optional getReason() { + return Optional.ofNullable(component); + } + + @Override + public RegisteredServer getAttemptedConnection() { + return attemptedConnection; + } + + /** + * Returns whether or not it is safe to attempt a reconnect. + * @return whether we can try to reconnect + */ + public boolean isSafe() { + return safe; + } } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java b/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java index da69a0571..fa66b533d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java @@ -85,40 +85,4 @@ public class NettyHttpClient { }); return reply; } - - private static class HostAndSsl { - private final InetSocketAddress address; - private final boolean ssl; - - private HostAndSsl(InetSocketAddress address, boolean ssl) { - this.address = address; - this.ssl = ssl; - } - - @Override - public String toString() { - return "HostAndSsl{" - + "address=" + address - + ", ssl=" + ssl - + '}'; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - HostAndSsl that = (HostAndSsl) o; - return ssl == that.ssl - && Objects.equals(address, that.address); - } - - @Override - public int hashCode() { - return Objects.hash(address, ssl); - } - } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaPluginLoader.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaPluginLoader.java index 9493a984c..65a1d5c61 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaPluginLoader.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaPluginLoader.java @@ -19,6 +19,7 @@ import java.io.BufferedInputStream; import java.io.InputStreamReader; import java.io.Reader; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.HashSet; @@ -92,7 +93,7 @@ public class JavaPluginLoader implements PluginLoader { JarEntry entry; while ((entry = in.getNextJarEntry()) != null) { if (entry.getName().equals("velocity-plugin.json")) { - try (Reader pluginInfoReader = new InputStreamReader(in)) { + try (Reader pluginInfoReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { return Optional.of(VelocityServer.GSON .fromJson(pluginInfoReader, SerializedPluginDescription.class)); } 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 bd7dbd76b..1944e9919 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/StateRegistry.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/StateRegistry.java @@ -361,7 +361,7 @@ public enum StateRegistry { } } - public static class PacketMapping { + public static final class PacketMapping { private final int id; private final ProtocolVersion protocolVersion; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java index 234f9b493..ca4c6a119 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java @@ -46,7 +46,7 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler 0x79, 0x65, 0x72, 0x5F, 0x00, 0x00}; // Contents to add into basic stat response. See ResponseWriter class below - private static final Set QUERY_BASIC_RESPONSE_CONTENTS = ImmutableSet.of( + private static final ImmutableSet QUERY_BASIC_RESPONSE_CONTENTS = ImmutableSet.of( "hostname", "gametype", "map", @@ -118,7 +118,7 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler queryResponse.writeByte(QUERY_TYPE_HANDSHAKE); queryResponse.writeInt(sessionId); writeString(queryResponse, Integer.toString(challengeToken)); - ctx.writeAndFlush(responsePacket); + ctx.writeAndFlush(responsePacket, ctx.voidPromise()); break; } @@ -169,7 +169,7 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler } // Send the response - ctx.writeAndFlush(responsePacket); + ctx.writeAndFlush(responsePacket, ctx.voidPromise()); }, ctx.channel().eventLoop()); break; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyDisconnect.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyDisconnect.java index 074f6ec28..5edf23f50 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyDisconnect.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyDisconnect.java @@ -74,7 +74,7 @@ public class LegacyDisconnect { */ public static LegacyDisconnect from(TextComponent component) { // We intentionally use the legacy serializers, because the old clients can't understand JSON. - @SuppressWarnings("deprecated") + @SuppressWarnings("deprecation") String serialized = ComponentSerializers.LEGACY.serialize(component); return new LegacyDisconnect(serialized); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PlayerListItem.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PlayerListItem.java index df5bffa5d..618c5e344 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PlayerListItem.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PlayerListItem.java @@ -175,8 +175,8 @@ public class PlayerListItem implements MinecraftPacket { return gameMode; } - public Item setGameMode(int gamemode) { - this.gameMode = gamemode; + public Item setGameMode(int gameMode) { + this.gameMode = gameMode; return this; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java index 70f4c4334..8ad1c8847 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/PluginMessage.java @@ -38,7 +38,7 @@ public class PluginMessage implements MinecraftPacket { public String toString() { return "PluginMessage{" + "channel='" + channel + '\'' - + ", data=" + ByteBufUtil.hexDump(data) + + ", data=" + + '}'; } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtilsTest.java b/proxy/src/test/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtilsTest.java index dda1c3cd5..99033c689 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtilsTest.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtilsTest.java @@ -36,7 +36,7 @@ class PluginDependencyUtilsTest { private static final PluginDescription CIRCULAR_DEPENDENCY_2 = testDescription("oval", ImmutableList.of(new PluginDependency("circle", "", false))); - private static final List EXPECTED = ImmutableList.of( + private static final ImmutableList EXPECTED = ImmutableList.of( NEVER_DEPENDED, NO_DEPENDENCY_1_EXAMPLE, MULTI_DEPENDENCY, diff --git a/settings.gradle b/settings.gradle index 525baa264..d0f3098b4 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,6 +6,4 @@ include( ) findProject(':api')?.name = 'velocity-api' findProject(':proxy')?.name = 'velocity-proxy' -findProject(':native')?.name = 'velocity-native' - -enableFeaturePreview('STABLE_PUBLISHING') \ No newline at end of file +findProject(':native')?.name = 'velocity-native' \ No newline at end of file