From 783054d098d49df713499c55aaa68de9ac53ee13 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 22 Nov 2020 12:00:47 -0500 Subject: [PATCH 01/69] Velocity 1.1.2 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 6d4f7e8d7..d38d21bbd 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ allprojects { apply plugin: "com.github.spotbugs" group 'com.velocitypowered' - version '1.1.2-SNAPSHOT' + version '1.1.2' ext { // dependency versions From dd8c670ef7d81ec0c18ccc03f1dbeb5b80b5931b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 22 Nov 2020 12:04:07 -0500 Subject: [PATCH 02/69] Velocity 1.1.3-SNAPSHOT --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index d38d21bbd..2e7b01568 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ allprojects { apply plugin: "com.github.spotbugs" group 'com.velocitypowered' - version '1.1.2' + version '1.1.3-SNAPSHOT' ext { // dependency versions From 084b741375d9cdd0311648b4e74c17217441b2f8 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 24 Nov 2020 12:03:34 -0500 Subject: [PATCH 03/69] Don't repeat validation in AvailableCommands When deserializing an AvailableCommands packet, we do a few sanity checks to ensure the packet is valid. Some of this work was repeated for each cycle (notably the root) so we now check the children and any redirects are defined only once. --- .../protocol/packet/AvailableCommands.java | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java index e6ffd5de8..1bbef5e5d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java @@ -204,6 +204,7 @@ public class AvailableCommands implements MinecraftPacket { private final int redirectTo; private final @Nullable ArgumentBuilder args; private @MonotonicNonNull CommandNode built; + private boolean validated; private WireNode(int idx, byte flags, int[] children, int redirectTo, @Nullable ArgumentBuilder args) { @@ -212,18 +213,33 @@ public class AvailableCommands implements MinecraftPacket { this.children = children; this.redirectTo = redirectTo; this.args = args; + this.validated = false; + } + + void validate(WireNode[] wireNodes) { + // Ensure all children exist. Note that we delay checking if the node has been built yet; + // that needs to come after this node is built. + for (int child : children) { + if (child >= wireNodes.length) { + throw new IllegalStateException("Node points to non-existent index " + redirectTo); + } + } + + if (redirectTo != -1) { + if (redirectTo >= wireNodes.length) { + throw new IllegalStateException("Node points to non-existent index " + redirectTo); + } + } + + this.validated = true; } boolean toNode(WireNode[] wireNodes) { - if (this.built == null) { - // Ensure all children exist. Note that we delay checking if the node has been built yet; - // that needs to come after this node is built. - for (int child : children) { - if (child >= wireNodes.length) { - throw new IllegalStateException("Node points to non-existent index " + redirectTo); - } - } + if (!this.validated) { + this.validate(wireNodes); + } + if (this.built == null) { int type = flags & FLAG_NODE_TYPE; if (type == NODE_TYPE_ROOT) { this.built = new RootCommandNode<>(); @@ -234,10 +250,6 @@ public class AvailableCommands implements MinecraftPacket { // Add any redirects if (redirectTo != -1) { - if (redirectTo >= wireNodes.length) { - throw new IllegalStateException("Node points to non-existent index " + redirectTo); - } - WireNode redirect = wireNodes[redirectTo]; if (redirect.built != null) { args.redirect(redirect.built); From aa7aee9dd779803e18aec67cbfc199661dbd1fb5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 24 Nov 2020 12:05:27 -0500 Subject: [PATCH 04/69] Add another validation case although it's not strictly required --- .../proxy/protocol/packet/AvailableCommands.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java index 1bbef5e5d..dc5e86ff5 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java @@ -220,13 +220,13 @@ public class AvailableCommands implements MinecraftPacket { // Ensure all children exist. Note that we delay checking if the node has been built yet; // that needs to come after this node is built. for (int child : children) { - if (child >= wireNodes.length) { + if (child < 0 || child >= wireNodes.length) { throw new IllegalStateException("Node points to non-existent index " + redirectTo); } } if (redirectTo != -1) { - if (redirectTo >= wireNodes.length) { + if (redirectTo < 0 || redirectTo >= wireNodes.length) { throw new IllegalStateException("Node points to non-existent index " + redirectTo); } } From fa2655d49b0bb27c930641657ee6f489010ee357 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 24 Nov 2020 12:09:49 -0500 Subject: [PATCH 05/69] Fix the debug message --- .../proxy/protocol/packet/AvailableCommands.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java index dc5e86ff5..891115511 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java @@ -221,13 +221,14 @@ public class AvailableCommands implements MinecraftPacket { // that needs to come after this node is built. for (int child : children) { if (child < 0 || child >= wireNodes.length) { - throw new IllegalStateException("Node points to non-existent index " + redirectTo); + throw new IllegalStateException("Node points to non-existent index " + child); } } if (redirectTo != -1) { if (redirectTo < 0 || redirectTo >= wireNodes.length) { - throw new IllegalStateException("Node points to non-existent index " + redirectTo); + throw new IllegalStateException("Redirect node points to non-existent index " + + redirectTo); } } From 9825f5891bf2ec19facddc829726255540d92000 Mon Sep 17 00:00:00 2001 From: Riley Park Date: Wed, 25 Nov 2020 12:22:33 -0800 Subject: [PATCH 06/69] Adventure 4.2.0 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 2e7b01568..ffa860d6c 100644 --- a/build.gradle +++ b/build.gradle @@ -21,7 +21,7 @@ allprojects { ext { // dependency versions textVersion = '3.0.4' - adventureVersion = '4.1.1' + adventureVersion = '4.2.0' adventurePlatformVersion = '4.0.0-SNAPSHOT' junitVersion = '5.7.0' slf4jVersion = '1.7.30' From 5da085d82f9cd7a91a4b8d44f6dc94dc3383b173 Mon Sep 17 00:00:00 2001 From: Riley Park Date: Sun, 6 Dec 2020 17:50:57 -0800 Subject: [PATCH 07/69] Adventure 4.3.0: Player list header/footer --- api/build.gradle | 11 +++--- .../com/velocitypowered/api/proxy/Player.java | 14 ++++++++ .../api/proxy/player/TabList.java | 2 ++ build.gradle | 2 +- proxy/build.gradle | 4 ++- .../connection/client/ConnectedPlayer.java | 36 +++++++++++++++++-- .../protocol/packet/HeaderAndFooter.java | 12 ++----- .../proxy/tablist/VelocityTabList.java | 33 ++++++++++------- .../proxy/tablist/VelocityTabListLegacy.java | 8 +++-- 9 files changed, 89 insertions(+), 33 deletions(-) diff --git a/api/build.gradle b/api/build.gradle index 8361c827e..b98bd6ea6 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -32,11 +32,12 @@ dependencies { // DEPRECATED: Will be removed in Velocity 2.0.0 api 'com.moandjiezana.toml:toml4j:0.7.2' - api "net.kyori:adventure-api:${adventureVersion}" - api "net.kyori:adventure-text-serializer-gson:${adventureVersion}" - api "net.kyori:adventure-text-serializer-legacy:${adventureVersion}" - api "net.kyori:adventure-text-serializer-plain:${adventureVersion}" - api "net.kyori:adventure-text-serializer-legacy-text3:${adventurePlatformVersion}" + api(platform("net.kyori:adventure-bom:${adventureVersion}")) + api("net.kyori:adventure-api") + api("net.kyori:adventure-text-serializer-gson") + api("net.kyori:adventure-text-serializer-legacy") + api("net.kyori:adventure-text-serializer-plain") + api("net.kyori:adventure-text-serializer-legacy-text3:${adventurePlatformVersion}") // adventure-platform api "org.slf4j:slf4j-api:${slf4jVersion}" api 'com.google.inject:guice:4.2.3' 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 598cb07a6..e5654a67e 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/Player.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/Player.java @@ -146,6 +146,20 @@ public interface Player extends CommandSource, Identified, InboundConnection, @Deprecated void clearHeaderAndFooter(); + /** + * Returns the player's player list header. + * + * @return this player's player list header + */ + Component getPlayerListHeader(); + + /** + * Returns the player's player list footer. + * + * @return this player's tab list + */ + Component getPlayerListFooter(); + /** * Returns the player's tab list. * diff --git a/api/src/main/java/com/velocitypowered/api/proxy/player/TabList.java b/api/src/main/java/com/velocitypowered/api/proxy/player/TabList.java index 0b862147c..59d8930be 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/player/TabList.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/player/TabList.java @@ -28,7 +28,9 @@ public interface TabList { * * @param header the header component * @param footer the footer component + * @deprecated Use {@link Player#sendPlayerListHeaderAndFooter(Component, Component)} instead */ + @Deprecated void setHeaderAndFooter(Component header, Component footer); /** diff --git a/build.gradle b/build.gradle index ffa860d6c..5ed54ddff 100644 --- a/build.gradle +++ b/build.gradle @@ -21,7 +21,7 @@ allprojects { ext { // dependency versions textVersion = '3.0.4' - adventureVersion = '4.2.0' + adventureVersion = '4.3.0' adventurePlatformVersion = '4.0.0-SNAPSHOT' junitVersion = '5.7.0' slf4jVersion = '1.7.30' diff --git a/proxy/build.gradle b/proxy/build.gradle index ff2e1fe0a..a345d4a2e 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -66,7 +66,9 @@ dependencies { implementation 'it.unimi.dsi:fastutil:8.4.1' implementation 'net.kyori:event-method-asm:4.0.0-SNAPSHOT' - implementation 'net.kyori:adventure-nbt:4.0.0-SNAPSHOT' + + implementation(platform("net.kyori:adventure-bom:${adventureVersion}")) + implementation("net.kyori:adventure-nbt") implementation 'org.asynchttpclient:async-http-client:2.12.1' 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 f635babf5..22b88fbca 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 @@ -45,6 +45,7 @@ import com.velocitypowered.proxy.protocol.StateRegistry; import com.velocitypowered.proxy.protocol.packet.Chat; import com.velocitypowered.proxy.protocol.packet.ClientSettings; import com.velocitypowered.proxy.protocol.packet.Disconnect; +import com.velocitypowered.proxy.protocol.packet.HeaderAndFooter; import com.velocitypowered.proxy.protocol.packet.KeepAlive; import com.velocitypowered.proxy.protocol.packet.PluginMessage; import com.velocitypowered.proxy.protocol.packet.ResourcePackRequest; @@ -61,6 +62,7 @@ import java.net.InetSocketAddress; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -105,6 +107,8 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { private @Nullable VelocityServerConnection connectionInFlight; private @Nullable PlayerSettings settings; private @Nullable ModInfo modInfo; + private Component playerListHeader = Component.empty(); + private Component playerListFooter = Component.empty(); private final VelocityTabList tabList; private final VelocityServer server; private ClientConnectionPhase connectionPhase; @@ -116,9 +120,9 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { @Nullable InetSocketAddress virtualHost, boolean onlineMode) { this.server = server; if (connection.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { - this.tabList = new VelocityTabList(connection); + this.tabList = new VelocityTabList(this); } else { - this.tabList = new VelocityTabListLegacy(connection); + this.tabList = new VelocityTabListLegacy(this); } this.profile = profile; this.connection = connection; @@ -301,6 +305,33 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { } } + @Override + public Component getPlayerListHeader() { + return this.playerListHeader; + } + + @Override + public Component getPlayerListFooter() { + return this.playerListFooter; + } + + @Override + public void sendPlayerListHeader(@NonNull final Component header) { + this.sendPlayerListHeaderAndFooter(header, this.playerListFooter); + } + + @Override + public void sendPlayerListFooter(@NonNull final Component footer) { + this.sendPlayerListHeaderAndFooter(this.playerListHeader, footer); + } + + @Override + public void sendPlayerListHeaderAndFooter(final Component header, final Component footer) { + this.playerListHeader = Objects.requireNonNull(header, "header"); + this.playerListFooter = Objects.requireNonNull(footer, "footer"); + this.connection.write(HeaderAndFooter.create(header, footer, this.getProtocolVersion())); + } + @Override public void showTitle(net.kyori.adventure.title.@NonNull Title title) { GsonComponentSerializer serializer = ProtocolUtils.getJsonChatSerializer(this @@ -363,6 +394,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { this.profile = profile.withProperties(Preconditions.checkNotNull(properties)); } + @Deprecated @Override public void setHeaderAndFooter(net.kyori.text.Component header, net.kyori.text.Component footer) { tabList.setHeaderAndFooter(header, footer); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HeaderAndFooter.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HeaderAndFooter.java index 93ceaed62..34f8d6098 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HeaderAndFooter.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HeaderAndFooter.java @@ -8,6 +8,7 @@ import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; +import net.kyori.adventure.text.Component; import net.kyori.adventure.text.serializer.gson.GsonComponentSerializer; public class HeaderAndFooter implements MinecraftPacket { @@ -51,15 +52,8 @@ public class HeaderAndFooter implements MinecraftPacket { return handler.handle(this); } - public static HeaderAndFooter create(net.kyori.text.Component header, - net.kyori.text.Component footer) { - return new HeaderAndFooter( - net.kyori.text.serializer.gson.GsonComponentSerializer.INSTANCE.serialize(header), - net.kyori.text.serializer.gson.GsonComponentSerializer.INSTANCE.serialize(footer)); - } - - public static HeaderAndFooter create(net.kyori.adventure.text.Component header, - net.kyori.adventure.text.Component footer, ProtocolVersion protocolVersion) { + public static HeaderAndFooter create(Component header, + Component footer, ProtocolVersion protocolVersion) { GsonComponentSerializer serializer = ProtocolUtils.getJsonChatSerializer(protocolVersion); return new HeaderAndFooter(serializer.serialize(header), serializer.serialize(footer)); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabList.java b/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabList.java index 5611a7761..1cae947fa 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabList.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabList.java @@ -5,6 +5,7 @@ import com.velocitypowered.api.proxy.player.TabList; import com.velocitypowered.api.proxy.player.TabListEntry; import com.velocitypowered.api.util.GameProfile; import com.velocitypowered.proxy.connection.MinecraftConnection; +import com.velocitypowered.proxy.connection.client.ConnectedPlayer; import com.velocitypowered.proxy.protocol.packet.HeaderAndFooter; import com.velocitypowered.proxy.protocol.packet.PlayerListItem; import java.util.ArrayList; @@ -15,31 +16,39 @@ import java.util.Map; import java.util.Optional; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import net.kyori.text.Component; +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.serializer.legacytext3.LegacyText3ComponentSerializer; import org.checkerframework.checker.nullness.qual.Nullable; public class VelocityTabList implements TabList { + protected final ConnectedPlayer player; protected final MinecraftConnection connection; protected final Map entries = new ConcurrentHashMap<>(); - public VelocityTabList(MinecraftConnection connection) { - this.connection = connection; + public VelocityTabList(final ConnectedPlayer player) { + this.player = player; + this.connection = player.getConnection(); } + @Deprecated + @Override + public void setHeaderAndFooter(net.kyori.text.Component header, + net.kyori.text.Component footer) { + Preconditions.checkNotNull(header, "header"); + Preconditions.checkNotNull(footer, "footer"); + this.player.sendPlayerListHeaderAndFooter( + LegacyText3ComponentSerializer.get().deserialize(header), + LegacyText3ComponentSerializer.get().deserialize(footer) + ); + } + + @Deprecated @Override public void setHeaderAndFooter(Component header, Component footer) { Preconditions.checkNotNull(header, "header"); Preconditions.checkNotNull(footer, "footer"); - connection.write(HeaderAndFooter.create(header, footer)); - } - - @Override - public void setHeaderAndFooter(net.kyori.adventure.text.Component header, - net.kyori.adventure.text.Component footer) { - Preconditions.checkNotNull(header, "header"); - Preconditions.checkNotNull(footer, "footer"); - connection.write(HeaderAndFooter.create(header, footer, connection.getProtocolVersion())); + this.player.sendPlayerListHeaderAndFooter(header, footer); } @Override diff --git a/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabListLegacy.java b/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabListLegacy.java index 305564ea7..95966c6b4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabListLegacy.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/tablist/VelocityTabListLegacy.java @@ -3,7 +3,7 @@ package com.velocitypowered.proxy.tablist; import com.google.common.collect.ImmutableList; import com.velocitypowered.api.proxy.player.TabListEntry; import com.velocitypowered.api.util.GameProfile; -import com.velocitypowered.proxy.connection.MinecraftConnection; +import com.velocitypowered.proxy.connection.client.ConnectedPlayer; import com.velocitypowered.proxy.protocol.packet.PlayerListItem; import com.velocitypowered.proxy.protocol.packet.PlayerListItem.Item; import java.util.Collections; @@ -18,14 +18,16 @@ public class VelocityTabListLegacy extends VelocityTabList { private final Map nameMapping = new ConcurrentHashMap<>(); - public VelocityTabListLegacy(MinecraftConnection connection) { - super(connection); + public VelocityTabListLegacy(final ConnectedPlayer player) { + super(player); } + @Deprecated @Override public void setHeaderAndFooter(net.kyori.text.Component header, net.kyori.text.Component footer) { } + @Deprecated @Override public void setHeaderAndFooter(Component header, Component footer) { } From 2a5bb1e48747f4e43d8eec3eb128248c21c6788c Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 7 Dec 2020 02:28:03 -0500 Subject: [PATCH 08/69] Fix tab list clearing bug. Fixes regression introduced in 5da085d --- .../proxy/connection/client/ConnectedPlayer.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 22b88fbca..653325c5f 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 @@ -119,11 +119,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { ConnectedPlayer(VelocityServer server, GameProfile profile, MinecraftConnection connection, @Nullable InetSocketAddress virtualHost, boolean onlineMode) { this.server = server; - if (connection.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { - this.tabList = new VelocityTabList(this); - } else { - this.tabList = new VelocityTabListLegacy(this); - } this.profile = profile; this.connection = connection; this.virtualHost = virtualHost; @@ -131,6 +126,12 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { this.connectionPhase = connection.getType().getInitialClientPhase(); this.knownChannels = CappedSet.create(MAX_PLUGIN_CHANNELS); this.onlineMode = onlineMode; + + if (connection.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + this.tabList = new VelocityTabList(this); + } else { + this.tabList = new VelocityTabListLegacy(this); + } } @Override From aef0e4a825f57186b8f1442fe1a2280a1f4ada8b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Wed, 9 Dec 2020 22:25:06 -0500 Subject: [PATCH 09/69] Update Netty to 4.1.55.Final. --- build.gradle | 2 +- proxy/build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 5ed54ddff..6aa4819d9 100644 --- a/build.gradle +++ b/build.gradle @@ -26,7 +26,7 @@ allprojects { junitVersion = '5.7.0' slf4jVersion = '1.7.30' log4jVersion = '2.13.3' - nettyVersion = '4.1.54.Final' + nettyVersion = '4.1.55.Final' guavaVersion = '25.1-jre' checkerFrameworkVersion = '3.6.1' configurateVersion = '3.7.1' diff --git a/proxy/build.gradle b/proxy/build.gradle index a345d4a2e..1b35443cf 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -51,7 +51,7 @@ dependencies { implementation "io.netty:netty-handler:${nettyVersion}" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-x86_64" - implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-aarch64" + implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-aarch_64" implementation "io.netty:netty-resolver-dns:${nettyVersion}" implementation "org.apache.logging.log4j:log4j-api:${log4jVersion}" From 88a57b77df6755fc4b5eee18e88ff1c6e658297d Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 10 Dec 2020 06:56:19 -0500 Subject: [PATCH 10/69] Stop bundling the Netty DNS resolver, we don't use it --- proxy/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/proxy/build.gradle b/proxy/build.gradle index 1b35443cf..f703a5749 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -52,7 +52,6 @@ dependencies { implementation "io.netty:netty-transport-native-epoll:${nettyVersion}" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-x86_64" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-aarch_64" - implementation "io.netty:netty-resolver-dns:${nettyVersion}" implementation "org.apache.logging.log4j:log4j-api:${log4jVersion}" implementation "org.apache.logging.log4j:log4j-core:${log4jVersion}" From 808205302eff95c03abae7f01d20ea9289bc343f Mon Sep 17 00:00:00 2001 From: Jk C Date: Fri, 11 Dec 2020 16:57:13 -0700 Subject: [PATCH 11/69] Spelling issue --- proxy/src/main/resources/default-velocity.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index c29cb1d01..c15135296 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -69,7 +69,7 @@ lobby = "127.0.0.1:30066" factions = "127.0.0.1:30067" minigames = "127.0.0.1:30068" -# In what order we should try servers when a player logs in or is kicked from aserver. +# In what order we should try servers when a player logs in or is kicked from a server. try = [ "lobby" ] @@ -169,4 +169,4 @@ online-mode-only = "&cThis server only accepts connections from online-mode clie no-available-servers = "&cThere are no available servers." already-connected = "&cYou are already connected to this proxy!" moved-to-new-server-prefix = "&cThe server you were on kicked you: " -generic-connection-error = "&cAn internal error occurred in your connection." \ No newline at end of file +generic-connection-error = "&cAn internal error occurred in your connection." From 5bd60a4b773749d58a74eda05d543461cc379b46 Mon Sep 17 00:00:00 2001 From: Jk C Date: Fri, 11 Dec 2020 18:57:01 -0700 Subject: [PATCH 12/69] Fix spelling issues and grammer issue --- proxy/src/main/resources/default-velocity.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index c15135296..2b54b385b 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -33,7 +33,7 @@ prevent-client-proxy-connections = false # Velocity's native forwarding. Only applicable for Minecraft 1.13 or higher. player-info-forwarding-mode = "NONE" -# If you are using modern or BungeeGuard IP forwarding, configure an unique secret here. +# If you are using modern or BungeeGuard IP forwarding, configure a unique secret here. forwarding-secret = "" # Announce whether or not your server supports Forge. If you run a modded server, we From 8c52341ff6049a38cb4500d4058500a749333a2a Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 13 Dec 2020 15:32:41 -0500 Subject: [PATCH 13/69] Do not pull in transitive dependencies of adventure-text-serializer-legacy-text3 Fixes #401 --- api/build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/build.gradle b/api/build.gradle index b98bd6ea6..74a20130c 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -37,7 +37,9 @@ dependencies { api("net.kyori:adventure-text-serializer-gson") api("net.kyori:adventure-text-serializer-legacy") api("net.kyori:adventure-text-serializer-plain") - api("net.kyori:adventure-text-serializer-legacy-text3:${adventurePlatformVersion}") // adventure-platform + api("net.kyori:adventure-text-serializer-legacy-text3:${adventurePlatformVersion}") { + transitive = false + } api "org.slf4j:slf4j-api:${slf4jVersion}" api 'com.google.inject:guice:4.2.3' From 991aaa31b12d701d0ce646114d64fdbee9504d54 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 13 Dec 2020 18:47:47 -0500 Subject: [PATCH 14/69] Revert "Stop bundling the Netty DNS resolver, we don't use it" This reverts commit 88a57b77df6755fc4b5eee18e88ff1c6e658297d. Breaks Geyser --- proxy/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/proxy/build.gradle b/proxy/build.gradle index f703a5749..1b35443cf 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -52,6 +52,7 @@ dependencies { implementation "io.netty:netty-transport-native-epoll:${nettyVersion}" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-x86_64" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-aarch_64" + implementation "io.netty:netty-resolver-dns:${nettyVersion}" implementation "org.apache.logging.log4j:log4j-api:${log4jVersion}" implementation "org.apache.logging.log4j:log4j-core:${log4jVersion}" From 4f5c315ef80f1d9de64fc9511234b171c0dca950 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 14 Dec 2020 04:29:28 -0500 Subject: [PATCH 15/69] Remove netty-resolver-dns dependency Geyser has fixed the issue in the latest builds --- proxy/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/proxy/build.gradle b/proxy/build.gradle index 1b35443cf..f703a5749 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -52,7 +52,6 @@ dependencies { implementation "io.netty:netty-transport-native-epoll:${nettyVersion}" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-x86_64" implementation "io.netty:netty-transport-native-epoll:${nettyVersion}:linux-aarch_64" - implementation "io.netty:netty-resolver-dns:${nettyVersion}" implementation "org.apache.logging.log4j:log4j-api:${log4jVersion}" implementation "org.apache.logging.log4j:log4j-core:${log4jVersion}" From 523b61e0c7d3803e25bed90574221b73b213fcd4 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 14 Dec 2020 14:39:39 -0500 Subject: [PATCH 16/69] Make sure unit tests actually run(!) and fix command hints --- api/build.gradle | 4 ++++ native/build.gradle | 4 ++++ proxy/build.gradle | 4 ++++ .../proxy/command/VelocityCommandManager.java | 7 ++---- .../proxy/util/BrigadierUtils.java | 22 +++++++++++++++++++ .../proxy/command/CommandManagerTests.java | 22 ++++--------------- 6 files changed, 40 insertions(+), 23 deletions(-) diff --git a/api/build.gradle b/api/build.gradle index 74a20130c..a02f510b9 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -98,6 +98,10 @@ javadoc { options.source = '8' } +test { + useJUnitPlatform() +} + publishing { publications { mavenJava(MavenPublication) { diff --git a/native/build.gradle b/native/build.gradle index 0a348523f..b5b260486 100644 --- a/native/build.gradle +++ b/native/build.gradle @@ -21,6 +21,10 @@ dependencies { testImplementation "org.junit.jupiter:junit-jupiter-engine:${junitVersion}" } +test { + useJUnitPlatform() +} + publishing { publications { mavenJava(MavenPublication) { diff --git a/proxy/build.gradle b/proxy/build.gradle index f703a5749..1ed87838e 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -81,6 +81,10 @@ dependencies { testImplementation "org.junit.jupiter:junit-jupiter-engine:${junitVersion}" } +test { + useJUnitPlatform() +} + shadowJar { exclude 'it/unimi/dsi/fastutil/booleans/**' exclude 'it/unimi/dsi/fastutil/bytes/**' 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 4165c0afa..f8adfc2e9 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -102,11 +102,8 @@ public class VelocityCommandManager implements CommandManager { } if (!(command instanceof BrigadierCommand)) { - if (!meta.getHints().isEmpty()) { - // If the user specified a hint, then add the hints to the command node directly. - for (CommandNode hint : meta.getHints()) { - node.addChild(hint); - } + for (CommandNode hint : meta.getHints()) { + node.addChild(BrigadierUtils.wrapForHinting(hint, node.getCommand())); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java index 1bad01978..73bf67f84 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java @@ -1,8 +1,10 @@ package com.velocitypowered.proxy.util; +import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.mojang.brigadier.Command; import com.mojang.brigadier.arguments.StringArgumentType; +import com.mojang.brigadier.builder.ArgumentBuilder; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.mojang.brigadier.context.CommandContext; @@ -11,6 +13,7 @@ import com.mojang.brigadier.tree.CommandNode; import com.mojang.brigadier.tree.LiteralCommandNode; import com.velocitypowered.api.command.CommandSource; import java.util.Locale; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Provides utilities for working with Brigadier commands. @@ -124,6 +127,25 @@ public final class BrigadierUtils { return command.toLowerCase(Locale.ENGLISH); } + /** + * Prepares the given command node prior for hinting metadata to + * a {@link com.velocitypowered.api.command.Command}. + * + * @param node the command node to be wrapped + * @param command the command to execute + * @return the wrapped command node + */ + public static CommandNode wrapForHinting( + final CommandNode node, final @Nullable Command command) { + Preconditions.checkNotNull(node, "node"); + ArgumentBuilder builder = node.createBuilder(); + builder.executes(command); + for (CommandNode child : node.getChildren()) { + builder.then(wrapForHinting(child, command)); + } + return builder.build(); + } + private BrigadierUtils() { throw new AssertionError(); } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java index ef2bf022f..d5dc457a9 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -122,20 +122,6 @@ public class CommandManagerTests { assertTrue(manager.hasCommand("foO")); } - @Test - void testAlreadyRegisteredThrows() { - VelocityCommandManager manager = createManager(); - manager.register("bar", new NoopDeprecatedCommand()); - assertThrows(IllegalArgumentException.class, () -> - manager.register("BAR", new NoopSimpleCommand())); - assertThrows(IllegalArgumentException.class, () -> { - CommandMeta meta = manager.metaBuilder("baz") - .aliases("BAr") - .build(); - manager.register(meta, new NoopRawCommand()); - }); - } - @Test void testBrigadierExecute() { VelocityCommandManager manager = createManager(); @@ -181,9 +167,9 @@ public class CommandManagerTests { assertTrue(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "buy 14").join()); assertTrue(checkedRequires.compareAndSet(true, false)); assertTrue(executed.get()); - assertFalse(manager.execute(MockCommandSource.INSTANCE, "buy 9"), + assertTrue(manager.execute(MockCommandSource.INSTANCE, "buy 9"), "Invalid arg returns false"); - assertFalse(manager.executeImmediately(MockCommandSource.INSTANCE, "buy 12 bananas")); + assertTrue(manager.executeImmediately(MockCommandSource.INSTANCE, "buy 12 bananas")); assertTrue(checkedRequires.get()); } @@ -392,7 +378,7 @@ public class CommandManagerTests { .join().isEmpty()); assertEquals( ImmutableList.of("123"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "simPle foo ").join()); + manager.offerSuggestions(MockCommandSource.INSTANCE, "simPle foo").join()); assertEquals( ImmutableList.of("baz", "foo"), manager.offerSuggestions(MockCommandSource.INSTANCE, "raw ").join()); @@ -411,7 +397,7 @@ public class CommandManagerTests { .join().isEmpty()); assertEquals( ImmutableList.of("123", "456"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "deprEcated foo ").join()); + manager.offerSuggestions(MockCommandSource.INSTANCE, "deprEcated foo").join()); assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "deprecated foo 789 ") .join().isEmpty()); } From 9c375f337bc458b88b7c08688eefe6377e883c14 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 17 Dec 2020 21:16:46 -0500 Subject: [PATCH 17/69] Bump to Netty 4.1.56.Final --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 6aa4819d9..cae9d9294 100644 --- a/build.gradle +++ b/build.gradle @@ -26,7 +26,7 @@ allprojects { junitVersion = '5.7.0' slf4jVersion = '1.7.30' log4jVersion = '2.13.3' - nettyVersion = '4.1.55.Final' + nettyVersion = '4.1.56.Final' guavaVersion = '25.1-jre' checkerFrameworkVersion = '3.6.1' configurateVersion = '3.7.1' From bac64ac95850aac30eb23b07ea4eedae6a27f51a Mon Sep 17 00:00:00 2001 From: Zach Levis Date: Sat, 19 Dec 2020 14:43:17 -0800 Subject: [PATCH 18/69] build: Fix search when generating Javadoc with broken java versions --- api/build.gradle | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/api/build.gradle b/api/build.gradle index a02f510b9..23864b6d0 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -93,9 +93,14 @@ javadoc { // Disable the crazy super-strict doclint tool in Java 8 options.addStringOption('Xdoclint:none', '-quiet') - + // Mark sources as Java 8 source compatible options.source = '8' + + // Remove 'undefined' from seach paths when generating javadoc for a non-modular project (JDK-8215291) + if (JavaVersion.current() >= JavaVersion.VERSION_1_9 && JavaVersion.current() < JavaVersion.VERSION_12) { + options.addBooleanOption('-no-module-directories', true) + } } test { From 98b74fd220e9c96e45dc800d15f84560e11a0e8e Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 21 Dec 2020 13:14:38 -0500 Subject: [PATCH 19/69] Correctly retain message buffer for LoginPluginMessage. Fixes #407 --- .../proxy/protocol/packet/LoginPluginMessage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LoginPluginMessage.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LoginPluginMessage.java index a3dae879d..c1eb6b95b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LoginPluginMessage.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LoginPluginMessage.java @@ -49,7 +49,7 @@ public class LoginPluginMessage extends DeferredByteBufHolder implements Minecra this.id = ProtocolUtils.readVarInt(buf); this.channel = ProtocolUtils.readString(buf); if (buf.isReadable()) { - this.replace(buf.readSlice(buf.readableBytes())); + this.replace(buf.readRetainedSlice(buf.readableBytes())); } else { this.replace(Unpooled.EMPTY_BUFFER); } From eb3868d91185c88e0fe4a0738fc5965de04b87ee Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 21 Dec 2020 19:56:11 -0500 Subject: [PATCH 20/69] Do not try to override colors in messages or server-sent messages --- .../proxy/connection/client/ConnectedPlayer.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 653325c5f..55d8df8b4 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 @@ -569,7 +569,8 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { logger.error("{}: kicked from server {}: {}", this, server.getServerInfo().getName(), plainTextReason); handleConnectionException(server, disconnectReason, Component.text() - .append(messages.getKickPrefix(server.getServerInfo().getName())) + .append(messages.getKickPrefix(server.getServerInfo().getName()) + .colorIfAbsent(NamedTextColor.RED)) .color(NamedTextColor.RED) .append(disconnectReason) .build(), safe); @@ -577,8 +578,8 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { logger.error("{}: disconnected while connecting to {}: {}", this, server.getServerInfo().getName(), plainTextReason); handleConnectionException(server, disconnectReason, Component.text() - .append(messages.getDisconnectPrefix(server.getServerInfo().getName())) - .color(NamedTextColor.RED) + .append(messages.getDisconnectPrefix(server.getServerInfo().getName()) + .colorIfAbsent(NamedTextColor.RED)) .append(disconnectReason) .build(), safe); } From 07b95d46ac01b680e11af644c4ed0e0d4f8149af Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 21 Dec 2020 20:14:57 -0500 Subject: [PATCH 21/69] Delay sending stats to bStats for 3-5 minutes after server startup --- proxy/src/main/java/com/velocitypowered/proxy/Metrics.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java b/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java index 182316d7d..d8d635afa 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java @@ -18,6 +18,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ThreadLocalRandom; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.zip.GZIPOutputStream; @@ -33,6 +34,8 @@ import org.asynchttpclient.Response; */ public class Metrics { + private static final int ONE_MINUTE_MS = 60_000; + // The version of this bStats class private static final int B_STATS_METRICS_REVISION = 2; @@ -96,12 +99,14 @@ public class Metrics { */ private void startSubmitting() { final Timer timer = new Timer(true); + long initialDelay = ONE_MINUTE_MS * 3 + (((long) (ThreadLocalRandom.current().nextDouble() * 2 + * ONE_MINUTE_MS))); timer.scheduleAtFixedRate(new TimerTask() { @Override public void run() { submitData(); } - }, 1000, 1000 * 60 * 30); + }, initialDelay, 1000 * 60 * 30); // Submit the data every 30 minutes, first time after 5 minutes to give other plugins enough // time to start. // From 7fe2fc71e928e5980ae143acf51bda3bd21b394c Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 25 Dec 2020 17:03:01 -0500 Subject: [PATCH 22/69] Add some small debug for refcount issues with plugin messages. --- .../proxy/protocol/packet/PluginMessage.java | 6 ++++++ 1 file changed, 6 insertions(+) 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 5b9b3f136..ecd610629 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 @@ -63,6 +63,12 @@ public class PluginMessage extends DeferredByteBufHolder implements MinecraftPac if (channel == null) { throw new IllegalStateException("Channel is not specified."); } + + if (refCnt() == 0) { + throw new IllegalStateException("Plugin message contents for " + this.channel + + " freed too many times."); + } + if (version.compareTo(ProtocolVersion.MINECRAFT_1_13) >= 0) { ProtocolUtils.writeString(buf, transformLegacyToModernChannel(this.channel)); } else { From b1f7980c5d2d31925af7fbae46fb7726fc5372c1 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 26 Dec 2020 19:49:28 -0500 Subject: [PATCH 23/69] Fix ForwardToPlayer subchannel, closes #406 --- .../proxy/connection/backend/BungeeCordMessageResponder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java index ba205c6b6..1450564a6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java @@ -250,8 +250,8 @@ public class BungeeCordMessageResponder { private void processForwardToPlayer(ByteBufDataInput in) { proxy.getPlayer(in.readUTF()) - .flatMap(Player::getCurrentServer) - .ifPresent(server -> sendServerResponse(player, prepareForwardMessage(in))); + .ifPresent(foundPlayer -> sendServerResponse((ConnectedPlayer) foundPlayer, + prepareForwardMessage(in))); } private void processForwardToServer(ByteBufDataInput in) { From f6078e9b74fac3f7fabf5060e4303143b08da59b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 26 Dec 2020 20:52:12 -0500 Subject: [PATCH 24/69] Fix several problems and clean up the BungeeCord plugin messaging support. Fixes #402 --- .../backend/BungeeCordMessageResponder.java | 65 ++++++------------- .../server/VelocityRegisteredServer.java | 7 +- 2 files changed, 26 insertions(+), 46 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java index 1450564a6..1a0289051 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java @@ -236,75 +236,52 @@ public class BungeeCordMessageResponder { }); } - private ByteBuf prepareForwardMessage(ByteBufDataInput in) { - String channel = in.readUTF(); - short messageLength = in.readShort(); - - ByteBuf buf = Unpooled.buffer(); - ByteBufDataOutput forwarded = new ByteBufDataOutput(buf); - forwarded.writeUTF(channel); - forwarded.writeShort(messageLength); - buf.writeBytes(in.unwrap().readSlice(messageLength)); - return buf; - } - private void processForwardToPlayer(ByteBufDataInput in) { - proxy.getPlayer(in.readUTF()) - .ifPresent(foundPlayer -> sendServerResponse((ConnectedPlayer) foundPlayer, - prepareForwardMessage(in))); + Optional player = proxy.getPlayer(in.readUTF()); + if (player.isPresent()) { + ByteBuf toForward = in.unwrap().copy(); + sendServerResponse((ConnectedPlayer) player.get(), toForward); + } } private void processForwardToServer(ByteBufDataInput in) { String target = in.readUTF(); - ByteBuf toForward = prepareForwardMessage(in); + ByteBuf toForward = in.unwrap().copy(); if (target.equals("ALL")) { - ByteBuf unreleasableForward = Unpooled.unreleasableBuffer(toForward); try { for (RegisteredServer rs : proxy.getAllServers()) { - ((VelocityRegisteredServer) rs).sendPluginMessage(LEGACY_CHANNEL, unreleasableForward); + ((VelocityRegisteredServer) rs).sendPluginMessage(LEGACY_CHANNEL, + toForward.retainedSlice()); } } finally { toForward.release(); } } else { - proxy.getServer(target).ifPresent(rs -> ((VelocityRegisteredServer) rs) - .sendPluginMessage(LEGACY_CHANNEL, toForward)); + Optional server = proxy.getServer(target); + if (server.isPresent()) { + ((VelocityRegisteredServer) server.get()).sendPluginMessage(LEGACY_CHANNEL, toForward); + } else { + toForward.release(); + } } } - // Note: this method will always release the buffer! - private void sendResponseOnConnection(ByteBuf buf) { - sendServerResponse(this.player, buf); - } - static String getBungeeCordChannel(ProtocolVersion version) { return version.compareTo(ProtocolVersion.MINECRAFT_1_13) >= 0 ? MODERN_CHANNEL.getId() : LEGACY_CHANNEL.getId(); } + // Note: this method will always release the buffer! + private void sendResponseOnConnection(ByteBuf buf) { + sendServerResponse(this.player, buf); + } + // Note: this method will always release the buffer! private static void sendServerResponse(ConnectedPlayer player, ByteBuf buf) { MinecraftConnection serverConnection = player.ensureAndGetCurrentServer().ensureConnected(); String chan = getBungeeCordChannel(serverConnection.getProtocolVersion()); - - PluginMessage msg = null; - boolean released = false; - - try { - VelocityServerConnection vsc = player.getConnectedServer(); - if (vsc == null) { - return; - } - - MinecraftConnection serverConn = vsc.ensureConnected(); - msg = new PluginMessage(chan, buf); - serverConn.write(msg); - released = true; - } finally { - if (!released && msg != null) { - msg.release(); - } - } + PluginMessage msg = new PluginMessage(chan, buf); + serverConnection.write(msg); } boolean process(PluginMessage message) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/server/VelocityRegisteredServer.java b/proxy/src/main/java/com/velocitypowered/proxy/server/VelocityRegisteredServer.java index 5987311e1..3cf0313d9 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/server/VelocityRegisteredServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/server/VelocityRegisteredServer.java @@ -125,7 +125,9 @@ public class VelocityRegisteredServer implements RegisteredServer, ForwardingAud } /** - * Sends a plugin message to the server through this connection. + * Sends a plugin message to the server through this connection. The message will be released + * afterwards. + * * @param identifier the channel ID to use * @param data the data * @return whether or not the message was sent @@ -133,11 +135,12 @@ public class VelocityRegisteredServer implements RegisteredServer, ForwardingAud public boolean sendPluginMessage(ChannelIdentifier identifier, ByteBuf data) { for (ConnectedPlayer player : players.values()) { VelocityServerConnection connection = player.getConnectedServer(); - if (connection != null && connection.getServerInfo().equals(serverInfo)) { + if (connection != null && connection.getServer() == this) { return connection.sendPluginMessage(identifier, data); } } + data.release(); return false; } From 6a8ec21f261fadfc55208f8ca0ffc9f53428cc2c Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 26 Dec 2020 20:59:17 -0500 Subject: [PATCH 25/69] Use a different quiet exception type to indicate the user should enable a debugging flag if needed --- .../proxy/protocol/netty/MinecraftDecoder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 5139bedfd..9e442e493 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 @@ -5,7 +5,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.StateRegistry; -import com.velocitypowered.proxy.util.except.QuietDecoderException; +import com.velocitypowered.proxy.util.except.QuietRuntimeException; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; @@ -14,8 +14,8 @@ import io.netty.handler.codec.CorruptedFrameException; public class MinecraftDecoder extends ChannelInboundHandlerAdapter { public static final boolean DEBUG = Boolean.getBoolean("velocity.packet-decode-logging"); - private static final QuietDecoderException DECODE_FAILED = - new QuietDecoderException("A packet did not decode successfully (invalid data). If you are a " + private static final QuietRuntimeException DECODE_FAILED = + new QuietRuntimeException("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; From 7329d165f64c6a59e0762d00315b5fe58589f7dd Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 26 Dec 2020 21:56:19 -0500 Subject: [PATCH 26/69] 2021 --- .../velocitypowered/proxy/command/builtin/VelocityCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java b/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java index 3218960b9..1b5e61919 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java @@ -209,7 +209,7 @@ public class VelocityCommand implements SimpleCommand { .append(Component.text(version.getVersion()).decoration(TextDecoration.BOLD, false)) .build(); TextComponent copyright = Component - .text("Copyright 2018-2020 " + version.getVendor() + ". " + version.getName() + .text("Copyright 2018-2021 " + version.getVendor() + ". " + version.getName() + " is freely licensed under the terms of the MIT License."); source.sendMessage(Identity.nil(), velocity); source.sendMessage(Identity.nil(), copyright); From 2a1e83902da2b67e440d8cd4a47588702a31997a Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 27 Dec 2020 18:05:27 -0500 Subject: [PATCH 27/69] Add support for server-side backpressure --- .../proxy/connection/MinecraftConnection.java | 4 ++++ .../connection/backend/BackendPlaySessionHandler.java | 8 ++++++++ .../proxy/connection/client/ClientPlaySessionHandler.java | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) 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 acf7fe5cc..3571f7a0f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -195,6 +195,8 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { public void write(Object msg) { if (channel.isActive()) { channel.writeAndFlush(msg, channel.voidPromise()); + } else { + ReferenceCountUtil.release(msg); } } @@ -205,6 +207,8 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { public void delayedWrite(Object msg) { if (channel.isActive()) { channel.write(msg, channel.voidPromise()); + } else { + ReferenceCountUtil.release(msg); } } 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 abd1f7b91..476460dc1 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 @@ -28,6 +28,7 @@ import com.velocitypowered.proxy.protocol.util.PluginMessageUtil; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufUtil; import io.netty.buffer.Unpooled; +import io.netty.channel.Channel; import io.netty.handler.timeout.ReadTimeoutException; import java.util.Collection; import org.apache.logging.log4j.LogManager; @@ -284,4 +285,11 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { } } } + + @Override + public void writabilityChanged() { + Channel serverChan = serverConn.ensureConnected().getChannel(); + boolean writable = serverChan.isWritable(); + playerConnection.setAutoReading(writable); + } } 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 5757b3040..b41a5ae32 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -300,7 +300,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { boolean writable = player.getConnection().getChannel().isWritable(); if (!writable) { - // We might have packets queued for the server, so flush them now to free up memory. + // We might have packets queued from the server, so flush them now to free up memory. player.getConnection().flush(); } From dc7ab0f7f57ea08632a473b3e7426665fef678ee Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 27 Dec 2020 18:36:54 -0500 Subject: [PATCH 28/69] Log server backpressure --- .../connection/backend/BackendPlaySessionHandler.java | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 476460dc1..6dab7d8a5 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 @@ -37,6 +37,8 @@ import org.apache.logging.log4j.Logger; public class BackendPlaySessionHandler implements MinecraftSessionHandler { private static final Logger logger = LogManager.getLogger(BackendPlaySessionHandler.class); + private static final boolean BACKPRESSURE_LOG = Boolean + .getBoolean("velocity.log-server-backpressure"); private final VelocityServer server; private final VelocityServerConnection serverConn; private final ClientPlaySessionHandler playerSessionHandler; @@ -290,6 +292,15 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { public void writabilityChanged() { Channel serverChan = serverConn.ensureConnected().getChannel(); boolean writable = serverChan.isWritable(); + + if (BACKPRESSURE_LOG) { + if (writable) { + logger.info("{} is not writable, not auto-reading player connection data", this.serverConn); + } else { + logger.info("{} is writable, will auto-read player connection data", this.serverConn); + } + } + playerConnection.setAutoReading(writable); } } From 44046fe77264e083eb200b03b4e5b8c92d04ba28 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 29 Dec 2020 18:06:46 -0500 Subject: [PATCH 29/69] Velocity 1.1.3 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index cae9d9294..cd57a9f37 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ allprojects { apply plugin: "com.github.spotbugs" group 'com.velocitypowered' - version '1.1.3-SNAPSHOT' + version '1.1.3' ext { // dependency versions From 42d211bf08157cbbdb5cb7eb94abd02306bd60ac Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 29 Dec 2020 18:07:14 -0500 Subject: [PATCH 30/69] Velocity 1.1.4-SNAPSHOT --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index cd57a9f37..19667d01c 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ allprojects { apply plugin: "com.github.spotbugs" group 'com.velocitypowered' - version '1.1.3' + version '1.1.4-SNAPSHOT' ext { // dependency versions From 1f9c24566d9c3715e22a94158d2b9b2525f9d81e Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 31 Dec 2020 13:18:01 -0500 Subject: [PATCH 31/69] Fix off-by-one error in MinecraftCompressEncoder --- .../proxy/protocol/netty/MinecraftCompressEncoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java index a3e36f4cc..23c67c024 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java @@ -20,7 +20,7 @@ public class MinecraftCompressEncoder extends MessageToByteEncoder { @Override protected void encode(ChannelHandlerContext ctx, ByteBuf msg, ByteBuf out) throws Exception { int uncompressed = msg.readableBytes(); - if (uncompressed <= threshold) { + if (uncompressed < threshold) { // Under the threshold, there is nothing to do. ProtocolUtils.writeVarInt(out, 0); out.writeBytes(msg); From 4df8f70156cfdb2be4329739e014f2affe12d361 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Wed, 6 Jan 2021 13:29:30 -0500 Subject: [PATCH 32/69] Rename proxy-protocol setting to haproxy-protocol to make it clearer this is for HAProxy. --- .../velocitypowered/proxy/config/VelocityConfiguration.java | 6 +++++- proxy/src/main/resources/default-velocity.toml | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index 5660b913e..5f849ff47 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -646,7 +646,11 @@ public class VelocityConfiguration implements ProxyConfig { this.loginRatelimit = config.getIntOrElse("login-ratelimit", 3000); this.connectionTimeout = config.getIntOrElse("connection-timeout", 5000); this.readTimeout = config.getIntOrElse("read-timeout", 30000); - this.proxyProtocol = config.getOrElse("proxy-protocol", false); + if (config.contains("haproxy-protocol")) { + this.proxyProtocol = config.getOrElse("haproxy-protocol", false); + } else { + this.proxyProtocol = config.getOrElse("proxy-protocol", false); + } this.tcpFastOpen = config.getOrElse("tcp-fast-open", false); this.bungeePluginMessageChannel = config.getOrElse("bungee-plugin-message-channel", true); this.showPingRequests = config.getOrElse("show-ping-requests", false); diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index 2b54b385b..fe3ad5942 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -106,7 +106,7 @@ connection-timeout = 5000 read-timeout = 30000 # Enables compatibility with HAProxy. -proxy-protocol = false +haproxy-protocol = false # Enables TCP fast open support on the proxy. Requires the proxy to run on Linux. tcp-fast-open = false From ba1c1eef648ecc8b2a8797052ecbbfd5e007d76d Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Wed, 6 Jan 2021 13:30:57 -0500 Subject: [PATCH 33/69] More clarification --- proxy/src/main/resources/default-velocity.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index fe3ad5942..050b9c1fe 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -105,7 +105,8 @@ connection-timeout = 5000 # Specify a read timeout for connections here. The default is 30 seconds. read-timeout = 30000 -# Enables compatibility with HAProxy. +# Enables compatibility with HAProxy's PROXY protocol. If you don't know what this is for, then +# don't enable it. haproxy-protocol = false # Enables TCP fast open support on the proxy. Requires the proxy to run on Linux. From 567a3b236534cb537a82f343f868c52ad38d414e Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 9 Jan 2021 16:39:38 -0500 Subject: [PATCH 34/69] Swallow and more usefully log exceptions in scheduler task running. --- .../proxy/scheduler/VelocityScheduler.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/scheduler/VelocityScheduler.java b/proxy/src/main/java/com/velocitypowered/proxy/scheduler/VelocityScheduler.java index 1bec0d119..4049be785 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/scheduler/VelocityScheduler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/scheduler/VelocityScheduler.java @@ -7,6 +7,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.velocitypowered.api.plugin.PluginContainer; import com.velocitypowered.api.plugin.PluginManager; import com.velocitypowered.api.scheduler.ScheduledTask; import com.velocitypowered.api.scheduler.Scheduler; @@ -183,8 +184,18 @@ public class VelocityScheduler implements Scheduler { currentTaskThread = Thread.currentThread(); try { runnable.run(); - } catch (Exception e) { - Log.logger.error("Exception in task {} by plugin {}", runnable, plugin, e); + } catch (Throwable e) { + //noinspection ConstantConditions + if (e instanceof InterruptedException) { + Thread.currentThread().interrupt(); + } else { + String friendlyPluginName = pluginManager.fromInstance(plugin) + .map(container -> container.getDescription().getName() + .orElse(container.getDescription().getId())) + .orElse("UNKNOWN"); + Log.logger.error("Exception in task {} by plugin {}", runnable, friendlyPluginName, + e); + } } finally { if (repeat == 0) { onFinish(); From 919319438d9c8017b97a3dcd4470609b0440210f Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 17 Jan 2021 00:43:35 -0500 Subject: [PATCH 35/69] Bump Netty version to 4.1.58.Final --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 19667d01c..9becd93df 100644 --- a/build.gradle +++ b/build.gradle @@ -26,7 +26,7 @@ allprojects { junitVersion = '5.7.0' slf4jVersion = '1.7.30' log4jVersion = '2.13.3' - nettyVersion = '4.1.56.Final' + nettyVersion = '4.1.58.Final' guavaVersion = '25.1-jre' checkerFrameworkVersion = '3.6.1' configurateVersion = '3.7.1' From 0fa61216e771cb5ff84c7a165d374476ac1f5c8a Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 19 Jan 2021 02:40:32 -0500 Subject: [PATCH 36/69] Bump to Adventure 4.4.0 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 9becd93df..ebb19f20a 100644 --- a/build.gradle +++ b/build.gradle @@ -21,7 +21,7 @@ allprojects { ext { // dependency versions textVersion = '3.0.4' - adventureVersion = '4.3.0' + adventureVersion = '4.4.0' adventurePlatformVersion = '4.0.0-SNAPSHOT' junitVersion = '5.7.0' slf4jVersion = '1.7.30' From 892ac6f6263f00c85d0e358d6efaa4ee646f6c3f Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 21 Jan 2021 17:57:40 -0500 Subject: [PATCH 37/69] Accept changes to compression treshold on the fly. Vanilla allows this for some reason, and there has been one case where Velocity's strict behavior has caused a bug. Fix this. --- .../proxy/connection/MinecraftConnection.java | 27 ++++++++++++------- .../netty/MinecraftCompressDecoder.java | 6 ++++- .../netty/MinecraftCompressEncoder.java | 6 ++++- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java index 3571f7a0f..42b3a766f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -382,16 +382,25 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { if (threshold == -1) { channel.pipeline().remove(COMPRESSION_DECODER); channel.pipeline().remove(COMPRESSION_ENCODER); - return; + } else { + MinecraftCompressDecoder decoder = (MinecraftCompressDecoder) channel.pipeline() + .get(COMPRESSION_DECODER); + MinecraftCompressEncoder encoder = (MinecraftCompressEncoder) channel.pipeline() + .get(COMPRESSION_ENCODER); + if (decoder != null && encoder != null) { + decoder.setThreshold(threshold); + encoder.setThreshold(threshold); + } else { + int level = server.getConfiguration().getCompressionLevel(); + VelocityCompressor compressor = Natives.compress.get().create(level); + + encoder = new MinecraftCompressEncoder(threshold, compressor); + decoder = new MinecraftCompressDecoder(threshold, compressor); + + channel.pipeline().addBefore(MINECRAFT_DECODER, COMPRESSION_DECODER, decoder); + channel.pipeline().addBefore(MINECRAFT_ENCODER, COMPRESSION_ENCODER, encoder); + } } - - int level = server.getConfiguration().getCompressionLevel(); - VelocityCompressor compressor = Natives.compress.get().create(level); - MinecraftCompressEncoder encoder = new MinecraftCompressEncoder(threshold, compressor); - MinecraftCompressDecoder decoder = new MinecraftCompressDecoder(threshold, compressor); - - channel.pipeline().addBefore(MINECRAFT_DECODER, COMPRESSION_DECODER, decoder); - channel.pipeline().addBefore(MINECRAFT_ENCODER, COMPRESSION_ENCODER, encoder); } /** diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java index a083a9291..21b958b05 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java @@ -20,7 +20,7 @@ public class MinecraftCompressDecoder extends MessageToMessageDecoder { Boolean.getBoolean("velocity.increased-compression-cap") ? HARD_MAXIMUM_UNCOMPRESSED_SIZE : VANILLA_MAXIMUM_UNCOMPRESSED_SIZE; - private final int threshold; + private int threshold; private final VelocityCompressor compressor; public MinecraftCompressDecoder(int threshold, VelocityCompressor compressor) { @@ -60,4 +60,8 @@ public class MinecraftCompressDecoder extends MessageToMessageDecoder { public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { compressor.close(); } + + public void setThreshold(int threshold) { + this.threshold = threshold; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java index 23c67c024..aad607602 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java @@ -9,7 +9,7 @@ import io.netty.handler.codec.MessageToByteEncoder; public class MinecraftCompressEncoder extends MessageToByteEncoder { - private final int threshold; + private int threshold; private final VelocityCompressor compressor; public MinecraftCompressEncoder(int threshold, VelocityCompressor compressor) { @@ -54,4 +54,8 @@ public class MinecraftCompressEncoder extends MessageToByteEncoder { public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { compressor.close(); } + + public void setThreshold(int threshold) { + this.threshold = threshold; + } } From 13ee59d20b5ead30210fb1504151c71e782b99bb Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 22 Jan 2021 19:49:38 -0500 Subject: [PATCH 38/69] Update CONTRIBUTING.md --- CONTRIBUTING.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e44a7026d..c650ff3af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,9 +28,6 @@ sure that you are properly adhering to the code style. To reduce bugs and ensure code quality, we run the following tools on all commits and pull requests: -* [Checker Framework](https://checkerframework.org/): an enhancement to Java's type - system that is designed to help catch bugs. Velocity runs the _Nullness Checker_ - and the _Optional Checker_. The build will fail if Checker Framework notices an - issue. +* [Error Prone](https://errorprone.info/) * [Checkstyle](http://checkstyle.sourceforge.net/): ensures that your code is - correctly formatted. The build will fail if Checkstyle detects a problem. \ No newline at end of file + correctly formatted. The build will fail if Checkstyle detects a problem. From a2a799a8a7328546d762585cb35160ff187989f6 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 22 Jan 2021 19:50:31 -0500 Subject: [PATCH 39/69] Update CONTRIBUTING.md --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c650ff3af..e6b626f81 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,6 +28,7 @@ sure that you are properly adhering to the code style. To reduce bugs and ensure code quality, we run the following tools on all commits and pull requests: -* [Error Prone](https://errorprone.info/) +* [SpotBugs](https://spotbugs.github.io/): ensures that common errors do not + get into the codebase. The build will fail if SpotBugs finds an issue. * [Checkstyle](http://checkstyle.sourceforge.net/): ensures that your code is correctly formatted. The build will fail if Checkstyle detects a problem. From ce7d1dcb2662e6c3f4416092d40dc226e344e441 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 23 Jan 2021 01:34:38 -0500 Subject: [PATCH 40/69] A new year, a new color for Velocity! --- .../proxy/command/builtin/VelocityCommand.java | 4 +++- .../velocitypowered/proxy/config/VelocityConfiguration.java | 2 +- proxy/src/main/resources/default-velocity.toml | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java b/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java index 1b5e61919..3297f457b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/builtin/VelocityCommand.java @@ -38,6 +38,7 @@ import net.kyori.adventure.text.TextComponent; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.event.HoverEvent; import net.kyori.adventure.text.format.NamedTextColor; +import net.kyori.adventure.text.format.TextColor; import net.kyori.adventure.text.format.TextDecoration; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -188,6 +189,7 @@ public class VelocityCommand implements SimpleCommand { private static class Info implements SubCommand { + private static final TextColor VELOCITY_COLOR = TextColor.fromHexString("#09add3"); private final ProxyServer server; private Info(ProxyServer server) { @@ -205,7 +207,7 @@ public class VelocityCommand implements SimpleCommand { TextComponent velocity = Component.text().content(version.getName() + " ") .decoration(TextDecoration.BOLD, true) - .color(NamedTextColor.DARK_AQUA) + .color(VELOCITY_COLOR) .append(Component.text(version.getVersion()).decoration(TextDecoration.BOLD, false)) .build(); TextComponent copyright = Component 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 5f849ff47..803312423 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -458,7 +458,7 @@ public class VelocityConfiguration implements ProxyConfig { PingPassthroughMode.DISABLED); String bind = config.getOrElse("bind", "0.0.0.0:25577"); - String motd = config.getOrElse("motd", "&3A Velocity Server"); + String motd = config.getOrElse("motd", " add3A Velocity Server"); int maxPlayers = config.getIntOrElse("show-max-players", 500); Boolean onlineMode = config.getOrElse("online-mode", true); Boolean announceForge = config.getOrElse("announce-forge", true); diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index 050b9c1fe..fe57acad7 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -6,7 +6,7 @@ bind = "0.0.0.0:25577" # What should be the MOTD? This gets displayed when the player adds your server to # their server list. Legacy color codes and JSON are accepted. -motd = "&3A Velocity Server" +motd = "&3#09add3A Velocity Server" # What should we display for the maximum number of players? (Velocity does not support a cap # on the number of players online.) From fcffccf0d8d291be9ba0bdff9c7ed6717fce9db2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 23 Jan 2021 01:35:49 -0500 Subject: [PATCH 41/69] Fix typo --- proxy/src/main/resources/default-velocity.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index fe57acad7..f3b890926 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -6,7 +6,7 @@ bind = "0.0.0.0:25577" # What should be the MOTD? This gets displayed when the player adds your server to # their server list. Legacy color codes and JSON are accepted. -motd = "&3#09add3A Velocity Server" +motd = " add3A Velocity Server" # What should we display for the maximum number of players? (Velocity does not support a cap # on the number of players online.) From 03e9fa79d6bbf19ca7446c234d78bb4c400d1854 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 23 Jan 2021 12:55:28 -0500 Subject: [PATCH 42/69] Raise limit on JoinGame NBT reading to 2MiB. This is required for particularly large mod packs (think All of Fabric 3 for instance). --- .../velocitypowered/proxy/protocol/ProtocolUtils.java | 9 ++++++--- .../velocitypowered/proxy/protocol/packet/JoinGame.java | 6 ++++-- .../velocitypowered/proxy/protocol/packet/Respawn.java | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index a7e0bd437..2c4f50a99 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -16,6 +16,8 @@ import io.netty.handler.codec.CorruptedFrameException; import io.netty.handler.codec.DecoderException; import io.netty.handler.codec.EncoderException; +import java.io.DataInput; +import java.io.DataOutput; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -227,11 +229,12 @@ public enum ProtocolUtils { /** * Reads a {@link net.kyori.adventure.nbt.CompoundBinaryTag} from the {@code buf}. * @param buf the buffer to read from + * @param reader the NBT reader to use * @return {@link net.kyori.adventure.nbt.CompoundBinaryTag} the CompoundTag from the buffer */ - public static CompoundBinaryTag readCompoundTag(ByteBuf buf) { + public static CompoundBinaryTag readCompoundTag(ByteBuf buf, BinaryTagIO.Reader reader) { try { - return BinaryTagIO.readDataInput(new ByteBufInputStream(buf)); + return reader.read((DataInput) new ByteBufInputStream(buf)); } catch (IOException thrown) { throw new DecoderException( "Unable to parse NBT CompoundTag, full error: " + thrown.getMessage()); @@ -245,7 +248,7 @@ public enum ProtocolUtils { */ public static void writeCompoundTag(ByteBuf buf, CompoundBinaryTag compoundTag) { try { - BinaryTagIO.writeDataOutput(compoundTag, new ByteBufOutputStream(buf)); + BinaryTagIO.writer().write(compoundTag, (DataOutput) new ByteBufOutputStream(buf)); } catch (IOException e) { throw new EncoderException("Unable to encode NBT CompoundTag"); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/JoinGame.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/JoinGame.java index 76c384584..84b4fd3ec 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/JoinGame.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/JoinGame.java @@ -8,6 +8,7 @@ import com.velocitypowered.proxy.connection.registry.DimensionInfo; import com.velocitypowered.proxy.connection.registry.DimensionRegistry; import com.velocitypowered.proxy.protocol.*; import io.netty.buffer.ByteBuf; +import net.kyori.adventure.nbt.BinaryTagIO; import net.kyori.adventure.nbt.BinaryTagTypes; import net.kyori.adventure.nbt.CompoundBinaryTag; import net.kyori.adventure.nbt.ListBinaryTag; @@ -15,6 +16,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; public class JoinGame implements MinecraftPacket { + private static final BinaryTagIO.Reader JOINGAME_READER = BinaryTagIO.reader(2 * 1024 * 1024); private int entityId; private short gamemode; private int dimension; @@ -178,7 +180,7 @@ public class JoinGame implements MinecraftPacket { if (version.compareTo(ProtocolVersion.MINECRAFT_1_16) >= 0) { this.previousGamemode = buf.readByte(); ImmutableSet levelNames = ImmutableSet.copyOf(ProtocolUtils.readStringArray(buf)); - CompoundBinaryTag registryContainer = ProtocolUtils.readCompoundTag(buf); + CompoundBinaryTag registryContainer = ProtocolUtils.readCompoundTag(buf, JOINGAME_READER); ListBinaryTag dimensionRegistryContainer = null; if (version.compareTo(ProtocolVersion.MINECRAFT_1_16_2) >= 0) { dimensionRegistryContainer = registryContainer.getCompound("minecraft:dimension_type") @@ -192,7 +194,7 @@ public class JoinGame implements MinecraftPacket { DimensionRegistry.fromGameData(dimensionRegistryContainer, version); this.dimensionRegistry = new DimensionRegistry(readData, levelNames); if (version.compareTo(ProtocolVersion.MINECRAFT_1_16_2) >= 0) { - CompoundBinaryTag currentDimDataTag = ProtocolUtils.readCompoundTag(buf); + CompoundBinaryTag currentDimDataTag = ProtocolUtils.readCompoundTag(buf, JOINGAME_READER); dimensionIdentifier = ProtocolUtils.readString(buf); this.currentDimensionData = DimensionData.decodeBaseCompoundTag(currentDimDataTag, version) .annotateWith(dimensionIdentifier, null); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Respawn.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Respawn.java index 9a5aee432..d8b0a674a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Respawn.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Respawn.java @@ -7,6 +7,7 @@ import com.velocitypowered.proxy.connection.registry.DimensionInfo; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; +import net.kyori.adventure.nbt.BinaryTagIO; import net.kyori.adventure.nbt.CompoundBinaryTag; public class Respawn implements MinecraftPacket { @@ -116,7 +117,7 @@ public class Respawn implements MinecraftPacket { String levelName = null; if (version.compareTo(ProtocolVersion.MINECRAFT_1_16) >= 0) { if (version.compareTo(ProtocolVersion.MINECRAFT_1_16_2) >= 0) { - CompoundBinaryTag dimDataTag = ProtocolUtils.readCompoundTag(buf); + CompoundBinaryTag dimDataTag = ProtocolUtils.readCompoundTag(buf, BinaryTagIO.reader()); dimensionIdentifier = ProtocolUtils.readString(buf); this.currentDimensionData = DimensionData.decodeBaseCompoundTag(dimDataTag, version) .annotateWith(dimensionIdentifier, null); From 7aed76ee3deab3b42051e11606173f1f7791174c Mon Sep 17 00:00:00 2001 From: 0x22 <0x22@futureclient.net> Date: Sat, 23 Jan 2021 18:35:14 -0500 Subject: [PATCH 43/69] Use keepalive queue instead of just the last keepalive. --- .../backend/BackendPlaySessionHandler.java | 2 +- .../backend/VelocityServerConnection.java | 22 +++++-------------- .../client/ClientPlaySessionHandler.java | 14 +++++++----- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 6dab7d8a5..667b1867e 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 @@ -83,7 +83,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { @Override public boolean handle(KeepAlive packet) { - serverConn.setLastPingId(packet.getRandomId()); + serverConn.getPendingPings().put(packet.getRandomId(), System.currentTimeMillis()); return false; // forwards on } 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 fe1144157..93958663e 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 @@ -29,7 +29,9 @@ import io.netty.buffer.Unpooled; import io.netty.channel.ChannelFutureListener; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.function.UnaryOperator; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @@ -44,8 +46,7 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, private boolean hasCompletedJoin = false; private boolean gracefulDisconnect = false; private BackendConnectionPhase connectionPhase = BackendConnectionPhases.UNKNOWN; - private long lastPingId; - private long lastPingSent; + private final Map pendingPings = new HashMap<>(); private @MonotonicNonNull DimensionRegistry activeDimensionRegistry; /** @@ -244,21 +245,8 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, return gracefulDisconnect; } - public long getLastPingId() { - return lastPingId; - } - - public long getLastPingSent() { - return lastPingSent; - } - - void setLastPingId(long lastPingId) { - this.lastPingId = lastPingId; - this.lastPingSent = System.currentTimeMillis(); - } - - public void resetLastPingId() { - this.lastPingId = -1; + public Map getPendingPings() { + return pendingPings; } /** 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 b41a5ae32..414e2520b 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 @@ -99,12 +99,14 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { @Override public boolean handle(KeepAlive packet) { VelocityServerConnection serverConnection = player.getConnectedServer(); - if (serverConnection != null && packet.getRandomId() == serverConnection.getLastPingId()) { - MinecraftConnection smc = serverConnection.getConnection(); - if (smc != null) { - player.setPing(System.currentTimeMillis() - serverConnection.getLastPingSent()); - smc.write(packet); - serverConnection.resetLastPingId(); + if (serverConnection != null) { + Long sentTime = serverConnection.getPendingPings().remove(packet.getRandomId()); + if (sentTime != null) { + MinecraftConnection smc = serverConnection.getConnection(); + if (smc != null) { + player.setPing(System.currentTimeMillis() - sentTime); + smc.write(packet); + } } } return true; From 4bc76b3012b5d8deeac4d007f30da44f649228ce Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 25 Jan 2021 01:50:56 -0500 Subject: [PATCH 44/69] Add JUL support to log4j Certain badly behaved plugins insist on logging with java.util.logging. Accommodate them. --- proxy/build.gradle | 1 + proxy/src/main/java/com/velocitypowered/proxy/Velocity.java | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/proxy/build.gradle b/proxy/build.gradle index 1ed87838e..25087f2e5 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -57,6 +57,7 @@ dependencies { implementation "org.apache.logging.log4j:log4j-core:${log4jVersion}" implementation "org.apache.logging.log4j:log4j-slf4j-impl:${log4jVersion}" implementation "org.apache.logging.log4j:log4j-iostreams:${log4jVersion}" + implementation "org.apache.logging.log4j:log4j-jul:${log4jVersion}" implementation 'net.sf.jopt-simple:jopt-simple:5.0.4' // command-line options implementation 'net.minecrell:terminalconsoleappender:1.2.0' diff --git a/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java b/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java index 9185a4f40..619cb4a01 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/Velocity.java @@ -8,9 +8,12 @@ import org.apache.logging.log4j.Logger; public class Velocity { - private static final Logger logger = LogManager.getLogger(Velocity.class); + private static final Logger logger; static { + System.setProperty("java.util.logging.manager", "org.apache.logging.log4j.jul.LogManager"); + logger = LogManager.getLogger(Velocity.class); + // We use BufferedImage for favicons, and on macOS this puts the Java application in the dock. // How inconvenient. Force AWT to work with its head chopped off. System.setProperty("java.awt.headless", "true"); From 8d71ea7135dc14fa9654fbd84f30e23b7693004d Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 25 Jan 2021 01:54:50 -0500 Subject: [PATCH 45/69] Also provide a "proper" java.util.logger logger to plugins. --- .../proxy/plugin/loader/java/VelocityPluginModule.java | 2 ++ 1 file changed, 2 insertions(+) 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 77e944ac8..2bfc33863 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 @@ -31,6 +31,8 @@ class VelocityPluginModule implements Module { binder.bind(description.getMainClass()).in(Scopes.SINGLETON); binder.bind(Logger.class).toInstance(LoggerFactory.getLogger(description.getId())); + binder.bind(java.util.logging.Logger.class) + .toInstance(java.util.logging.Logger.getLogger(description.getId())); binder.bind(Path.class).annotatedWith(DataDirectory.class) .toInstance(basePluginPath.resolve(description.getId())); binder.bind(PluginDescription.class).toInstance(description); From 959e75d16db352924e679fb5be545ee9b264fbd2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 25 Jan 2021 09:46:42 -0500 Subject: [PATCH 46/69] Revert "Also provide a "proper" java.util.logger logger to plugins." This reverts commit 8d71ea7135dc14fa9654fbd84f30e23b7693004d. Totally broken --- .../proxy/plugin/loader/java/VelocityPluginModule.java | 2 -- 1 file changed, 2 deletions(-) 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 2bfc33863..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 @@ -31,8 +31,6 @@ class VelocityPluginModule implements Module { binder.bind(description.getMainClass()).in(Scopes.SINGLETON); binder.bind(Logger.class).toInstance(LoggerFactory.getLogger(description.getId())); - binder.bind(java.util.logging.Logger.class) - .toInstance(java.util.logging.Logger.getLogger(description.getId())); binder.bind(Path.class).annotatedWith(DataDirectory.class) .toInstance(basePluginPath.resolve(description.getId())); binder.bind(PluginDescription.class).toInstance(description); From 5ceac16a821ea35572ff11412ace8929fd06e278 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 26 Jan 2021 12:33:35 -0500 Subject: [PATCH 47/69] Add upfront size checks for some packets. This is simply a further protection against certain attacks which send malformed packets to the proxy. --- .../proxy/protocol/MinecraftPacket.java | 8 +++++ .../protocol/netty/MinecraftDecoder.java | 30 ++++++++++++++++--- .../protocol/packet/EncryptionResponse.java | 15 +++++++++- .../proxy/protocol/packet/ServerLogin.java | 8 +++++ .../proxy/protocol/packet/StatusPing.java | 11 +++++++ .../proxy/protocol/packet/StatusRequest.java | 6 ++++ 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java index 11bed9297..6f1705765 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java @@ -11,4 +11,12 @@ public interface MinecraftPacket { void encode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion protocolVersion); boolean handle(MinecraftSessionHandler handler); + + default int expectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { + return -1; + } + + default int expectedMinLength(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { + return 0; + } } 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 9e442e493..aaf1a58e3 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 @@ -58,6 +58,8 @@ public class MinecraftDecoder extends ChannelInboundHandlerAdapter { ctx.fireChannelRead(buf); } else { try { + doLengthSanityChecks(buf, packet); + try { packet.decode(buf, direction, registry.version); } catch (Exception e) { @@ -65,7 +67,7 @@ public class MinecraftDecoder extends ChannelInboundHandlerAdapter { } if (buf.isReadable()) { - throw handleNotReadEnough(packet, packetId); + throw handleOverflow(packet, buf.readerIndex(), buf.writerIndex()); } ctx.fireChannelRead(packet); } finally { @@ -74,10 +76,30 @@ public class MinecraftDecoder extends ChannelInboundHandlerAdapter { } } - private Exception handleNotReadEnough(MinecraftPacket packet, int packetId) { + private void doLengthSanityChecks(ByteBuf buf, MinecraftPacket packet) throws Exception { + int expectedMinLen = packet.expectedMinLength(buf, direction, registry.version); + int expectedMaxLen = packet.expectedMaxLength(buf, direction, registry.version); + if (expectedMaxLen != -1 && buf.readableBytes() > expectedMaxLen) { + throw handleOverflow(packet, expectedMaxLen, buf.readableBytes()); + } + if (buf.readableBytes() < expectedMinLen) { + throw handleUnderflow(packet, expectedMaxLen, buf.readableBytes()); + } + } + + private Exception handleOverflow(MinecraftPacket packet, int expected, int actual) { if (DEBUG) { - return new CorruptedFrameException("Did not read full packet for " + packet.getClass() + " " - + getExtraConnectionDetail(packetId)); + return new CorruptedFrameException("Packet sent for " + packet.getClass() + " was too " + + "big (expected " + expected + " bytes, got " + actual + " bytes)"); + } else { + return DECODE_FAILED; + } + } + + private Exception handleUnderflow(MinecraftPacket packet, int expected, int actual) { + if (DEBUG) { + return new CorruptedFrameException("Packet sent for " + packet.getClass() + " was too " + + "small (expected " + expected + " bytes, got " + actual + " bytes)"); } else { return DECODE_FAILED; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/EncryptionResponse.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/EncryptionResponse.java index 5022f5c18..b59b46c61 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/EncryptionResponse.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/EncryptionResponse.java @@ -6,6 +6,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.ProtocolUtils.Direction; import io.netty.buffer.ByteBuf; import java.util.Arrays; @@ -33,7 +34,7 @@ public class EncryptionResponse implements MinecraftPacket { @Override public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { if (version.compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { - this.sharedSecret = ProtocolUtils.readByteArray(buf, 256); + this.sharedSecret = ProtocolUtils.readByteArray(buf, 128); this.verifyToken = ProtocolUtils.readByteArray(buf, 128); } else { this.sharedSecret = ProtocolUtils.readByteArray17(buf); @@ -56,4 +57,16 @@ public class EncryptionResponse implements MinecraftPacket { public boolean handle(MinecraftSessionHandler handler) { return handler.handle(this); } + + @Override + public int expectedMaxLength(ByteBuf buf, Direction direction, ProtocolVersion version) { + // It turns out these come out to the same length, whether we're talking >=1.8 or not. + // The length prefix always winds up being 2 bytes. + return 260; + } + + @Override + public int expectedMinLength(ByteBuf buf, Direction direction, ProtocolVersion version) { + return expectedMaxLength(buf, direction, version); + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/ServerLogin.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/ServerLogin.java index ebd7489e9..cef02eb0c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/ServerLogin.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/ServerLogin.java @@ -5,6 +5,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.ProtocolUtils.Direction; import com.velocitypowered.proxy.util.except.QuietDecoderException; import io.netty.buffer.ByteBuf; import org.checkerframework.checker.nullness.qual.Nullable; @@ -52,6 +53,13 @@ public class ServerLogin implements MinecraftPacket { ProtocolUtils.writeString(buf, username); } + @Override + public int expectedMaxLength(ByteBuf buf, Direction direction, ProtocolVersion version) { + // Accommodate the rare (but likely malicious) use of UTF-8 usernames, since it is technically + // legal on the protocol level. + return 1 + (16 * 4); + } + @Override public boolean handle(MinecraftSessionHandler handler) { return handler.handle(this); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusPing.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusPing.java index 7d61e6996..16a6fcd73 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusPing.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusPing.java @@ -4,6 +4,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.ProtocolUtils.Direction; import io.netty.buffer.ByteBuf; public class StatusPing implements MinecraftPacket { @@ -24,4 +25,14 @@ public class StatusPing implements MinecraftPacket { public boolean handle(MinecraftSessionHandler handler) { return handler.handle(this); } + + @Override + public int expectedMaxLength(ByteBuf buf, Direction direction, ProtocolVersion version) { + return 8; + } + + @Override + public int expectedMinLength(ByteBuf buf, Direction direction, ProtocolVersion version) { + return 8; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusRequest.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusRequest.java index 7d1b9bc57..e0aaef5e2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusRequest.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/StatusRequest.java @@ -4,6 +4,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.ProtocolUtils.Direction; import io.netty.buffer.ByteBuf; public class StatusRequest implements MinecraftPacket { @@ -33,4 +34,9 @@ public class StatusRequest implements MinecraftPacket { public boolean handle(MinecraftSessionHandler handler) { return handler.handle(this); } + + @Override + public int expectedMaxLength(ByteBuf buf, Direction direction, ProtocolVersion version) { + return 0; + } } From 0c90e94e85a521afd56ac83eb47b111675c827d2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 26 Jan 2021 19:50:43 -0500 Subject: [PATCH 48/69] fix --- .../proxy/connection/client/LoginSessionHandler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java index 2874ba137..f342cc15e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java @@ -36,6 +36,7 @@ import io.netty.buffer.ByteBuf; import java.net.InetSocketAddress; import java.security.GeneralSecurityException; import java.security.KeyPair; +import java.security.MessageDigest; import java.util.Arrays; import java.util.Optional; import java.util.UUID; @@ -90,7 +91,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { try { KeyPair serverKeyPair = server.getServerKeyPair(); byte[] decryptedVerifyToken = decryptRsa(serverKeyPair, packet.getVerifyToken()); - if (!Arrays.equals(verify, decryptedVerifyToken)) { + if (!MessageDigest.isEqual(verify, decryptedVerifyToken)) { throw new IllegalStateException("Unable to successfully decrypt the verification token."); } From db959b6051c0dc0622869610cacd61c695490e5b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 26 Jan 2021 19:55:26 -0500 Subject: [PATCH 49/69] Add GitHub Actions to replace Travis --- .github/workflows/gradle.yml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/workflows/gradle.yml diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml new file mode 100644 index 000000000..a4efca4f8 --- /dev/null +++ b/.github/workflows/gradle.yml @@ -0,0 +1,32 @@ +# This workflow will build a Java project with Gradle +# For more information see: https://help.github.com/actions/language-and-framework-guides/building-and-testing-java-with-gradle + +name: Java CI with Gradle + +on: [push, pull_request] + +jobs: + build-1.8: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up JDK 1.8 + uses: actions/setup-java@v1 + with: + java-version: 1.8 + - name: Grant execute permission for gradlew + run: chmod +x gradlew + - name: Build with Gradle + run: ./gradlew build + build-11: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up JDK 11 + uses: actions/setup-java@v1 + with: + java-version: 11 + - name: Grant execute permission for gradlew + run: chmod +x gradlew + - name: Build with Gradle + run: ./gradlew build From 77656615bbdab9de77b5966985b8f6884c9dc461 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 26 Jan 2021 19:56:17 -0500 Subject: [PATCH 50/69] Fix job name --- .github/workflows/gradle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index a4efca4f8..da4514613 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -6,7 +6,7 @@ name: Java CI with Gradle on: [push, pull_request] jobs: - build-1.8: + build-8: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 From b982c5b6ed63e0073ba9efb0c8fccb7632fe60f0 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 26 Jan 2021 19:59:23 -0500 Subject: [PATCH 51/69] Fix Checkstyle issues --- .../com/velocitypowered/proxy/protocol/MinecraftPacket.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java index 6f1705765..75cbe5221 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java @@ -12,7 +12,7 @@ public interface MinecraftPacket { boolean handle(MinecraftSessionHandler handler); - default int expectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { + default int expectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction,ProtocolVersion version) { return -1; } From 4219bf7b09d5707439900563a86c674cb011d1a5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 26 Jan 2021 20:01:25 -0500 Subject: [PATCH 52/69] *Actually* fix Checkstyle issues --- .../com/velocitypowered/proxy/protocol/MinecraftPacket.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java index 75cbe5221..e7baaecb1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/MinecraftPacket.java @@ -12,11 +12,13 @@ public interface MinecraftPacket { boolean handle(MinecraftSessionHandler handler); - default int expectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction,ProtocolVersion version) { + default int expectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction, + ProtocolVersion version) { return -1; } - default int expectedMinLength(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { + default int expectedMinLength(ByteBuf buf, ProtocolUtils.Direction direction, + ProtocolVersion version) { return 0; } } From 4f6d238b39609933a71c622c592f778c4341a754 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 29 Jan 2021 17:56:50 -0500 Subject: [PATCH 53/69] Strictly limit the hostname size and limit it to ASCII characters only --- .../proxy/protocol/ProtocolUtils.java | 20 ++++++++++++++++++- .../proxy/protocol/packet/Handshake.java | 8 +++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index 2c4f50a99..e0259b033 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -102,7 +102,7 @@ public enum ProtocolUtils { } /** - * Reads a VarInt length-prefixed string from the {@code buf}, making sure to not go over + * Reads a VarInt length-prefixed UTF-8 string from the {@code buf}, making sure to not go over * {@code cap} size. * @param buf the buffer to read from * @param cap the maximum size of the string, in UTF-8 character length @@ -113,6 +113,24 @@ public enum ProtocolUtils { return readString(buf, cap, length); } + /** + * Reads a VarInt length-prefixed ASCII string from the {@code buf}, making sure to not go over + * {@code cap} size. This method is specialized for select parts of the Minecraft protocol where + * ASCII characters are guaranteed to be used. + * + * @param buf the buffer to read from + * @param cap the maximum size of the string, in UTF-8 character length + * @return the decoded string + */ + public static String readAsciiString(ByteBuf buf, int cap) { + int length = readVarInt(buf); + checkFrame(length >= 0, "Got a negative-length string (%s)", length); + checkFrame(length <= cap, "Bad string size (got %s, maximum is %s)", length, cap); + String str = buf.toString(buf.readerIndex(), length, StandardCharsets.US_ASCII); + buf.skipBytes(length); + return str; + } + private static String readString(ByteBuf buf, int cap, int length) { checkFrame(length >= 0, "Got a negative-length string (%s)", length); // `cap` is interpreted as a UTF-8 character length. To cover the full Unicode plane, we must diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java index cf64b7b93..a1c902057 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java @@ -1,13 +1,19 @@ package com.velocitypowered.proxy.protocol.packet; +import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN; + import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; +import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; public class Handshake implements MinecraftPacket { + // This size was chosen to ensure Forge clients can still connect even with very long hostnames. + // While DNS technically allows any character to be used, in practice ASCII is used. + private static final int MAXIMUM_HOSTNAME_LENGTH = 255 + HANDSHAKE_HOSTNAME_TOKEN.length() + 1; private ProtocolVersion protocolVersion; private String serverAddress = ""; private int port; @@ -59,7 +65,7 @@ public class Handshake implements MinecraftPacket { public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion ignored) { int realProtocolVersion = ProtocolUtils.readVarInt(buf); this.protocolVersion = ProtocolVersion.getProtocolVersion(realProtocolVersion); - this.serverAddress = ProtocolUtils.readString(buf); + this.serverAddress = ProtocolUtils.readAsciiString(buf, MAXIMUM_HOSTNAME_LENGTH); this.port = buf.readUnsignedShort(); this.nextStatus = ProtocolUtils.readVarInt(buf); } From 501853e807a00b15ad9a181dd76a43e4f8cadd1c Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 29 Jan 2021 17:59:00 -0500 Subject: [PATCH 54/69] Fix Checkstyle issue for the umpteenth time... --- .../proxy/protocol/ProtocolUtils.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index e0259b033..7da858d9f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -97,22 +97,6 @@ public enum ProtocolUtils { } } - public static String readString(ByteBuf buf) { - return readString(buf, DEFAULT_MAX_STRING_SIZE); - } - - /** - * Reads a VarInt length-prefixed UTF-8 string from the {@code buf}, making sure to not go over - * {@code cap} size. - * @param buf the buffer to read from - * @param cap the maximum size of the string, in UTF-8 character length - * @return the decoded string - */ - public static String readString(ByteBuf buf, int cap) { - int length = readVarInt(buf); - return readString(buf, cap, length); - } - /** * Reads a VarInt length-prefixed ASCII string from the {@code buf}, making sure to not go over * {@code cap} size. This method is specialized for select parts of the Minecraft protocol where @@ -131,6 +115,22 @@ public enum ProtocolUtils { return str; } + public static String readString(ByteBuf buf) { + return readString(buf, DEFAULT_MAX_STRING_SIZE); + } + + /** + * Reads a VarInt length-prefixed UTF-8 string from the {@code buf}, making sure to not go over + * {@code cap} size. + * @param buf the buffer to read from + * @param cap the maximum size of the string, in UTF-8 character length + * @return the decoded string + */ + public static String readString(ByteBuf buf, int cap) { + int length = readVarInt(buf); + return readString(buf, cap, length); + } + private static String readString(ByteBuf buf, int cap, int length) { checkFrame(length >= 0, "Got a negative-length string (%s)", length); // `cap` is interpreted as a UTF-8 character length. To cover the full Unicode plane, we must From 4f80d2b261d0a90d9076474e9e5d55f63fd171d9 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 29 Jan 2021 23:08:14 -0500 Subject: [PATCH 55/69] Revert strict handshake hostname checks These will break TCPShield and Cosmic Guard plugins. Sad as this was a worthwhile mitigation. --- .../proxy/protocol/ProtocolUtils.java | 20 +------------------ .../proxy/protocol/packet/Handshake.java | 8 +------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index 7da858d9f..2c4f50a99 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -97,30 +97,12 @@ public enum ProtocolUtils { } } - /** - * Reads a VarInt length-prefixed ASCII string from the {@code buf}, making sure to not go over - * {@code cap} size. This method is specialized for select parts of the Minecraft protocol where - * ASCII characters are guaranteed to be used. - * - * @param buf the buffer to read from - * @param cap the maximum size of the string, in UTF-8 character length - * @return the decoded string - */ - public static String readAsciiString(ByteBuf buf, int cap) { - int length = readVarInt(buf); - checkFrame(length >= 0, "Got a negative-length string (%s)", length); - checkFrame(length <= cap, "Bad string size (got %s, maximum is %s)", length, cap); - String str = buf.toString(buf.readerIndex(), length, StandardCharsets.US_ASCII); - buf.skipBytes(length); - return str; - } - public static String readString(ByteBuf buf) { return readString(buf, DEFAULT_MAX_STRING_SIZE); } /** - * Reads a VarInt length-prefixed UTF-8 string from the {@code buf}, making sure to not go over + * Reads a VarInt length-prefixed string from the {@code buf}, making sure to not go over * {@code cap} size. * @param buf the buffer to read from * @param cap the maximum size of the string, in UTF-8 character length diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java index a1c902057..cf64b7b93 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java @@ -1,19 +1,13 @@ package com.velocitypowered.proxy.protocol.packet; -import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN; - import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; -import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; public class Handshake implements MinecraftPacket { - // This size was chosen to ensure Forge clients can still connect even with very long hostnames. - // While DNS technically allows any character to be used, in practice ASCII is used. - private static final int MAXIMUM_HOSTNAME_LENGTH = 255 + HANDSHAKE_HOSTNAME_TOKEN.length() + 1; private ProtocolVersion protocolVersion; private String serverAddress = ""; private int port; @@ -65,7 +59,7 @@ public class Handshake implements MinecraftPacket { public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion ignored) { int realProtocolVersion = ProtocolUtils.readVarInt(buf); this.protocolVersion = ProtocolVersion.getProtocolVersion(realProtocolVersion); - this.serverAddress = ProtocolUtils.readAsciiString(buf, MAXIMUM_HOSTNAME_LENGTH); + this.serverAddress = ProtocolUtils.readString(buf); this.port = buf.readUnsignedShort(); this.nextStatus = ProtocolUtils.readVarInt(buf); } From b88c573eb11839a95bea1af947b0c59a5956368b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 1 Feb 2021 16:17:02 -0500 Subject: [PATCH 56/69] Correctly forward the player's virtual host to the remote server. --- .../proxy/connection/backend/VelocityServerConnection.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 93958663e..cf70bf5ad 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 @@ -101,7 +101,9 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation, // separated by \0 (the null byte). In order, you send the original host, the player's IP, their // UUID (undashed), and if you are in online-mode, their login properties (from Mojang). StringBuilder data = new StringBuilder() - .append(registeredServer.getServerInfo().getAddress().getHostString()) + .append(proxyPlayer.getVirtualHost() + .orElseGet(() -> registeredServer.getServerInfo().getAddress()) + .getHostString()) .append('\0') .append(proxyPlayer.getRemoteAddress().getHostString()) .append('\0') From 7e42c5b2e7dbdaae1a56197b0406a6378b0a3749 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 7 Feb 2021 15:19:59 -0500 Subject: [PATCH 57/69] Turn some APIs into no-ops if they apply to clients without the relevant client functionality. --- .../connection/client/ConnectedPlayer.java | 80 +++++++++++-------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 55d8df8b4..949243e17 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 @@ -330,54 +330,66 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { public void sendPlayerListHeaderAndFooter(final Component header, final Component footer) { this.playerListHeader = Objects.requireNonNull(header, "header"); this.playerListFooter = Objects.requireNonNull(footer, "footer"); - this.connection.write(HeaderAndFooter.create(header, footer, this.getProtocolVersion())); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + this.connection.write(HeaderAndFooter.create(header, footer, this.getProtocolVersion())); + } } @Override public void showTitle(net.kyori.adventure.title.@NonNull Title title) { - GsonComponentSerializer serializer = ProtocolUtils.getJsonChatSerializer(this - .getProtocolVersion()); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + GsonComponentSerializer serializer = ProtocolUtils.getJsonChatSerializer(this + .getProtocolVersion()); - TitlePacket titlePkt = new TitlePacket(); - titlePkt.setAction(TitlePacket.SET_TITLE); - titlePkt.setComponent(serializer.serialize(title.title())); - connection.delayedWrite(titlePkt); + TitlePacket titlePkt = new TitlePacket(); + titlePkt.setAction(TitlePacket.SET_TITLE); + titlePkt.setComponent(serializer.serialize(title.title())); + connection.delayedWrite(titlePkt); - TitlePacket subtitlePkt = new TitlePacket(); - subtitlePkt.setAction(TitlePacket.SET_SUBTITLE); - subtitlePkt.setComponent(serializer.serialize(title.subtitle())); - connection.delayedWrite(subtitlePkt); + TitlePacket subtitlePkt = new TitlePacket(); + subtitlePkt.setAction(TitlePacket.SET_SUBTITLE); + subtitlePkt.setComponent(serializer.serialize(title.subtitle())); + connection.delayedWrite(subtitlePkt); - TitlePacket timesPkt = TitlePacket.timesForProtocolVersion(this.getProtocolVersion()); - net.kyori.adventure.title.Title.Times times = title.times(); - if (times != null) { - timesPkt.setFadeIn((int) DurationUtils.toTicks(times.fadeIn())); - timesPkt.setStay((int) DurationUtils.toTicks(times.stay())); - timesPkt.setFadeOut((int) DurationUtils.toTicks(times.fadeOut())); + TitlePacket timesPkt = TitlePacket.timesForProtocolVersion(this.getProtocolVersion()); + net.kyori.adventure.title.Title.Times times = title.times(); + if (times != null) { + timesPkt.setFadeIn((int) DurationUtils.toTicks(times.fadeIn())); + timesPkt.setStay((int) DurationUtils.toTicks(times.stay())); + timesPkt.setFadeOut((int) DurationUtils.toTicks(times.fadeOut())); + } + connection.delayedWrite(timesPkt); + + connection.flush(); } - connection.delayedWrite(timesPkt); - - connection.flush(); } @Override public void clearTitle() { - connection.write(TitlePacket.hideForProtocolVersion(this.getProtocolVersion())); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + connection.write(TitlePacket.hideForProtocolVersion(this.getProtocolVersion())); + } } @Override public void resetTitle() { - connection.write(TitlePacket.resetForProtocolVersion(this.getProtocolVersion())); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + connection.write(TitlePacket.resetForProtocolVersion(this.getProtocolVersion())); + } } @Override public void hideBossBar(@NonNull BossBar bar) { - this.server.getBossBarManager().removeBossBar(this, bar); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_9) >= 0) { + this.server.getBossBarManager().removeBossBar(this, bar); + } } @Override public void showBossBar(@NonNull BossBar bar) { - this.server.getBossBarManager().addBossBar(this, bar); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_9) >= 0) { + this.server.getBossBarManager().addBossBar(this, bar); + } } @Override @@ -835,10 +847,12 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { public void sendResourcePack(String url) { Preconditions.checkNotNull(url, "url"); - ResourcePackRequest request = new ResourcePackRequest(); - request.setUrl(url); - request.setHash(""); - connection.write(request); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + ResourcePackRequest request = new ResourcePackRequest(); + request.setUrl(url); + request.setHash(""); + connection.write(request); + } } @Override @@ -847,10 +861,12 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { Preconditions.checkNotNull(hash, "hash"); Preconditions.checkArgument(hash.length == 20, "Hash length is not 20"); - ResourcePackRequest request = new ResourcePackRequest(); - request.setUrl(url); - request.setHash(ByteBufUtil.hexDump(hash)); - connection.write(request); + if (this.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_8) >= 0) { + ResourcePackRequest request = new ResourcePackRequest(); + request.setUrl(url); + request.setHash(ByteBufUtil.hexDump(hash)); + connection.write(request); + } } /** From d47b339908c546c52e5c08576e27322a48404bc0 Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Tue, 9 Feb 2021 13:51:43 -0500 Subject: [PATCH 58/69] Add PlayerPluginMessageRegisterEvent --- .../PlayerPluginMessageRegisterEvent.java | 37 +++++++++++++++++++ .../client/ClientPlaySessionHandler.java | 6 ++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java diff --git a/api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java new file mode 100644 index 000000000..0683661f3 --- /dev/null +++ b/api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java @@ -0,0 +1,37 @@ +package com.velocitypowered.api.event.player; + +import com.google.common.base.Preconditions; +import com.velocitypowered.api.proxy.Player; + +import java.util.List; + +/** + * This event is fired when a client ({@link Player}) sends a plugin message through the + * register channel. + */ +public final class PlayerPluginMessageRegisterEvent { + + private final Player player; + private final List channels; + + public PlayerPluginMessageRegisterEvent(Player player, List channels) { + this.player = Preconditions.checkNotNull(player, "player"); + this.channels = Preconditions.checkNotNull(channels, "channels"); + } + + public Player getPlayer() { + return player; + } + + public List getChannels() { + return channels; + } + + @Override + public String toString() { + return "PlayerPluginMessageRegisterEvent{" + + "player=" + player + + ", channels=" + channels + + '}'; + } +} 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 414e2520b..0df9b7594 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 @@ -8,6 +8,7 @@ import static com.velocitypowered.proxy.protocol.util.PluginMessageUtil.construc import com.velocitypowered.api.event.command.CommandExecuteEvent.CommandResult; import com.velocitypowered.api.event.connection.PluginMessageEvent; import com.velocitypowered.api.event.player.PlayerChatEvent; +import com.velocitypowered.api.event.player.PlayerPluginMessageRegisterEvent; import com.velocitypowered.api.event.player.PlayerResourcePackStatusEvent; import com.velocitypowered.api.event.player.TabCompleteEvent; import com.velocitypowered.api.network.ProtocolVersion; @@ -190,7 +191,10 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { logger.warn("A plugin message was received while the backend server was not " + "ready. Channel: {}. Packet discarded.", packet.getChannel()); } else if (PluginMessageUtil.isRegister(packet)) { - player.getKnownChannels().addAll(PluginMessageUtil.getChannels(packet)); + List channels = PluginMessageUtil.getChannels(packet); + player.getKnownChannels().addAll(channels); + server.getEventManager().fireAndForget(new PlayerPluginMessageRegisterEvent(player, + channels)); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isUnregister(packet)) { player.getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet)); From fb879cb498c108daa4f85f18081855d4382688cc Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Tue, 9 Feb 2021 14:13:33 -0500 Subject: [PATCH 59/69] Apply initial requested changes --- ...ent.java => PlayerChannelRegisterEvent.java} | 11 ++++++----- .../client/ClientPlaySessionHandler.java | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 8 deletions(-) rename api/src/main/java/com/velocitypowered/api/event/player/{PlayerPluginMessageRegisterEvent.java => PlayerChannelRegisterEvent.java} (66%) diff --git a/api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/PlayerChannelRegisterEvent.java similarity index 66% rename from api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java rename to api/src/main/java/com/velocitypowered/api/event/player/PlayerChannelRegisterEvent.java index 0683661f3..d9c790eed 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/PlayerPluginMessageRegisterEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/player/PlayerChannelRegisterEvent.java @@ -2,6 +2,7 @@ package com.velocitypowered.api.event.player; import com.google.common.base.Preconditions; import com.velocitypowered.api.proxy.Player; +import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import java.util.List; @@ -9,12 +10,12 @@ import java.util.List; * This event is fired when a client ({@link Player}) sends a plugin message through the * register channel. */ -public final class PlayerPluginMessageRegisterEvent { +public final class PlayerChannelRegisterEvent { private final Player player; - private final List channels; + private final List channels; - public PlayerPluginMessageRegisterEvent(Player player, List channels) { + public PlayerChannelRegisterEvent(Player player, List channels) { this.player = Preconditions.checkNotNull(player, "player"); this.channels = Preconditions.checkNotNull(channels, "channels"); } @@ -23,13 +24,13 @@ public final class PlayerPluginMessageRegisterEvent { return player; } - public List getChannels() { + public List getChannels() { return channels; } @Override public String toString() { - return "PlayerPluginMessageRegisterEvent{" + return "PlayerChannelRegisterEvent{" + "player=" + player + ", channels=" + channels + '}'; 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 0df9b7594..2f0375575 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 @@ -5,14 +5,17 @@ import static com.velocitypowered.api.network.ProtocolVersion.MINECRAFT_1_16; import static com.velocitypowered.api.network.ProtocolVersion.MINECRAFT_1_8; import static com.velocitypowered.proxy.protocol.util.PluginMessageUtil.constructChannelsPacket; +import com.google.common.collect.ImmutableList; import com.velocitypowered.api.event.command.CommandExecuteEvent.CommandResult; import com.velocitypowered.api.event.connection.PluginMessageEvent; +import com.velocitypowered.api.event.player.PlayerChannelRegisterEvent; import com.velocitypowered.api.event.player.PlayerChatEvent; -import com.velocitypowered.api.event.player.PlayerPluginMessageRegisterEvent; import com.velocitypowered.api.event.player.PlayerResourcePackStatusEvent; import com.velocitypowered.api.event.player.TabCompleteEvent; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.api.proxy.messages.ChannelIdentifier; +import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; +import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.ConnectionTypes; import com.velocitypowered.proxy.connection.MinecraftConnection; @@ -193,8 +196,16 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { } else if (PluginMessageUtil.isRegister(packet)) { List channels = PluginMessageUtil.getChannels(packet); player.getKnownChannels().addAll(channels); - server.getEventManager().fireAndForget(new PlayerPluginMessageRegisterEvent(player, - channels)); + List channelIdentifiers = new ArrayList<>(); + for (String channel : channels) { + try { + channelIdentifiers.add(MinecraftChannelIdentifier.from(channel)); + } catch (IllegalArgumentException e) { + channelIdentifiers.add(new LegacyChannelIdentifier(channel)); + } + } + server.getEventManager().fireAndForget(new PlayerChannelRegisterEvent(player, + ImmutableList.copyOf(channelIdentifiers))); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isUnregister(packet)) { player.getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet)); From a6a9d1e0fbacdd7eac857b0d2a369886847de4ec Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 9 Feb 2021 14:25:12 -0500 Subject: [PATCH 60/69] Forward slashes are allowed in channel IDs Fixes an issue where the Fabric registry sync packet would not be allowed by Velocity. --- .../api/proxy/messages/MinecraftChannelIdentifier.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifier.java b/api/src/main/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifier.java index 01a582035..2cb66007a 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifier.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifier.java @@ -13,7 +13,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; */ public final class MinecraftChannelIdentifier implements ChannelIdentifier { - private static final Pattern VALID_IDENTIFIER_REGEX = Pattern.compile("[a-z0-9\\-_]*"); + private static final Pattern VALID_IDENTIFIER_REGEX = Pattern.compile("[a-z0-9/\\-_]*"); private final String namespace; private final String name; From 3aee47166f495265ae7bc77a67076dd6ccd3fca5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 9 Feb 2021 14:36:10 -0500 Subject: [PATCH 61/69] Allow clearing favicon in ServerPing builder. --- .../com/velocitypowered/api/proxy/server/ServerPing.java | 5 +++++ .../api/proxy/messages/MinecraftChannelIdentifierTest.java | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java b/api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java index dcb44f7ff..dc9b7fc40 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 @@ -262,6 +262,11 @@ public final class ServerPing { return this; } + public Builder clearFavicon() { + this.favicon = null; + return this; + } + /** * Uses the information from this builder to create a new {@link ServerPing} instance. The * builder can be re-used after this event has been called. diff --git a/api/src/test/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifierTest.java b/api/src/test/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifierTest.java index dfa738198..e7f02e95a 100644 --- a/api/src/test/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifierTest.java +++ b/api/src/test/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifierTest.java @@ -35,6 +35,11 @@ class MinecraftChannelIdentifierTest { assertEquals(expected, MinecraftChannelIdentifier.from("velocity:test")); } + @Test + void createAllowsSlashes() { + create("velocity", "test/test2"); + } + @Test void fromIdentifierThrowsOnBadValues() { assertAll( From 7a713e937972852166468df2679f73f600e3d5f2 Mon Sep 17 00:00:00 2001 From: Bastian Date: Tue, 26 Jan 2021 12:58:16 +0100 Subject: [PATCH 62/69] Update bStats and migrate to new config file This commit simplifies the Metrics class by using the new base module from bStats. It also migrates to the new bStats config file that will be used by plugins that integrate bStats. If a user disabled bStats in the old config file, this setting will be copied to the new config file. --- proxy/build.gradle | 4 + .../com/velocitypowered/proxy/Metrics.java | 693 +++--------------- .../velocitypowered/proxy/VelocityServer.java | 2 - .../proxy/config/VelocityConfiguration.java | 31 +- .../src/main/resources/default-velocity.toml | 13 - 5 files changed, 104 insertions(+), 639 deletions(-) diff --git a/proxy/build.gradle b/proxy/build.gradle index 25087f2e5..254d4bdea 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -76,6 +76,8 @@ dependencies { implementation 'com.electronwill.night-config:toml:3.6.3' + implementation 'org.bstats:bstats-base:2.2.0' + compileOnly 'com.github.spotbugs:spotbugs-annotations:4.1.2' testImplementation "org.junit.jupiter:junit-jupiter-api:${junitVersion}" @@ -126,6 +128,8 @@ shadowJar { exclude 'it/unimi/dsi/fastutil/objects/*Reference*' exclude 'it/unimi/dsi/fastutil/shorts/**' exclude 'org/checkerframework/checker/**' + + relocate 'org.bstats', 'com.velocitypowered.proxy.bstats' } artifacts { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java b/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java index d8d635afa..5728ca673 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/Metrics.java @@ -1,85 +1,67 @@ package com.velocitypowered.proxy; -import com.google.gson.JsonArray; -import com.google.gson.JsonObject; import com.velocitypowered.proxy.config.VelocityConfiguration; -import io.netty.handler.codec.http.HttpHeaderNames; -import java.io.BufferedWriter; -import java.io.ByteArrayOutputStream; + +import java.io.File; import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.Writer; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; +import java.nio.file.Paths; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.Timer; -import java.util.TimerTask; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ThreadLocalRandom; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.zip.GZIPOutputStream; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.asynchttpclient.ListenableFuture; -import org.asynchttpclient.Response; +import org.bstats.MetricsBase; +import org.bstats.charts.CustomChart; +import org.bstats.charts.DrilldownPie; +import org.bstats.charts.SimplePie; +import org.bstats.charts.SingleLineChart; +import org.bstats.config.MetricsConfig; +import org.bstats.json.JsonObjectBuilder; -/** - * bStats collects some data for plugin authors. - *

- * Check out https://bStats.org/ to learn more about bStats! - */ public class Metrics { - private static final int ONE_MINUTE_MS = 60_000; + private MetricsBase metricsBase; - // The version of this bStats class - private static final int B_STATS_METRICS_REVISION = 2; + private Metrics(Logger logger, int serviceId, boolean defaultEnabled) { + File configFile = Paths.get("plugins").resolve("bStats").resolve("config.txt").toFile(); + MetricsConfig config; + try { + config = new MetricsConfig(configFile, defaultEnabled); + } catch (IOException e) { + logger.error("Failed to create bStats config", e); + return; + } - // The url to which the data is sent - private static final String URL = "https://bstats.org/submitData/server-implementation"; + metricsBase = new MetricsBase( + "server-implementation", + config.getServerUUID(), + serviceId, + config.isEnabled(), + this::appendPlatformData, + jsonObjectBuilder -> { /* NOP */ }, + null, + () -> true, + logger::warn, + logger::info, + config.isLogErrorsEnabled(), + config.isLogSentDataEnabled(), + config.isLogResponseStatusTextEnabled() + ); - // The logger for the failed requests - private static final Logger logger = LogManager.getLogger(Metrics.class); - - // Should failed requests be logged? - private boolean logFailedRequests = false; - - // 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; - - // A list with all custom charts - private final List charts = new ArrayList<>(); - - private final VelocityServer server; - - /** - * 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, int pluginId, String serverUuid, boolean logFailedRequests, - VelocityServer server) { - this.name = name; - this.pluginId = pluginId; - this.serverUuid = serverUuid; - this.logFailedRequests = logFailedRequests; - this.server = server; - - // Start submitting the data - startSubmitting(); + if (!config.didExistBefore()) { + // Send an info message when the bStats config file gets created for the first time + logger.info("Velocity and some of its plugins collect metrics" + + " and send them to bStats (https://bStats.org)."); + logger.info("bStats collects some basic information for plugin" + + " authors, like how many people use"); + logger.info("their plugin and their total player count." + + " It's recommended to keep bStats enabled, but"); + logger.info("if you're not comfortable with this, you can opt-out" + + " by editing the config.txt file in"); + logger.info("the '/plugins/bStats/' folder and setting enabled to false."); + } } /** @@ -88,546 +70,69 @@ public class Metrics { * @param chart The chart to add. */ public void addCustomChart(CustomChart chart) { - if (chart == null) { - throw new IllegalArgumentException("Chart cannot be null!"); - } - charts.add(chart); + metricsBase.addCustomChart(chart); } - /** - * Starts the Scheduler which submits our data every 30 minutes. - */ - private void startSubmitting() { - final Timer timer = new Timer(true); - long initialDelay = ONE_MINUTE_MS * 3 + (((long) (ThreadLocalRandom.current().nextDouble() * 2 - * ONE_MINUTE_MS))); - timer.scheduleAtFixedRate(new TimerTask() { - @Override - public void run() { - submitData(); - } - }, initialDelay, 1000 * 60 * 30); - // Submit the data every 30 minutes, first time after 5 minutes to give other plugins enough - // time to start. - // - // WARNING: Changing the frequency has no effect but your plugin WILL be blocked/deleted! - // WARNING: Just don't do it! - } - - /** - * Gets the plugin specific data. - * - * @return The plugin specific data. - */ - private JsonObject getPluginData() { - 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 - JsonObject chart = customChart.getRequestJsonObject(); - if (chart == null) { // If the chart is null, we skip it - continue; - } - customCharts.add(chart); - } - data.add("customCharts", customCharts); - - return data; - } - - /** - * Gets the server specific data. - * - * @return The server specific data. - */ - private JsonObject getServerData() { - // OS specific data - String osName = System.getProperty("os.name"); - String osArch = System.getProperty("os.arch"); - String osVersion = System.getProperty("os.version"); - int coreCount = Runtime.getRuntime().availableProcessors(); - - JsonObject data = new JsonObject(); - - data.addProperty("serverUUID", serverUuid); - - data.addProperty("osName", osName); - data.addProperty("osArch", osArch); - data.addProperty("osVersion", osVersion); - data.addProperty("coreCount", coreCount); - - return data; - } - - /** - * Collects the data and sends it afterwards. - */ - private void submitData() { - final JsonObject data = getServerData(); - - JsonArray pluginData = new JsonArray(); - pluginData.add(getPluginData()); - data.add("plugins", pluginData); - - try { - // We are still in the Thread of the timer, so nothing get blocked :) - sendData(data); - } catch (Exception e) { - // Something went wrong! :( - if (logFailedRequests) { - logger.warn("Could not submit stats of {}", name, e); - } - } - } - - /** - * Sends the data to the bStats server. - * - * @param data The data to send. - * @throws Exception If the request failed. - */ - private void sendData(JsonObject data) throws Exception { - if (data == null) { - throw new IllegalArgumentException("Data cannot be null!"); - } - - // Compress the data to save bandwidth - ListenableFuture future = server.getAsyncHttpClient() - .preparePost(URL) - .addHeader(HttpHeaderNames.CONTENT_ENCODING, "gzip") - .addHeader(HttpHeaderNames.ACCEPT, "application/json") - .addHeader(HttpHeaderNames.CONTENT_TYPE, "application/json") - .setBody(createResponseBody(data)) - .execute(); - future.addListener(() -> { - if (logFailedRequests) { - try { - Response r = future.get(); - if (r.getStatusCode() != 429) { - logger.error("Got HTTP status code {} when sending metrics to bStats", - r.getStatusCode()); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } catch (ExecutionException e) { - logger.error("Unable to send metrics to bStats", e); - } - } - }, null); - } - - private static byte[] createResponseBody(JsonObject object) throws IOException { - ByteArrayOutputStream os = new ByteArrayOutputStream(); - try (Writer writer = - new BufferedWriter( - new OutputStreamWriter( - new GZIPOutputStream(os), StandardCharsets.UTF_8 - ) - ) - ) { - VelocityServer.GENERAL_GSON.toJson(object, writer); - } - return os.toByteArray(); - } - - /** - * Represents a custom chart. - */ - public abstract static class CustomChart { - - // The id of the chart - final String chartId; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - */ - CustomChart(String chartId) { - if (chartId == null || chartId.isEmpty()) { - throw new IllegalArgumentException("ChartId cannot be null or empty!"); - } - this.chartId = chartId; - } - - private JsonObject getRequestJsonObject() { - JsonObject chart = new JsonObject(); - chart.addProperty("chartId", chartId); - try { - JsonObject data = getChartData(); - if (data == null) { - // If the data is null we don't send the chart. - return null; - } - chart.add("data", data); - } catch (Throwable t) { - return null; - } - return chart; - } - - protected abstract JsonObject getChartData() throws Exception; - - } - - /** - * Represents a custom simple pie. - */ - public static class SimplePie extends CustomChart { - - private final Callable callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public SimplePie(String chartId, Callable callable) { - super(chartId); - this.callable = callable; - } - - @Override - protected JsonObject getChartData() throws Exception { - JsonObject data = new JsonObject(); - String value = callable.call(); - if (value == null || value.isEmpty()) { - // Null = skip the chart - return null; - } - data.addProperty("value", value); - return data; - } - } - - /** - * Represents a custom advanced pie. - */ - public static class AdvancedPie extends CustomChart { - - private final Callable> callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public AdvancedPie(String chartId, Callable> callable) { - super(chartId); - this.callable = callable; - } - - @Override - protected JsonObject getChartData() throws Exception { - Map map = callable.call(); - if (map == null || map.isEmpty()) { - // Null = skip the chart - return null; - } - - JsonObject data = new JsonObject(); - JsonObject values = new JsonObject(); - boolean allSkipped = true; - for (Map.Entry entry : map.entrySet()) { - if (entry.getValue() == 0) { - continue; // Skip this invalid - } - allSkipped = false; - values.addProperty(entry.getKey(), entry.getValue()); - } - if (allSkipped) { - // Null = skip the chart - return null; - } - data.add("values", values); - return data; - } - } - - /** - * Represents a custom drilldown pie. - */ - public static class DrilldownPie extends CustomChart { - - private final Callable>> callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public DrilldownPie(String chartId, Callable>> callable) { - super(chartId); - this.callable = callable; - } - - @Override - public JsonObject getChartData() throws Exception { - Map> map = callable.call(); - if (map == null || map.isEmpty()) { - // Null = skip the chart - return null; - } - boolean reallyAllSkipped = true; - JsonObject data = new JsonObject(); - JsonObject values = new JsonObject(); - for (Map.Entry> entryValues : map.entrySet()) { - JsonObject value = new JsonObject(); - boolean allSkipped = true; - for (Map.Entry valueEntry : map.get(entryValues.getKey()).entrySet()) { - value.addProperty(valueEntry.getKey(), valueEntry.getValue()); - allSkipped = false; - } - if (!allSkipped) { - reallyAllSkipped = false; - values.add(entryValues.getKey(), value); - } - } - if (reallyAllSkipped) { - // Null = skip the chart - return null; - } - data.add("values", values); - return data; - } - } - - /** - * Represents a custom single line chart. - */ - public static class SingleLineChart extends CustomChart { - - private final Callable callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public SingleLineChart(String chartId, Callable callable) { - super(chartId); - this.callable = callable; - } - - @Override - protected JsonObject getChartData() throws Exception { - JsonObject data = new JsonObject(); - int value = callable.call(); - if (value == 0) { - // Null = skip the chart - return null; - } - data.addProperty("value", value); - return data; - } - - } - - /** - * Represents a custom multi line chart. - */ - public static class MultiLineChart extends CustomChart { - - private final Callable> callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public MultiLineChart(String chartId, Callable> callable) { - super(chartId); - this.callable = callable; - } - - @Override - protected JsonObject getChartData() throws Exception { - Map map = callable.call(); - if (map == null || map.isEmpty()) { - // Null = skip the chart - return null; - } - JsonObject data = new JsonObject(); - JsonObject values = new JsonObject(); - boolean allSkipped = true; - for (Map.Entry entry : map.entrySet()) { - if (entry.getValue() == 0) { - continue; // Skip this invalid - } - allSkipped = false; - values.addProperty(entry.getKey(), entry.getValue()); - } - if (allSkipped) { - // Null = skip the chart - return null; - } - data.add("values", values); - return data; - } - - } - - /** - * Represents a custom simple bar chart. - */ - public static class SimpleBarChart extends CustomChart { - - private final Callable> callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public SimpleBarChart(String chartId, Callable> callable) { - super(chartId); - this.callable = callable; - } - - @Override - protected JsonObject getChartData() throws Exception { - JsonObject data = new JsonObject(); - JsonObject values = new JsonObject(); - Map map = callable.call(); - if (map == null || map.isEmpty()) { - // Null = skip the chart - return null; - } - for (Map.Entry entry : map.entrySet()) { - JsonArray categoryValues = new JsonArray(); - categoryValues.add(entry.getValue()); - values.add(entry.getKey(), categoryValues); - } - data.add("values", values); - return data; - } - - } - - /** - * Represents a custom advanced bar chart. - */ - public static class AdvancedBarChart extends CustomChart { - - private final Callable> callable; - - /** - * Class constructor. - * - * @param chartId The id of the chart. - * @param callable The callable which is used to request the chart data. - */ - public AdvancedBarChart(String chartId, Callable> callable) { - super(chartId); - this.callable = callable; - } - - @Override - protected JsonObject getChartData() throws Exception { - JsonObject values = new JsonObject(); - Map map = callable.call(); - if (map == null || map.isEmpty()) { - // Null = skip the chart - return null; - } - boolean allSkipped = true; - for (Map.Entry entry : map.entrySet()) { - if (entry.getValue().length == 0) { - continue; // Skip this invalid - } - allSkipped = false; - JsonArray categoryValues = new JsonArray(); - for (int categoryValue : entry.getValue()) { - categoryValues.add(categoryValue); - } - values.add(entry.getKey(), categoryValues); - } - if (allSkipped) { - // Null = skip the chart - return null; - } - JsonObject data = new JsonObject(); - data.add("values", values); - return data; - } - + private void appendPlatformData(JsonObjectBuilder builder) { + builder.appendField("osName", System.getProperty("os.name")); + builder.appendField("osArch", System.getProperty("os.arch")); + builder.appendField("osVersion", System.getProperty("os.version")); + builder.appendField("coreCount", Runtime.getRuntime().availableProcessors()); } static class VelocityMetrics { + + private static final Logger logger = LogManager.getLogger(Metrics.class); + static void startMetrics(VelocityServer server, VelocityConfiguration.Metrics metricsConfig) { - if (!metricsConfig.isFromConfig()) { - // Log an informational message. - logger.info("Velocity collects metrics and sends them to bStats (https://bstats.org)."); - logger.info("bStats collects some basic information like how many people use Velocity and"); - logger.info("their player count. This has no impact on performance and this data does not"); - logger.info("identify your server in any way. However, you may opt-out by editing your"); - logger.info("velocity.toml and setting enabled = false in the [metrics] section."); - } + Metrics metrics = new Metrics(logger, 4752, metricsConfig.isEnabled()); - // Load the data - String serverUuid = metricsConfig.getId(); - boolean logFailedRequests = metricsConfig.isLogFailure(); - // Only start Metrics, if it's enabled in the config - if (metricsConfig.isEnabled()) { - Metrics metrics = new Metrics("Velocity", 4752, serverUuid, logFailedRequests, server); + metrics.addCustomChart( + new SingleLineChart("players", server::getPlayerCount) + ); + metrics.addCustomChart( + new SingleLineChart("managed_servers", () -> server.getAllServers().size()) + ); + metrics.addCustomChart( + new SimplePie("online_mode", + () -> server.getConfiguration().isOnlineMode() ? "online" : "offline") + ); + metrics.addCustomChart(new SimplePie("velocity_version", + () -> server.getVersion().getVersion())); - metrics.addCustomChart( - new Metrics.SingleLineChart("players", server::getPlayerCount) - ); - metrics.addCustomChart( - new Metrics.SingleLineChart("managed_servers", () -> server.getAllServers().size()) - ); - metrics.addCustomChart( - new Metrics.SimplePie("online_mode", - () -> server.getConfiguration().isOnlineMode() ? "online" : "offline") - ); - metrics.addCustomChart(new Metrics.SimplePie("velocity_version", - () -> server.getVersion().getVersion())); + metrics.addCustomChart(new DrilldownPie("java_version", () -> { + Map> map = new HashMap<>(); + String javaVersion = System.getProperty("java.version"); + Map entry = new HashMap<>(); + entry.put(javaVersion, 1); - metrics.addCustomChart(new Metrics.DrilldownPie("java_version", () -> { - Map> map = new HashMap<>(); - String javaVersion = System.getProperty("java.version"); - Map entry = new HashMap<>(); - entry.put(javaVersion, 1); + // http://openjdk.java.net/jeps/223 + // Java decided to change their versioning scheme and in doing so modified the + // java.version system property to return $major[.$minor][.$security][-ea], as opposed to + // 1.$major.0_$identifier we can handle pre-9 by checking if the "major" is equal to "1", + // otherwise, 9+ + String majorVersion = javaVersion.split("\\.")[0]; + String release; - // http://openjdk.java.net/jeps/223 - // Java decided to change their versioning scheme and in doing so modified the - // java.version system property to return $major[.$minor][.$security][-ea], as opposed to - // 1.$major.0_$identifier we can handle pre-9 by checking if the "major" is equal to "1", - // otherwise, 9+ - String majorVersion = javaVersion.split("\\.")[0]; - String release; + int indexOf = javaVersion.lastIndexOf('.'); - int indexOf = javaVersion.lastIndexOf('.'); - - if (majorVersion.equals("1")) { - release = "Java " + javaVersion.substring(0, indexOf); - } else { - // of course, it really wouldn't be all that simple if they didn't add a quirk, now - // would it valid strings for the major may potentially include values such as -ea to - // denote a pre release - Matcher versionMatcher = Pattern.compile("\\d+").matcher(majorVersion); - if (versionMatcher.find()) { - majorVersion = versionMatcher.group(0); - } - release = "Java " + majorVersion; + if (majorVersion.equals("1")) { + release = "Java " + javaVersion.substring(0, indexOf); + } else { + // of course, it really wouldn't be all that simple if they didn't add a quirk, now + // would it valid strings for the major may potentially include values such as -ea to + // denote a pre release + Matcher versionMatcher = Pattern.compile("\\d+").matcher(majorVersion); + if (versionMatcher.find()) { + majorVersion = versionMatcher.group(0); } - map.put(release, entry); - - return map; - })); - } + release = "Java " + majorVersion; + } + map.put(release, entry); + return map; + })); } } -} + +} \ No newline at end of file diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 863a481c7..12564cd9f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -77,8 +77,6 @@ import java.util.stream.Collectors; import net.kyori.adventure.audience.Audience; import net.kyori.adventure.audience.ForwardingAudience; import net.kyori.adventure.text.Component; -import net.kyori.adventure.text.TextComponent; -import net.kyori.adventure.text.TranslatableComponent; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.asynchttpclient.AsyncHttpClient; 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 803312423..fa455e886 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -29,7 +29,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Random; -import java.util.UUID; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.serializer.gson.GsonComponentSerializer; import net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer; @@ -436,11 +435,6 @@ public class VelocityConfiguration implements ProxyConfig { } forwardingSecret = forwardingSecretString.getBytes(StandardCharsets.UTF_8); - if (!config.contains("metrics.id") || config.get("metrics.id").isEmpty()) { - config.set("metrics.id", UUID.randomUUID().toString()); - mustResave = true; - } - if (mustResave) { config.save(); } @@ -783,39 +777,16 @@ public class VelocityConfiguration implements ProxyConfig { public static class Metrics { private boolean enabled = true; - private String id = UUID.randomUUID().toString(); - private boolean logFailure = false; - - private boolean fromConfig; - - private Metrics() { - this.fromConfig = false; - } private Metrics(CommentedConfig toml) { if (toml != null) { - this.enabled = toml.getOrElse("enabled", false); - this.id = toml.getOrElse("id", UUID.randomUUID().toString()); - this.logFailure = toml.getOrElse("log-failure", false); - this.fromConfig = true; + this.enabled = toml.getOrElse("enabled", true); } } public boolean isEnabled() { return enabled; } - - public String getId() { - return id; - } - - public boolean isLogFailure() { - return logFailure; - } - - public boolean isFromConfig() { - return fromConfig; - } } public static class Messages { diff --git a/proxy/src/main/resources/default-velocity.toml b/proxy/src/main/resources/default-velocity.toml index f3b890926..fbad6626c 100644 --- a/proxy/src/main/resources/default-velocity.toml +++ b/proxy/src/main/resources/default-velocity.toml @@ -143,19 +143,6 @@ map = "Velocity" # Whether plugins should be shown in query response by default or not show-plugins = false -[metrics] -# Whether metrics will be reported to bStats (https://bstats.org). -# bStats collects some basic information, like how many people use Velocity and their -# player count. We recommend keeping bStats enabled, but if you're not comfortable with -# this, you can turn this setting off. There is no performance penalty associated with -# having metrics enabled, and data sent to bStats can't identify your server. -enabled = true - -# A unique, anonymous ID to identify this proxy with. -id = "" - -log-failure = false - # Legacy color codes and JSON are accepted in all messages. [messages] # Prefix when the player gets kicked from a server. From ae6f2cba2a03bcb253f36edfc5fb62573a0ee698 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 13 Feb 2021 18:02:03 -0500 Subject: [PATCH 63/69] Bump to Adventure 4.5.0 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index ebb19f20a..74b1efeb9 100644 --- a/build.gradle +++ b/build.gradle @@ -21,7 +21,7 @@ allprojects { ext { // dependency versions textVersion = '3.0.4' - adventureVersion = '4.4.0' + adventureVersion = '4.5.0' adventurePlatformVersion = '4.0.0-SNAPSHOT' junitVersion = '5.7.0' slf4jVersion = '1.7.30' From d1229ddca8f7cede89c6bc044278f3591049c1ab Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 13 Feb 2021 18:06:26 -0500 Subject: [PATCH 64/69] Velocity 1.1.4 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 74b1efeb9..44d625acc 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ allprojects { apply plugin: "com.github.spotbugs" group 'com.velocitypowered' - version '1.1.4-SNAPSHOT' + version '1.1.4' ext { // dependency versions From 2a8a982538d77ae6eefe372297147e4a96736514 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 13 Feb 2021 18:08:50 -0500 Subject: [PATCH 65/69] Velocity 1.1.5-SNAPSHOT --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 44d625acc..5de9efd07 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ allprojects { apply plugin: "com.github.spotbugs" group 'com.velocitypowered' - version '1.1.4' + version '1.1.5-SNAPSHOT' ext { // dependency versions From b94303d2be2c2afc3ec161865dd76854f8f1f94a Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 16 Feb 2021 01:14:22 -0500 Subject: [PATCH 66/69] Map command nodes being serialized by their identities Fixes #429 --- .../protocol/packet/AvailableCommands.java | 5 +++- .../util/collect/IdentityHashStrategy.java | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/util/collect/IdentityHashStrategy.java diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java index 891115511..90a9d3bc2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java @@ -23,7 +23,9 @@ import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.ProtocolUtils.Direction; import com.velocitypowered.proxy.protocol.packet.brigadier.ArgumentPropertyRegistry; +import com.velocitypowered.proxy.util.collect.IdentityHashStrategy; import io.netty.buffer.ByteBuf; +import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenCustomHashMap; import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap; import it.unimi.dsi.fastutil.objects.Object2IntMap; import java.util.ArrayDeque; @@ -96,7 +98,8 @@ public class AvailableCommands implements MinecraftPacket { public void encode(ByteBuf buf, Direction direction, ProtocolVersion protocolVersion) { // Assign all the children an index. Deque> childrenQueue = new ArrayDeque<>(ImmutableList.of(rootNode)); - Object2IntMap> idMappings = new Object2IntLinkedOpenHashMap<>(); + Object2IntMap> idMappings = new Object2IntLinkedOpenCustomHashMap<>( + IdentityHashStrategy.instance()); while (!childrenQueue.isEmpty()) { CommandNode child = childrenQueue.poll(); if (!idMappings.containsKey(child)) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/collect/IdentityHashStrategy.java b/proxy/src/main/java/com/velocitypowered/proxy/util/collect/IdentityHashStrategy.java new file mode 100644 index 000000000..b741d3ac5 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/collect/IdentityHashStrategy.java @@ -0,0 +1,24 @@ +package com.velocitypowered.proxy.util.collect; + +import it.unimi.dsi.fastutil.Hash.Strategy; + +public final class IdentityHashStrategy implements Strategy { + + @SuppressWarnings("rawtypes") + private static final IdentityHashStrategy INSTANCE = new IdentityHashStrategy(); + + public static Strategy instance() { + //noinspection unchecked + return INSTANCE; + } + + @Override + public int hashCode(T o) { + return System.identityHashCode(o); + } + + @Override + public boolean equals(T a, T b) { + return a == b; + } +} From 1a768bda9d7b7df8dd7a559fe4433e3d2efa9398 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 16 Feb 2021 01:15:58 -0500 Subject: [PATCH 67/69] We're going to need custom hash maps after all. --- proxy/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/proxy/build.gradle b/proxy/build.gradle index 254d4bdea..bde1cf168 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -118,7 +118,6 @@ shadowJar { exclude 'it/unimi/dsi/fastutil/objects/*Object2Float*' exclude 'it/unimi/dsi/fastutil/objects/*Object2IntArray*' exclude 'it/unimi/dsi/fastutil/objects/*Object2IntAVL*' - exclude 'it/unimi/dsi/fastutil/objects/*Object*OpenCustom*' exclude 'it/unimi/dsi/fastutil/objects/*Object2IntRB*' exclude 'it/unimi/dsi/fastutil/objects/*Object2Long*' exclude 'it/unimi/dsi/fastutil/objects/*Object2Object*' From 648624d33317751dad5e820c66b67548eb92083a Mon Sep 17 00:00:00 2001 From: David Mayr Date: Wed, 17 Feb 2021 01:13:48 +0100 Subject: [PATCH 68/69] Check permissions before providing suggestions (#430) --- .../proxy/command/CommandNodeFactory.java | 7 +++ .../proxy/command/CommandManagerTests.java | 49 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java index 5f0c9fdff..0229a43e6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java @@ -46,6 +46,10 @@ public interface CommandNodeFactory { }, (context, builder) -> { String[] args = BrigadierUtils.getSplitArguments(context); + if (!command.hasPermission(context.getSource(), args)) { + return builder.buildFuture(); + } + return command.suggestAsync(context.getSource(), args).thenApply(values -> { for (String value : values) { builder.suggest(value); @@ -82,6 +86,9 @@ public interface CommandNodeFactory { (context, builder) -> { I invocation = createInvocation(context); + if (!command.hasPermission(invocation)) { + return builder.buildFuture(); + } return command.suggestAsync(invocation).thenApply(values -> { for (String value : values) { builder.suggest(value); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java index d5dc457a9..ae8fa8343 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -25,6 +25,7 @@ import com.velocitypowered.api.command.SimpleCommand; import com.velocitypowered.proxy.plugin.MockEventManager; import com.velocitypowered.proxy.plugin.VelocityEventManager; import java.util.List; +import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import org.checkerframework.checker.nullness.qual.NonNull; @@ -487,6 +488,54 @@ public class CommandManagerTests { manager.offerSuggestions(MockCommandSource.INSTANCE, "foo2 baz ").join()); } + @Test + void testSuggestionPermissions() throws ExecutionException, InterruptedException { + VelocityCommandManager manager = createManager(); + RawCommand rawCommand = new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail("The Command should not be executed while testing suggestions"); + } + + @Override + public boolean hasPermission(Invocation invocation) { + return invocation.arguments().length() > 0; + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("suggestion"); + } + }; + + manager.register(rawCommand, "foo"); + + assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "foo").get().isEmpty()); + assertFalse(manager.offerSuggestions(MockCommandSource.INSTANCE, "foo bar").get().isEmpty()); + + Command oldCommand = new Command() { + @Override + public void execute(CommandSource source, String @NonNull [] args) { + fail("The Command should not be executed while testing suggestions"); + } + + @Override + public boolean hasPermission(CommandSource source, String @NonNull [] args) { + return args.length > 0; + } + + @Override + public List suggest(CommandSource source, String @NonNull [] currentArgs) { + return ImmutableList.of("suggestion"); + } + }; + + manager.register(oldCommand, "bar"); + + assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "bar").get().isEmpty()); + assertFalse(manager.offerSuggestions(MockCommandSource.INSTANCE, "bar foo").get().isEmpty()); + } + static class NoopSimpleCommand implements SimpleCommand { @Override public void execute(final Invocation invocation) { From 72d47b5a3dd703659c15d4594747e728ae4241aa Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 18 Feb 2021 18:12:38 -0500 Subject: [PATCH 69/69] Don't advertise the BungeeCord plugin messaging channel if it is disabled. --- .../connection/backend/BackendPlaySessionHandler.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 667b1867e..b396799dc 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 @@ -65,10 +65,13 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { @Override public void activated() { serverConn.getServer().addPlayer(serverConn.getPlayer()); - MinecraftConnection serverMc = serverConn.ensureConnected(); - serverMc.write(PluginMessageUtil.constructChannelsPacket(serverMc.getProtocolVersion(), - ImmutableList.of(getBungeeCordChannel(serverMc.getProtocolVersion())) - )); + + if (server.getConfiguration().isBungeePluginChannelEnabled()) { + MinecraftConnection serverMc = serverConn.ensureConnected(); + serverMc.write(PluginMessageUtil.constructChannelsPacket(serverMc.getProtocolVersion(), + ImmutableList.of(getBungeeCordChannel(serverMc.getProtocolVersion())) + )); + } } @Override