Skip to content

Commit

Permalink
Fix StackOverflowError in CommandGraphInjector when using redirects (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrygames authored May 30, 2024
1 parent 1c36b66 commit deacdb6
Showing 1 changed file with 13 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
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;
Expand Down Expand Up @@ -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);
Expand All @@ -88,7 +91,7 @@ public void inject(final RootCommandNode<S> dest, final S source) {
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()) {
Expand All @@ -102,7 +105,10 @@ public void inject(final RootCommandNode<S> dest, final S source) {
}
}

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
Expand All @@ -116,18 +122,18 @@ public void inject(final RootCommandNode<S> dest, final S source) {
// 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);
}
Expand Down

0 comments on commit deacdb6

Please sign in to comment.