From 941800ce96a84108cab201d298735f4bec786493 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 30 Dec 2019 23:20:25 -0700 Subject: [PATCH 01/12] Add VelocityServer#shutdown(boolean, TextComponent) --- .../velocitypowered/proxy/VelocityServer.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 2f260cce6..ad80a9161 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -360,11 +360,12 @@ public class VelocityServer implements ProxyServer { } /** - * Shuts down the proxy. + * Shuts down the proxy, kicking players with the specified {@param reason}. * * @param explicitExit whether the user explicitly shut down the proxy + * @param reason message to kick online players with */ - public void shutdown(boolean explicitExit) { + public void shutdown(boolean explicitExit, TextComponent reason) { if (eventManager == null || pluginManager == null || cm == null || scheduler == null) { throw new AssertionError(); } @@ -382,7 +383,7 @@ public class VelocityServer implements ProxyServer { ImmutableList players = ImmutableList.copyOf(connectionsByUuid.values()); for (ConnectedPlayer player : players) { - player.disconnect(TextComponent.of("Proxy shutting down.")); + player.disconnect(reason); } try { @@ -425,6 +426,15 @@ public class VelocityServer implements ProxyServer { thread.start(); } + /** + * Calls {@link #shutdown(boolean, TextComponent)} with the default reason "Proxy shutting down." + * + * @param explicitExit whether the user explicitly shut down the proxy + */ + public void shutdown(boolean explicitExit) { + shutdown(explicitExit, TextComponent.of("Proxy shutting down.")); + } + public AsyncHttpClient getAsyncHttpClient() { return ensureInitialized(cm).getHttpClient(); } From 9f340347b6d08d6ccff4a329ffe0ef8eebe97ddd Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 30 Dec 2019 23:20:55 -0700 Subject: [PATCH 02/12] Allow users to specify a reason in the shutdown command --- .../velocitypowered/proxy/command/ShutdownCommand.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/ShutdownCommand.java b/proxy/src/main/java/com/velocitypowered/proxy/command/ShutdownCommand.java index ba94083c8..87f53b211 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/ShutdownCommand.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/ShutdownCommand.java @@ -5,6 +5,7 @@ import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.proxy.VelocityServer; import net.kyori.text.TextComponent; import net.kyori.text.format.TextColor; +import net.kyori.text.serializer.legacy.LegacyComponentSerializer; import org.checkerframework.checker.nullness.qual.NonNull; public class ShutdownCommand implements Command { @@ -17,7 +18,12 @@ public class ShutdownCommand implements Command { @Override public void execute(CommandSource source, String @NonNull [] args) { - server.shutdown(true); + if (args.length == 0) { + server.shutdown(true); + } else { + String reason = String.join(" ", args); + server.shutdown(true, LegacyComponentSerializer.legacy().deserialize(reason, '&')); + } } @Override From 6e41ce7f154f7cc8a02efea406aa3f08cef2bfa2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 21 Jan 2020 13:28:01 -0500 Subject: [PATCH 03/12] Minecraft 1.15.2 --- .../java/com/velocitypowered/api/network/ProtocolVersion.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/com/velocitypowered/api/network/ProtocolVersion.java b/api/src/main/java/com/velocitypowered/api/network/ProtocolVersion.java index 28f729748..ca4ad3f91 100644 --- a/api/src/main/java/com/velocitypowered/api/network/ProtocolVersion.java +++ b/api/src/main/java/com/velocitypowered/api/network/ProtocolVersion.java @@ -35,7 +35,8 @@ public enum ProtocolVersion { MINECRAFT_1_14_3(490, "1.14.3"), MINECRAFT_1_14_4(498, "1.14.4"), MINECRAFT_1_15(573, "1.15"), - MINECRAFT_1_15_1(575, "1.15.1"); + MINECRAFT_1_15_1(575, "1.15.1"), + MINECRAFT_1_15_2(578, "1.15.2"); private final int protocol; private final String name; From c7bac69290e3643fd6bc135f99aff10e7726c167 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 20 Jan 2020 17:37:21 -0500 Subject: [PATCH 04/12] Don't attempt to retain the buffer if it goes to a closed connection --- .../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 2072372f5..f68448d8c 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 @@ -256,7 +256,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } MinecraftConnection smc = serverConnection.getConnection(); - if (smc != null && serverConnection.getPhase().consideredComplete()) { + if (smc != null && !smc.isClosed() && serverConnection.getPhase().consideredComplete()) { smc.write(buf.retain()); } } From da8cee2260eea6c65c73033d5658227f46230488 Mon Sep 17 00:00:00 2001 From: alexstaeding Date: Fri, 7 Feb 2020 22:22:47 +0100 Subject: [PATCH 05/12] Make PluginContainer injectable (#272) --- .../proxy/plugin/VelocityPluginManager.java | 47 ++++++++++++++++--- .../proxy/plugin/loader/PluginLoader.java | 31 +++++++++++- .../loader/VelocityPluginContainer.java | 9 ++-- .../plugin/loader/java/JavaPluginLoader.java | 29 ++++++++---- .../loader/java/VelocityPluginModule.java | 13 ++--- 5 files changed, 102 insertions(+), 27 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java index b6ce18016..f6e6712c4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java @@ -4,11 +4,18 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.Joiner; +import com.google.inject.AbstractModule; +import com.google.inject.Module; +import com.google.inject.name.Names; +import com.velocitypowered.api.command.CommandManager; +import com.velocitypowered.api.event.EventManager; import com.velocitypowered.api.plugin.PluginContainer; import com.velocitypowered.api.plugin.PluginDescription; import com.velocitypowered.api.plugin.PluginManager; import com.velocitypowered.api.plugin.meta.PluginDependency; +import com.velocitypowered.api.proxy.ProxyServer; import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.plugin.loader.VelocityPluginContainer; import com.velocitypowered.proxy.plugin.loader.java.JavaPluginLoader; import com.velocitypowered.proxy.plugin.util.PluginDependencyUtils; import java.io.IOException; @@ -80,6 +87,7 @@ public class VelocityPluginManager implements PluginManager { found.sort(Comparator.comparing(PluginDescription::getId)); List sortedPlugins = PluginDependencyUtils.sortCandidates(found); + Map pluginContainers = new HashMap<>(); // Now load the plugins pluginLoad: for (PluginDescription plugin : sortedPlugins) { @@ -92,19 +100,44 @@ public class VelocityPluginManager implements PluginManager { } } - // Actually create the plugin - PluginContainer pluginObject; + try { + VelocityPluginContainer container = new VelocityPluginContainer(plugin); + pluginContainers.put(container, loader.createModule(container)); + } catch (Exception e) { + logger.error("Can't create module for plugin {}", plugin.getId(), e); + } + } + + // Make a global Guice module that with common bindings for every plugin + AbstractModule commonModule = new AbstractModule() { + @Override + protected void configure() { + bind(ProxyServer.class).toInstance(server); + bind(PluginManager.class).toInstance(server.getPluginManager()); + bind(EventManager.class).toInstance(server.getEventManager()); + bind(CommandManager.class).toInstance(server.getCommandManager()); + for (PluginContainer container : pluginContainers.keySet()) { + bind(PluginContainer.class) + .annotatedWith(Names.named(container.getDescription().getId())) + .toInstance(container); + } + } + }; + + for (Map.Entry plugin : pluginContainers.entrySet()) { + PluginContainer container = plugin.getKey(); + PluginDescription description = container.getDescription(); try { - pluginObject = loader.createPlugin(plugin); + loader.createPlugin(container, plugin.getValue(), commonModule); } catch (Exception e) { - logger.error("Can't create plugin {}", plugin.getId(), e); + logger.error("Can't create plugin {}", description.getId(), e); continue; } - logger.info("Loaded plugin {} {} by {}", plugin.getId(), plugin.getVersion() - .orElse(""), Joiner.on(", ").join(plugin.getAuthors())); - registerPlugin(pluginObject); + logger.info("Loaded plugin {} {} by {}", description.getId(), description.getVersion() + .orElse(""), Joiner.on(", ").join(description.getAuthors())); + registerPlugin(container); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java index 743a17067..e261ee586 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java @@ -1,7 +1,9 @@ package com.velocitypowered.proxy.plugin.loader; +import com.google.inject.Module; import com.velocitypowered.api.plugin.PluginContainer; import com.velocitypowered.api.plugin.PluginDescription; + import java.nio.file.Path; /** @@ -11,5 +13,32 @@ public interface PluginLoader { PluginDescription loadPlugin(Path source) throws Exception; - PluginContainer createPlugin(PluginDescription plugin) throws Exception; + /** + * Creates a {@link Module} for the provided {@link PluginContainer} + * and verifies that the container's {@link PluginDescription} is correct. + * + *

Does not create an instance of the plugin.

+ * + * @param container the plugin container + * @return the module containing bindings specific to this plugin + * @throws IllegalArgumentException If the {@link PluginDescription} + * is missing the path + */ + Module createModule(PluginContainer container) throws Exception; + + /** + * Creates an instance of the plugin as specified by the + * plugin's main class found in the {@link PluginDescription}. + * + *

The provided {@link Module modules} are used to create an + * injector which is then used to create the plugin instance.

+ * + *

The plugin instance is set in the provided {@link PluginContainer}.

+ * + * @param container the plugin container + * @param modules the modules to be used when creating this plugin's injector + * @throws IllegalStateException If the plugin instance could not be + * created from the provided modules + */ + void createPlugin(PluginContainer container, Module... modules) throws Exception; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/VelocityPluginContainer.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/VelocityPluginContainer.java index c7975c55b..1a20bddc1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/VelocityPluginContainer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/VelocityPluginContainer.java @@ -7,11 +7,10 @@ import java.util.Optional; public class VelocityPluginContainer implements PluginContainer { private final PluginDescription description; - private final Object instance; + private Object instance; - public VelocityPluginContainer(PluginDescription description, Object instance) { + public VelocityPluginContainer(PluginDescription description) { this.description = description; - this.instance = instance; } @Override @@ -23,4 +22,8 @@ public class VelocityPluginContainer implements PluginContainer { public Optional getInstance() { return Optional.ofNullable(instance); } + + public void setInstance(Object instance) { + this.instance = instance; + } } 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 65a1d5c61..a87846006 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 @@ -2,6 +2,7 @@ package com.velocitypowered.proxy.plugin.loader.java; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.Module; import com.velocitypowered.api.plugin.InvalidPluginException; import com.velocitypowered.api.plugin.PluginContainer; import com.velocitypowered.api.plugin.PluginDescription; @@ -13,8 +14,6 @@ import com.velocitypowered.proxy.plugin.PluginClassLoader; import com.velocitypowered.proxy.plugin.loader.PluginLoader; import com.velocitypowered.proxy.plugin.loader.VelocityPluginContainer; import com.velocitypowered.proxy.plugin.loader.VelocityPluginDescription; -import com.velocitypowered.proxy.plugin.loader.java.JavaVelocityPluginDescription; -import com.velocitypowered.proxy.plugin.loader.java.VelocityPluginModule; import java.io.BufferedInputStream; import java.io.InputStreamReader; import java.io.Reader; @@ -61,7 +60,8 @@ public class JavaPluginLoader implements PluginLoader { } @Override - public PluginContainer createPlugin(PluginDescription description) throws Exception { + public Module createModule(PluginContainer container) throws Exception { + PluginDescription description = container.getDescription(); if (!(description instanceof JavaVelocityPluginDescription)) { throw new IllegalArgumentException("Description provided isn't of the Java plugin loader"); } @@ -73,16 +73,29 @@ public class JavaPluginLoader implements PluginLoader { throw new IllegalArgumentException("No path in plugin description"); } - Injector injector = Guice - .createInjector(new VelocityPluginModule(server, javaDescription, baseDirectory)); - Object instance = injector.getInstance(javaDescription.getMainClass()); + return new VelocityPluginModule(server, javaDescription, container, baseDirectory); + } + + @Override + public void createPlugin(PluginContainer container, Module... modules) { + if (!(container instanceof VelocityPluginContainer)) { + throw new IllegalArgumentException("Container provided isn't of the Java plugin loader"); + } + PluginDescription description = container.getDescription(); + if (!(description instanceof JavaVelocityPluginDescription)) { + throw new IllegalArgumentException("Description provided isn't of the Java plugin loader"); + } + + Injector injector = Guice.createInjector(modules); + Object instance = injector + .getInstance(((JavaVelocityPluginDescription) description).getMainClass()); if (instance == null) { throw new IllegalStateException( - "Got nothing from injector for plugin " + javaDescription.getId()); + "Got nothing from injector for plugin " + description.getId()); } - return new VelocityPluginContainer(description, instance); + ((VelocityPluginContainer) container).setInstance(instance); } private Optional getSerializedPluginInfo(Path source) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/VelocityPluginModule.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/VelocityPluginModule.java index 897e37130..77e944ac8 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/VelocityPluginModule.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/VelocityPluginModule.java @@ -3,10 +3,8 @@ package com.velocitypowered.proxy.plugin.loader.java; import com.google.inject.Binder; import com.google.inject.Module; import com.google.inject.Scopes; -import com.velocitypowered.api.command.CommandManager; -import com.velocitypowered.api.event.EventManager; +import com.velocitypowered.api.plugin.PluginContainer; import com.velocitypowered.api.plugin.PluginDescription; -import com.velocitypowered.api.plugin.PluginManager; import com.velocitypowered.api.plugin.annotation.DataDirectory; import com.velocitypowered.api.proxy.ProxyServer; import java.nio.file.Path; @@ -17,12 +15,14 @@ class VelocityPluginModule implements Module { private final ProxyServer server; private final JavaVelocityPluginDescription description; + private final PluginContainer pluginContainer; private final Path basePluginPath; VelocityPluginModule(ProxyServer server, JavaVelocityPluginDescription description, - Path basePluginPath) { + PluginContainer pluginContainer, Path basePluginPath) { this.server = server; this.description = description; + this.pluginContainer = pluginContainer; this.basePluginPath = basePluginPath; } @@ -31,12 +31,9 @@ class VelocityPluginModule implements Module { binder.bind(description.getMainClass()).in(Scopes.SINGLETON); binder.bind(Logger.class).toInstance(LoggerFactory.getLogger(description.getId())); - binder.bind(ProxyServer.class).toInstance(server); binder.bind(Path.class).annotatedWith(DataDirectory.class) .toInstance(basePluginPath.resolve(description.getId())); binder.bind(PluginDescription.class).toInstance(description); - binder.bind(PluginManager.class).toInstance(server.getPluginManager()); - binder.bind(EventManager.class).toInstance(server.getEventManager()); - binder.bind(CommandManager.class).toInstance(server.getCommandManager()); + binder.bind(PluginContainer.class).toInstance(pluginContainer); } } From e558b7ca1f19eab2e01dcbc47894c1e1bc44e3dc Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 13 Feb 2020 12:57:48 -0500 Subject: [PATCH 06/12] Update to new bStats revision --- .../java/com/velocitypowered/proxy/Metrics.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java b/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java index 402746171..2ac4f983c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java @@ -35,7 +35,7 @@ import org.asynchttpclient.Response; public class Metrics { // The version of this bStats class - private static final int B_STATS_VERSION = 1; + private static final int B_STATS_METRICS_REVISION = 2; // The url to which the data is sent private static final String URL = "https://bstats.org/submitData/server-implementation"; @@ -49,6 +49,9 @@ public class Metrics { // The name of the server software private final String name; + // The plugin ID for the server software as assigned by bStats. + private final int pluginId; + // The uuid of the server private final String serverUuid; @@ -60,13 +63,15 @@ public class Metrics { /** * Class constructor. * @param name The name of the server software. + * @param pluginId The plugin ID for the server software as assigned by bStats. * @param serverUuid The uuid of the server. * @param logFailedRequests Whether failed requests should be logged or not. * @param server The Velocity server instance. */ - private Metrics(String name, String serverUuid, boolean logFailedRequests, + private Metrics(String name, int pluginId, String serverUuid, boolean logFailedRequests, VelocityServer server) { this.name = name; + this.pluginId = pluginId; this.serverUuid = serverUuid; Metrics.logFailedRequests = logFailedRequests; this.server = server; @@ -114,6 +119,8 @@ public class Metrics { JsonObject data = new JsonObject(); data.addProperty("pluginName", name); // Append the name of the server software + data.addProperty("id", pluginId); + data.addProperty("metricsRevision", B_STATS_METRICS_REVISION); JsonArray customCharts = new JsonArray(); for (CustomChart customChart : charts) { // Add the data of the custom charts @@ -573,7 +580,7 @@ public class Metrics { boolean logFailedRequests = metricsConfig.isLogFailure(); // Only start Metrics, if it's enabled in the config if (metricsConfig.isEnabled()) { - Metrics metrics = new Metrics("Velocity", serverUuid, logFailedRequests, server); + Metrics metrics = new Metrics("Velocity", 4752, serverUuid, logFailedRequests, server); metrics.addCustomChart( new Metrics.SingleLineChart("players", server::getPlayerCount) From a5350c68226bb7b8bc282a2d10463230d188d675 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 13 Feb 2020 13:32:22 -0500 Subject: [PATCH 07/12] Deal with potentially nullable player sample entries --- .../java/com/velocitypowered/api/proxy/server/ServerPing.java | 4 ++-- 1 file changed, 2 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 db3ef1ed5..7049c3c30 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 @@ -114,7 +114,7 @@ public final class ServerPing { if (players != null) { builder.onlinePlayers = players.online; builder.maximumPlayers = players.max; - builder.samplePlayers.addAll(players.sample); + builder.samplePlayers.addAll(players.getSample()); } else { builder.nullOutPlayers = true; } @@ -367,7 +367,7 @@ public final class ServerPing { } public List getSample() { - return sample; + return sample == null ? ImmutableList.of() : sample; } @Override From 9444cfa70fcc7189f5c1f35d139fa76160282cc1 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 13 Feb 2020 19:20:53 -0500 Subject: [PATCH 08/12] Use more obvious/broken-down test cases for topological sort tests --- .../util/PluginDependencyUtilsTest.java | 95 +++++++++++-------- 1 file changed, 53 insertions(+), 42 deletions(-) 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 99033c689..5709b2864 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 @@ -14,49 +14,64 @@ import org.junit.jupiter.api.Test; class PluginDependencyUtilsTest { - private static final PluginDescription NO_DEPENDENCY_1_EXAMPLE = testDescription("example"); - private static final PluginDescription NEVER_DEPENDED = testDescription("and-again"); - private static final PluginDescription SOFT_DEPENDENCY_EXISTS = testDescription("soft", - ImmutableList.of(new PluginDependency("example", "", true))); - private static final PluginDescription SOFT_DEPENDENCY_DOES_NOT_EXIST = testDescription("fluffy", - ImmutableList.of(new PluginDependency("i-dont-exist", "", false))); - private static final PluginDescription MULTI_DEPENDENCY = testDescription("multi-depend", - ImmutableList.of( - new PluginDependency("example", "", false) - ) - ); - private static final PluginDescription TEST_WITH_DUPLICATE_DEPEND = testDescription("dup-depend", - ImmutableList.of( - new PluginDependency("multi-depend", "", false) - ) - ); + private static final PluginDescription NO_DEPENDENCY = testDescription("trivial"); + private static final PluginDescription NO_DEPENDENCY_2 = testDescription("trivial2"); + private static final PluginDescription HAS_DEPENDENCY_1 = testDescription("dependent1", + new PluginDependency("trivial", null, false)); + private static final PluginDescription HAS_DEPENDENCY_2 = testDescription("dependent2", + new PluginDependency("dependent1", null, false)); + private static final PluginDescription HAS_DEPENDENCY_3 = testDescription("dependent3", + new PluginDependency("trivial", null, false)); private static final PluginDescription CIRCULAR_DEPENDENCY_1 = testDescription("circle", - ImmutableList.of(new PluginDependency("oval", "", false))); + new PluginDependency("oval", "", false)); private static final PluginDescription CIRCULAR_DEPENDENCY_2 = testDescription("oval", - ImmutableList.of(new PluginDependency("circle", "", false))); - - private static final ImmutableList EXPECTED = ImmutableList.of( - NEVER_DEPENDED, - NO_DEPENDENCY_1_EXAMPLE, - MULTI_DEPENDENCY, - TEST_WITH_DUPLICATE_DEPEND, - SOFT_DEPENDENCY_DOES_NOT_EXIST, - SOFT_DEPENDENCY_EXISTS - ); + new PluginDependency("circle", "", false)); @Test - void sortCandidates() throws Exception { + void sortCandidatesTrivial() throws Exception { List descriptionList = new ArrayList<>(); - descriptionList.add(NO_DEPENDENCY_1_EXAMPLE); - descriptionList.add(NEVER_DEPENDED); - descriptionList.add(SOFT_DEPENDENCY_DOES_NOT_EXIST); - descriptionList.add(SOFT_DEPENDENCY_EXISTS); - descriptionList.add(MULTI_DEPENDENCY); - descriptionList.add(TEST_WITH_DUPLICATE_DEPEND); - descriptionList.sort(Comparator.comparing(PluginDescription::getId)); + assertEquals(descriptionList, PluginDependencyUtils.sortCandidates(descriptionList)); + } - assertEquals(EXPECTED, PluginDependencyUtils.sortCandidates(descriptionList)); + @Test + void sortCandidatesSingleton() throws Exception { + List plugins = ImmutableList.of(NO_DEPENDENCY); + assertEquals(plugins, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesBasicDependency() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_1, NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesNestedDependency() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_1, HAS_DEPENDENCY_2, + NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1, + HAS_DEPENDENCY_2); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesTypical() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_2, NO_DEPENDENCY_2, + HAS_DEPENDENCY_1, NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1, + HAS_DEPENDENCY_2, NO_DEPENDENCY_2); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesMultiplePluginsDependentOnOne() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_3, HAS_DEPENDENCY_1, + NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_3, + HAS_DEPENDENCY_1); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); } @Test @@ -65,14 +80,10 @@ class PluginDependencyUtilsTest { assertThrows(IllegalStateException.class, () -> PluginDependencyUtils.sortCandidates(descs)); } - private static PluginDescription testDescription(String id) { - return testDescription(id, ImmutableList.of()); - } - - private static PluginDescription testDescription(String id, List dependencies) { + private static PluginDescription testDescription(String id, PluginDependency... dependencies) { return new VelocityPluginDescription( id, "tuxed", "0.1", null, null, ImmutableList.of(), - dependencies, null + ImmutableList.copyOf(dependencies), null ); } } From 3b6f8e2421dbd93d4fa7d36c25221f7483dc0964 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 13 Feb 2020 19:20:53 -0500 Subject: [PATCH 09/12] Use more obvious/broken-down test cases for topological sort tests --- .../util/PluginDependencyUtilsTest.java | 95 +++++++++++-------- 1 file changed, 53 insertions(+), 42 deletions(-) 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 99033c689..5709b2864 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 @@ -14,49 +14,64 @@ import org.junit.jupiter.api.Test; class PluginDependencyUtilsTest { - private static final PluginDescription NO_DEPENDENCY_1_EXAMPLE = testDescription("example"); - private static final PluginDescription NEVER_DEPENDED = testDescription("and-again"); - private static final PluginDescription SOFT_DEPENDENCY_EXISTS = testDescription("soft", - ImmutableList.of(new PluginDependency("example", "", true))); - private static final PluginDescription SOFT_DEPENDENCY_DOES_NOT_EXIST = testDescription("fluffy", - ImmutableList.of(new PluginDependency("i-dont-exist", "", false))); - private static final PluginDescription MULTI_DEPENDENCY = testDescription("multi-depend", - ImmutableList.of( - new PluginDependency("example", "", false) - ) - ); - private static final PluginDescription TEST_WITH_DUPLICATE_DEPEND = testDescription("dup-depend", - ImmutableList.of( - new PluginDependency("multi-depend", "", false) - ) - ); + private static final PluginDescription NO_DEPENDENCY = testDescription("trivial"); + private static final PluginDescription NO_DEPENDENCY_2 = testDescription("trivial2"); + private static final PluginDescription HAS_DEPENDENCY_1 = testDescription("dependent1", + new PluginDependency("trivial", null, false)); + private static final PluginDescription HAS_DEPENDENCY_2 = testDescription("dependent2", + new PluginDependency("dependent1", null, false)); + private static final PluginDescription HAS_DEPENDENCY_3 = testDescription("dependent3", + new PluginDependency("trivial", null, false)); private static final PluginDescription CIRCULAR_DEPENDENCY_1 = testDescription("circle", - ImmutableList.of(new PluginDependency("oval", "", false))); + new PluginDependency("oval", "", false)); private static final PluginDescription CIRCULAR_DEPENDENCY_2 = testDescription("oval", - ImmutableList.of(new PluginDependency("circle", "", false))); - - private static final ImmutableList EXPECTED = ImmutableList.of( - NEVER_DEPENDED, - NO_DEPENDENCY_1_EXAMPLE, - MULTI_DEPENDENCY, - TEST_WITH_DUPLICATE_DEPEND, - SOFT_DEPENDENCY_DOES_NOT_EXIST, - SOFT_DEPENDENCY_EXISTS - ); + new PluginDependency("circle", "", false)); @Test - void sortCandidates() throws Exception { + void sortCandidatesTrivial() throws Exception { List descriptionList = new ArrayList<>(); - descriptionList.add(NO_DEPENDENCY_1_EXAMPLE); - descriptionList.add(NEVER_DEPENDED); - descriptionList.add(SOFT_DEPENDENCY_DOES_NOT_EXIST); - descriptionList.add(SOFT_DEPENDENCY_EXISTS); - descriptionList.add(MULTI_DEPENDENCY); - descriptionList.add(TEST_WITH_DUPLICATE_DEPEND); - descriptionList.sort(Comparator.comparing(PluginDescription::getId)); + assertEquals(descriptionList, PluginDependencyUtils.sortCandidates(descriptionList)); + } - assertEquals(EXPECTED, PluginDependencyUtils.sortCandidates(descriptionList)); + @Test + void sortCandidatesSingleton() throws Exception { + List plugins = ImmutableList.of(NO_DEPENDENCY); + assertEquals(plugins, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesBasicDependency() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_1, NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesNestedDependency() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_1, HAS_DEPENDENCY_2, + NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1, + HAS_DEPENDENCY_2); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesTypical() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_2, NO_DEPENDENCY_2, + HAS_DEPENDENCY_1, NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1, + HAS_DEPENDENCY_2, NO_DEPENDENCY_2); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); + } + + @Test + void sortCandidatesMultiplePluginsDependentOnOne() throws Exception { + List plugins = ImmutableList.of(HAS_DEPENDENCY_3, HAS_DEPENDENCY_1, + NO_DEPENDENCY); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_3, + HAS_DEPENDENCY_1); + assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); } @Test @@ -65,14 +80,10 @@ class PluginDependencyUtilsTest { assertThrows(IllegalStateException.class, () -> PluginDependencyUtils.sortCandidates(descs)); } - private static PluginDescription testDescription(String id) { - return testDescription(id, ImmutableList.of()); - } - - private static PluginDescription testDescription(String id, List dependencies) { + private static PluginDescription testDescription(String id, PluginDependency... dependencies) { return new VelocityPluginDescription( id, "tuxed", "0.1", null, null, ImmutableList.of(), - dependencies, null + ImmutableList.copyOf(dependencies), null ); } } From 37994449d76c6d9fcafa479bc84c673b91009e3d Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 16 Feb 2020 03:19:13 -0500 Subject: [PATCH 10/12] Fix regressions with plugin dependency loading from #272 This is a quick and dirty fix because it's late. I'll need to commit a better fix later. --- .../proxy/plugin/VelocityPluginManager.java | 26 ++++++------ .../proxy/plugin/loader/PluginLoader.java | 4 +- .../plugin/loader/java/JavaPluginLoader.java | 41 +++++++++++++++---- ...avaVelocityPluginDescriptionCandidate.java | 27 ++++++++++++ .../plugin/util/PluginDependencyUtils.java | 10 +++-- .../util/PluginDependencyUtilsTest.java | 4 +- 6 files changed, 86 insertions(+), 26 deletions(-) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaVelocityPluginDescriptionCandidate.java diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java index f6e6712c4..b1a0c3aaa 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java @@ -16,6 +16,7 @@ import com.velocitypowered.api.plugin.meta.PluginDependency; import com.velocitypowered.api.proxy.ProxyServer; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.plugin.loader.VelocityPluginContainer; +import com.velocitypowered.proxy.plugin.loader.VelocityPluginDescription; import com.velocitypowered.proxy.plugin.loader.java.JavaPluginLoader; import com.velocitypowered.proxy.plugin.util.PluginDependencyUtils; import java.io.IOException; @@ -25,12 +26,13 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -68,7 +70,7 @@ public class VelocityPluginManager implements PluginManager { .newDirectoryStream(directory, p -> p.toFile().isFile() && p.toString().endsWith(".jar"))) { for (Path path : stream) { try { - found.add(loader.loadPlugin(path)); + found.add(loader.loadPluginDescription(path)); } catch (Exception e) { logger.error("Unable to load plugin {}", path, e); } @@ -80,31 +82,29 @@ public class VelocityPluginManager implements PluginManager { return; } - // Sort the loaded plugins twice. First, sort the already-loaded plugins by their IDs, so as - // to make the topographic sort deterministic (since the order will differ depending on the - // first node chosen in the graph, which is the first plugin we found). Afterwards, we execute - // a depth-first search over the loaded plugins. - found.sort(Comparator.comparing(PluginDescription::getId)); List sortedPlugins = PluginDependencyUtils.sortCandidates(found); + Set loadedPluginsById = new HashSet<>(); Map pluginContainers = new HashMap<>(); // Now load the plugins pluginLoad: - for (PluginDescription plugin : sortedPlugins) { + for (PluginDescription candidate : sortedPlugins) { // Verify dependencies - for (PluginDependency dependency : plugin.getDependencies()) { - if (!dependency.isOptional() && !isLoaded(dependency.getId())) { - logger.error("Can't load plugin {} due to missing dependency {}", plugin.getId(), + for (PluginDependency dependency : candidate.getDependencies()) { + if (!dependency.isOptional() && !loadedPluginsById.contains(dependency.getId())) { + logger.error("Can't load plugin {} due to missing dependency {}", candidate.getId(), dependency.getId()); continue pluginLoad; } } try { - VelocityPluginContainer container = new VelocityPluginContainer(plugin); + PluginDescription realPlugin = loader.loadPlugin(candidate); + VelocityPluginContainer container = new VelocityPluginContainer(realPlugin); pluginContainers.put(container, loader.createModule(container)); + loadedPluginsById.add(realPlugin.getId()); } catch (Exception e) { - logger.error("Can't create module for plugin {}", plugin.getId(), e); + logger.error("Can't create module for plugin {}", candidate.getId(), e); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java index e261ee586..3ffdeb4b4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java @@ -11,7 +11,9 @@ import java.nio.file.Path; */ public interface PluginLoader { - PluginDescription loadPlugin(Path source) throws Exception; + PluginDescription loadPluginDescription(Path source) throws Exception; + + PluginDescription loadPlugin(PluginDescription source) throws Exception; /** * Creates a {@link Module} for the provided {@link PluginContainer} 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 a87846006..17a075c23 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 @@ -38,7 +38,7 @@ public class JavaPluginLoader implements PluginLoader { } @Override - public PluginDescription loadPlugin(Path source) throws Exception { + public PluginDescription loadPluginDescription(Path source) throws Exception { Optional serialized = getSerializedPluginInfo(source); if (!serialized.isPresent()) { @@ -50,13 +50,23 @@ public class JavaPluginLoader implements PluginLoader { throw new InvalidPluginException("Plugin ID '" + pd.getId() + "' is invalid."); } + return createCandidateDescription(pd, source); + } + + @Override + public PluginDescription loadPlugin(PluginDescription source) throws Exception { + if (!(source instanceof JavaVelocityPluginDescriptionCandidate)) { + throw new IllegalArgumentException("Description provided isn't of the Java plugin loader"); + } PluginClassLoader loader = new PluginClassLoader( - new URL[]{source.toUri().toURL()} + new URL[]{source.getSource().get().toUri().toURL()} ); loader.addToClassloaders(); - Class mainClass = loader.loadClass(pd.getMain()); - return createDescription(pd, source, mainClass); + JavaVelocityPluginDescriptionCandidate candidate = + (JavaVelocityPluginDescriptionCandidate) source; + Class mainClass = loader.loadClass(candidate.getMainClass()); + return createDescription(candidate, mainClass); } @Override @@ -127,15 +137,16 @@ public class JavaPluginLoader implements PluginLoader { } } - private VelocityPluginDescription createDescription(SerializedPluginDescription description, - Path source, Class mainClass) { + private VelocityPluginDescription createCandidateDescription( + SerializedPluginDescription description, + Path source) { Set dependencies = new HashSet<>(); for (SerializedPluginDescription.Dependency dependency : description.getDependencies()) { dependencies.add(toDependencyMeta(dependency)); } - return new JavaVelocityPluginDescription( + return new JavaVelocityPluginDescriptionCandidate( description.getId(), description.getName(), description.getVersion(), @@ -144,6 +155,22 @@ public class JavaPluginLoader implements PluginLoader { description.getAuthors(), dependencies, source, + description.getMain() + ); + } + + private VelocityPluginDescription createDescription( + JavaVelocityPluginDescriptionCandidate description, + Class mainClass) { + return new JavaVelocityPluginDescription( + description.getId(), + description.getName().orElse(null), + description.getVersion().orElse(null), + description.getDescription().orElse(null), + description.getUrl().orElse(null), + description.getAuthors(), + description.getDependencies(), + description.getSource().orElse(null), mainClass ); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaVelocityPluginDescriptionCandidate.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaVelocityPluginDescriptionCandidate.java new file mode 100644 index 000000000..a5d6d0a1a --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaVelocityPluginDescriptionCandidate.java @@ -0,0 +1,27 @@ +package com.velocitypowered.proxy.plugin.loader.java; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.velocitypowered.api.plugin.meta.PluginDependency; +import com.velocitypowered.proxy.plugin.loader.VelocityPluginDescription; +import java.nio.file.Path; +import java.util.Collection; +import java.util.List; +import org.checkerframework.checker.nullness.qual.Nullable; + +class JavaVelocityPluginDescriptionCandidate extends VelocityPluginDescription { + + private final String mainClass; + + JavaVelocityPluginDescriptionCandidate(String id, @Nullable String name, @Nullable String version, + @Nullable String description, @Nullable String url, + @Nullable List authors, Collection dependencies, Path source, + String mainClass) { + super(id, name, version, description, url, authors, dependencies, source); + this.mainClass = checkNotNull(mainClass); + } + + String getMainClass() { + return mainClass; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtils.java index 8bf4d1ff8..e09cc5bec 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtils.java @@ -8,6 +8,7 @@ import com.velocitypowered.api.plugin.PluginDescription; import com.velocitypowered.api.plugin.meta.PluginDependency; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Comparator; import java.util.Deque; import java.util.HashMap; import java.util.List; @@ -28,17 +29,20 @@ public class PluginDependencyUtils { * @throws IllegalStateException if there is a circular loop in the dependency graph */ public static List sortCandidates(List candidates) { + List sortedCandidates = new ArrayList<>(candidates); + sortedCandidates.sort(Comparator.comparing(PluginDescription::getId)); + // Create a graph and populate it with plugin dependencies. Specifically, each graph has plugin // nodes, and edges that represent the dependencies that plugin relies on. Non-existent plugins // are ignored. MutableGraph graph = GraphBuilder.directed() .allowsSelfLoops(false) - .expectedNodeCount(candidates.size()) + .expectedNodeCount(sortedCandidates.size()) .build(); - Map candidateMap = Maps.uniqueIndex(candidates, + Map candidateMap = Maps.uniqueIndex(sortedCandidates, PluginDescription::getId); - for (PluginDescription description : candidates) { + for (PluginDescription description : sortedCandidates) { graph.addNode(description); for (PluginDependency dependency : description.getDependencies()) { 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 5709b2864..29fc31e7e 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 @@ -69,8 +69,8 @@ class PluginDependencyUtilsTest { void sortCandidatesMultiplePluginsDependentOnOne() throws Exception { List plugins = ImmutableList.of(HAS_DEPENDENCY_3, HAS_DEPENDENCY_1, NO_DEPENDENCY); - List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_3, - HAS_DEPENDENCY_1); + List expected = ImmutableList.of(NO_DEPENDENCY, HAS_DEPENDENCY_1, + HAS_DEPENDENCY_3); assertEquals(expected, PluginDependencyUtils.sortCandidates(plugins)); } From 8ae7945b9f41ab98ab42f0678634dd9d3aaa6aa4 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 17 Feb 2020 19:34:22 -0500 Subject: [PATCH 11/12] Upon connection exception, discard all incoming packets instead --- .../proxy/connection/MinecraftConnection.java | 2 ++ .../proxy/network/netty/DiscardHandler.java | 17 ++++++++++ .../protocol/netty/MinecraftDecoder.java | 34 ++++++++++++++++--- 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/network/netty/DiscardHandler.java 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 c1f15cc19..d36846cc7 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -16,6 +16,7 @@ import com.velocitypowered.natives.encryption.VelocityCipher; import com.velocitypowered.natives.encryption.VelocityCipherFactory; import com.velocitypowered.natives.util.Natives; import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.network.netty.DiscardHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.StateRegistry; import com.velocitypowered.proxy.protocol.netty.MinecraftCipherDecoder; @@ -144,6 +145,7 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { } } + ctx.pipeline().addBefore(MINECRAFT_DECODER, "discard", DiscardHandler.HANDLER); ctx.close(); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/netty/DiscardHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/network/netty/DiscardHandler.java new file mode 100644 index 000000000..80b34056e --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/netty/DiscardHandler.java @@ -0,0 +1,17 @@ +package com.velocitypowered.proxy.network.netty; + +import io.netty.channel.ChannelHandler.Sharable; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.util.ReferenceCountUtil; + +@Sharable +public class DiscardHandler extends ChannelInboundHandlerAdapter { + + public static final DiscardHandler HANDLER = new DiscardHandler(); + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + ReferenceCountUtil.release(msg); + } +} 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 2cf96e84b..98e06f6ab 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 @@ -2,17 +2,26 @@ package com.velocitypowered.proxy.protocol.netty; import com.google.common.base.Preconditions; import com.velocitypowered.api.network.ProtocolVersion; +import com.velocitypowered.proxy.network.Connections; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.StateRegistry; +import com.velocitypowered.proxy.util.except.QuietException; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.CorruptedFrameException; import io.netty.handler.codec.MessageToMessageDecoder; +import io.netty.util.ReferenceCountUtil; import java.util.List; public class MinecraftDecoder extends MessageToMessageDecoder { + private static final boolean DEBUG = Boolean.getBoolean("velocity.packet-decode-logging"); + private static final QuietException DECODE_FAILED = + new QuietException("A packet did not decode successfully (invalid data). If you are a " + + "developer, launch Velocity with -Dvelocity.packet-decode-logging=true to see more."); + private final ProtocolUtils.Direction direction; private StateRegistry state; private StateRegistry.PacketRegistry.ProtocolRegistry registry; @@ -46,17 +55,34 @@ public class MinecraftDecoder extends MessageToMessageDecoder { try { packet.decode(msg, direction, registry.version); } catch (Exception e) { - throw new CorruptedFrameException( - "Error decoding " + packet.getClass() + " " + getExtraConnectionDetail(packetId), e); + throw handleDecodeFailure(e, packet, packetId); } + if (msg.isReadable()) { - throw new CorruptedFrameException("Did not read full packet for " + packet.getClass() + " " - + getExtraConnectionDetail(packetId)); + throw handleNotReadEnough(packet, packetId); } out.add(packet); } } + private Exception handleNotReadEnough(MinecraftPacket packet, int packetId) { + if (DEBUG) { + return new CorruptedFrameException("Did not read full packet for " + packet.getClass() + " " + + getExtraConnectionDetail(packetId)); + } else { + return DECODE_FAILED; + } + } + + private Exception handleDecodeFailure(Exception cause, MinecraftPacket packet, int packetId) { + if (DEBUG) { + return new CorruptedFrameException( + "Error decoding " + packet.getClass() + " " + getExtraConnectionDetail(packetId), cause); + } else { + return DECODE_FAILED; + } + } + private String getExtraConnectionDetail(int packetId) { return "Direction " + direction + " Protocol " + registry.version + " State " + state + " ID " + Integer.toHexString(packetId); From c63bd4cd02365b5edec6556f01bd5e1e0d193a32 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 17 Feb 2020 20:35:28 -0500 Subject: [PATCH 12/12] Fix checkstyle error --- .../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 98e06f6ab..55da0dd78 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 @@ -19,8 +19,8 @@ public class MinecraftDecoder extends MessageToMessageDecoder { private static final boolean DEBUG = Boolean.getBoolean("velocity.packet-decode-logging"); private static final QuietException DECODE_FAILED = - new QuietException("A packet did not decode successfully (invalid data). If you are a " + - "developer, launch Velocity with -Dvelocity.packet-decode-logging=true to see more."); + new QuietException("A packet did not decode successfully (invalid data). If you are a " + + "developer, launch Velocity with -Dvelocity.packet-decode-logging=true to see more."); private final ProtocolUtils.Direction direction; private StateRegistry state;