From 084b741375d9cdd0311648b4e74c17217441b2f8 Mon Sep 17 00:00:00 2001
From: Andrew Steinborn <git@steinborn.me>
Date: Tue, 24 Nov 2020 12:03:34 -0500
Subject: [PATCH] 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<CommandSource, ?> args;
     private @MonotonicNonNull CommandNode<CommandSource> built;
+    private boolean validated;
 
     private WireNode(int idx, byte flags, int[] children, int redirectTo,
         @Nullable ArgumentBuilder<CommandSource, ?> 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);