From 329e2b0dc972b8cdb5f09df9969af19deba71ef1 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 16 Nov 2020 12:24:14 -0500 Subject: [PATCH 1/8] Use our fork's removeChildByName --- .../proxy/command/VelocityCommandManager.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 92232a380..4165c0afa 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -125,11 +125,7 @@ public class VelocityCommandManager implements CommandManager { @Override public void unregister(final String alias) { Preconditions.checkNotNull(alias, "alias"); - CommandNode node = - dispatcher.getRoot().getChild(alias.toLowerCase(Locale.ENGLISH)); - if (node != null) { - dispatcher.getRoot().getChildren().remove(node); - } + dispatcher.getRoot().removeChildByName(alias.toLowerCase(Locale.ENGLISH)); } /** From 563a96e6242d6a22ec9023b9e3a395cf6cd0ff50 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 17 Nov 2020 04:38:24 -0500 Subject: [PATCH 2/8] Bump Netty version --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 7a1b37c86..6d4f7e8d7 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.52.Final' + nettyVersion = '4.1.54.Final' guavaVersion = '25.1-jre' checkerFrameworkVersion = '3.6.1' configurateVersion = '3.7.1' From 3d0cb505699369eccdbdfdfac727f9b9ae53bade Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 17 Nov 2020 04:48:20 -0500 Subject: [PATCH 3/8] Better check for unfinished decompression --- .../natives/compression/Java11VelocityCompressor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/native/src/main/java/com/velocitypowered/natives/compression/Java11VelocityCompressor.java b/native/src/main/java/com/velocitypowered/natives/compression/Java11VelocityCompressor.java index 845c3adcd..49ac1da26 100644 --- a/native/src/main/java/com/velocitypowered/natives/compression/Java11VelocityCompressor.java +++ b/native/src/main/java/com/velocitypowered/natives/compression/Java11VelocityCompressor.java @@ -78,9 +78,9 @@ public class Java11VelocityCompressor implements VelocityCompressor { destination.writerIndex(destination.writerIndex() + produced); } - if (inflater.getBytesWritten() != uncompressedSize) { - throw new DataFormatException("Did not write the exact expected number of" - + " uncompressed bytes, expected " + uncompressedSize); + if (!inflater.finished()) { + throw new DataFormatException("Received a deflate stream that was too large, wanted " + + uncompressedSize); } source.readerIndex(origIdx + inflater.getTotalIn()); } catch (Throwable e) { From 783054d098d49df713499c55aaa68de9ac53ee13 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 22 Nov 2020 12:00:47 -0500 Subject: [PATCH 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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); } }