PaperMC / Velocity

The modern, next-generation Minecraft server proxy.
https://papermc.io/software/velocity
GNU General Public License v3.0
1.79k stars 632 forks source link

SimpleCommand fails to forward commands with args #555

Open roccodev opened 3 years ago

roccodev commented 3 years ago

Might be related to #369.

When hasPermission() == false, SimpleCommand classes will only forward the command if no arguments are used. Adding any argument will make the proxy catch it, throwing an error.

Simple PoC:

import com.velocitypowered.api.command.SimpleCommand;
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.format.NamedTextColor;

public class TestCommand implements SimpleCommand {
    @Override
    public void execute(Invocation invocation) {
        invocation.source().sendMessage(Component.text("Invoked!", NamedTextColor.GREEN));
    }

    @Override
    public boolean hasPermission(Invocation invocation) {
        // With this, the command should be forwarded to the backend server in any case.
        return false;
    }
}

// ...

commandManager.register("test", new TestCommand()); // same effect with CommandMeta

Expected result /test -> Forwarded to backend /test abc -> Forwarded to backend

Actual result /test -> Forwarded to backend /test abc -> Incorrect argument for command at position 5: test <--[HERE] (in red, Brigadier error)

When hasPermission is changed to return true, the command works as expected for hasPermission() == true. (printing Invoked! with and without args)

hugmanrique commented 3 years ago

This is expected behavior (and as you say is essentially the same as the linked issue) and mimics Brigadier's parsing mechanism.