Uploaded image for project: 'Spigot'
  1. Spigot
  2. SPIGOT-7948

`Bukkit#dispatchCommand` uses the wrong `CommandListenerWrapper` for Players

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • None
    • None
    • None
    • >version [10:10:11] [Server thread/INFO]: This server is running CraftBukkit version 4372-Spigot-e65d67a-7b44d46 (MC: 1.21.3) (Implementing API version 1.21.3-R0.1-SNAPSHOT) [10:10:11] [Server thread/INFO]: Checking version, please wait... [10:10:11] [Th
    • Yes

      What I did:

      Registered a command that runs `Bukkit#dispatchCommand`:

      public final class BukkitTestPlugin extends JavaPlugin {
          @Override
          public void onEnable() {
              getCommand("dispatch").setExecutor(new CommandExecutor() {
                  @Override
                  public boolean onCommand(CommandSender commandSender, Command command, String s, String[] strings) {
                      Bukkit.dispatchCommand(commandSender, String.join(" ", strings));
                      return true;
                  }
              });
          }
      }
      

      Join as a player with operator permissions and run these commands (just the first example I found):

      /setblock 50 50 50 glass
      /data get block 50 50 50
      /dispatch data get block 50 50 50

      Expected result:

      On 1.21.1, running `data get block 50 50 50` works the same whether the player directly runs it or `Bukkit#dispatchCommand` runs it. Since the block at `50 50 50`, is glass, which is not a block entity, the command sends a failure message explaining that.

      Actual result:

      On 1.21.3, running `data get block 50 50 50` through `Bukkit#dispatchCommand` does not send the failure message as expected.

      Notes:

      As mentioned at the bottom of SPIGOT-7945, I noticed that `VanillaCommandWrapper` was given a check for `CommandSender sender instanceof EntityPlayer player`. As far as I can tell, `EntityPlayer` does not implement `CommandSender`, and `EntityPlayer` does not have any subclasses, so this check would never be true. I believe this line was supposed to be `sender instanceof Player` or `sender instanceof CraftPlayer` (`sender instanceof Player` and later casting to `CraftPlayer` matches the style of the other `instaceof` checks in this method (except for entity for some reason), though it could cause a `ClassCastException`, not that that's a real concern since other classes extending `Player` is unsupported).

      Since `sender instanceof EntityPlayer` is always false, players get treated as entities. This did resolve SPIGOT-7945, but I wondered if the `CommandListenerWrapper` for entities and players differed in any important way. Looking at the source, the `ICommandListener` for players implements `sendSystemMessage` by forwarding the message to the `EntityPlayer`, while entities do not do anything when `sendSystemMessage` is called. Therefore, messages like `The target block is not a block entity` do not get sent to the player when using `Bukkit#dispatchCommand` as mentioned previously.

      I also noticed that entities always have permission level 0 while players have a `getPermissionLevel` method, and there was a bit of a difference in the server parameter. I haven't checked whether these differences cause any issues, but the resolution for those would probably be the same for the `sendSystemMessage` problem showcased here. `VanilliaCommandWrapper` should probably be using `((CraftPlayer) sender).getHandle().createCommandSourceStack()`, as it did in previous versions.

            Unassigned Unassigned
            WillKroboth Will Kroboth
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: