From deacdb622818b43fdb3de426a98b7b869afbb542 Mon Sep 17 00:00:00 2001
From: Gero <gecam59@gmail.com>
Date: Thu, 30 May 2024 13:40:07 +0200
Subject: [PATCH] Fix StackOverflowError in CommandGraphInjector when using
 redirects (#1335)

---
 .../proxy/command/CommandGraphInjector.java   | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java
index 16e3119a6..848d76d13 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java
@@ -27,6 +27,8 @@ import com.mojang.brigadier.tree.CommandNode;
 import com.mojang.brigadier.tree.LiteralCommandNode;
 import com.mojang.brigadier.tree.RootCommandNode;
 import com.velocitypowered.proxy.command.brigadier.VelocityArgumentCommandNode;
+import java.util.IdentityHashMap;
+import java.util.Map;
 import java.util.concurrent.locks.Lock;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -66,6 +68,7 @@ public final class CommandGraphInjector<S> {
   public void inject(final RootCommandNode<S> dest, final S source) {
     lock.lock();
     try {
+      final Map<CommandNode<S>, CommandNode<S>> done = new IdentityHashMap<>();
       final RootCommandNode<S> origin = this.dispatcher.getRoot();
       final CommandContextBuilder<S> rootContext =
           new CommandContextBuilder<>(this.dispatcher, source, origin, 0);
@@ -88,7 +91,7 @@ public final class CommandGraphInjector<S> {
             VelocityCommands.getArgumentsNode(asLiteral);
         if (argsNode == null) {
           // This literal is associated to a BrigadierCommand, filter normally.
-          this.copyChildren(node, copy, source);
+          this.copyChildren(node, copy, source, done);
         } else {
           // Copy all children nodes (arguments node and hints)
           for (final CommandNode<S> child : node.getChildren()) {
@@ -102,7 +105,10 @@ public final class CommandGraphInjector<S> {
     }
   }
 
-  private @Nullable CommandNode<S> filterNode(final CommandNode<S> node, final S source) {
+  private @Nullable CommandNode<S> filterNode(final CommandNode<S> node, final S source, final Map<CommandNode<S>, CommandNode<S>> done) {
+    if (done.containsKey(node)) {
+      return done.get(node);
+    }
     // We only check the non-context requirement when filtering alias nodes.
     // Otherwise, we would need to manually craft context builder and reader instances,
     // which is both incorrect and inefficient. The reason why we can do so for alias
@@ -116,18 +122,18 @@ public final class CommandGraphInjector<S> {
       // Redirects to non-Brigadier commands are not supported. Luckily,
       // we don't expose the root node to API users, so they can't access
       // nodes associated to other commands.
-      final CommandNode<S> target = this.filterNode(node.getRedirect(), source);
+      final CommandNode<S> target = this.filterNode(node.getRedirect(), source, done);
       builder.forward(target, builder.getRedirectModifier(), builder.isFork());
     }
     final CommandNode<S> result = builder.build();
-    this.copyChildren(node, result, source);
+    done.put(node, result);
+    this.copyChildren(node, result, source, done);
     return result;
   }
 
-  private void copyChildren(final CommandNode<S> parent, final CommandNode<S> dest,
-      final S source) {
+  private void copyChildren(final CommandNode<S> parent, final CommandNode<S> dest, final S source, final Map<CommandNode<S>, CommandNode<S>> done) {
     for (final CommandNode<S> child : parent.getChildren()) {
-      final CommandNode<S> filtered = this.filterNode(child, source);
+      final CommandNode<S> filtered = this.filterNode(child, source, done);
       if (filtered != null) {
         dest.addChild(filtered);
       }