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 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) 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(); } 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 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 91d99ceea..8d2e4c895 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; @@ -151,6 +152,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/plugin/VelocityPluginManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java index b6ce18016..b1a0c3aaa 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,19 @@ 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.VelocityPluginDescription; import com.velocitypowered.proxy.plugin.loader.java.JavaPluginLoader; import com.velocitypowered.proxy.plugin.util.PluginDependencyUtils; import java.io.IOException; @@ -18,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; @@ -61,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); } @@ -73,38 +82,62 @@ 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; } } - // Actually create the plugin - PluginContainer pluginObject; + try { + 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 {}", candidate.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..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 @@ -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; /** @@ -9,7 +11,36 @@ import java.nio.file.Path; */ public interface PluginLoader { - PluginDescription loadPlugin(Path source) throws Exception; + PluginDescription loadPluginDescription(Path source) throws Exception; - PluginContainer createPlugin(PluginDescription plugin) throws Exception; + PluginDescription loadPlugin(PluginDescription source) 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..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 @@ -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; @@ -39,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()) { @@ -51,17 +50,28 @@ public class JavaPluginLoader implements PluginLoader { throw new InvalidPluginException("Plugin ID '" + pd.getId() + "' is invalid."); } - PluginClassLoader loader = new PluginClassLoader( - new URL[]{source.toUri().toURL()} - ); - loader.addToClassloaders(); - - Class mainClass = loader.loadClass(pd.getMain()); - return createDescription(pd, source, mainClass); + return createCandidateDescription(pd, source); } @Override - public PluginContainer createPlugin(PluginDescription description) throws Exception { + 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.getSource().get().toUri().toURL()} + ); + loader.addToClassloaders(); + + JavaVelocityPluginDescriptionCandidate candidate = + (JavaVelocityPluginDescriptionCandidate) source; + Class mainClass = loader.loadClass(candidate.getMainClass()); + return createDescription(candidate, mainClass); + } + + @Override + 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 +83,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) @@ -114,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(), @@ -131,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/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); } } 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/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java index 2cf96e84b..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 @@ -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); 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..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 @@ -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_1, + HAS_DEPENDENCY_3); + 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 ); } }