Improve Player chat API handling

Properly split up the chat and command handling to reflect the server now
having separate packets for both, and the client always using the correct packet. Text
from a chat packet should never be parsed into a command, even if it starts with the `/`
character.

Add a missing async catcher and improve Spigot's async catcher error message.

== AT ==
public net.minecraft.server.network.ServerGamePacketListenerImpl isChatMessageIllegal(Ljava/lang/String;)Z

Co-authored-by: Jake Potrebic <jake.m.potrebic@gmail.com>
Co-authored-by: SoSeDiK <mrsosedik@gmail.com>

paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch

@@ -1017,12 +1017,10 @@
          }
      }
  
-@@ -1564,8 +2090,128 @@
+@@ -1566,6 +2092,127 @@
-         }
- 
          return false;
-+    }
+     }
-+
+ 
 +    // CraftBukkit start - add method
 +    public void chat(String s, PlayerChatMessage original, boolean async) {
 +        if (s.isEmpty() || this.player.getChatVisibility() == ChatVisiblity.HIDDEN) {
@@ -1030,7 +1028,7 @@
 +        }
 +        OutgoingChatMessage outgoing = OutgoingChatMessage.create(original);
 +
-+        if (!async && s.startsWith("/")) {
++        if (false && !async && s.startsWith("/")) { // Paper - Don't handle commands in chat logic
 +            this.handleCommand(s);
 +        } else if (this.player.getChatVisibility() == ChatVisiblity.SYSTEM) {
 +            // Do nothing, this is coming from a plugin
@@ -1115,9 +1113,10 @@
 +                this.server.console.sendMessage(s);
 +            }
 +        }
-     }
++    }
- 
++
 +    private void handleCommand(String s) {
++        org.spigotmc.AsyncCatcher.catchOp("Command Dispatched Async: " + s); // Paper - Add async catcher
 +        if ( org.spigotmc.SpigotConfig.logCommands ) // Spigot
 +        this.LOGGER.info(this.player.getScoreboardName() + " issued server command: " + s);
 +
@@ -1146,7 +1145,7 @@
      private PlayerChatMessage getSignedMessage(ServerboundChatPacket packet, LastSeenMessages lastSeenMessages) throws SignedMessageChain.DecodeException {
          SignedMessageBody signedmessagebody = new SignedMessageBody(packet.message(), packet.timeStamp(), packet.salt(), lastSeenMessages);
  
-@@ -1573,13 +2219,42 @@
+@@ -1573,13 +2220,42 @@
      }
  
      private void broadcastChatMessage(PlayerChatMessage message) {
@@ -1194,7 +1193,7 @@
              this.disconnect((Component) Component.translatable("disconnect.spam"));
          }
  
-@@ -1601,7 +2276,33 @@
+@@ -1601,7 +2277,33 @@
      @Override
      public void handleAnimate(ServerboundSwingPacket packet) {
          PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@@ -1228,7 +1227,7 @@
          this.player.swing(packet.getHand());
      }
  
-@@ -1609,6 +2310,29 @@
+@@ -1609,6 +2311,29 @@
      public void handlePlayerCommand(ServerboundPlayerCommandPacket packet) {
          PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
          if (this.player.hasClientLoaded()) {
@@ -1258,7 +1257,7 @@
              this.player.resetLastActionTime();
              Entity entity;
              PlayerRideableJumping ijumpable;
-@@ -1691,6 +2415,12 @@
+@@ -1691,6 +2416,12 @@
      }
  
      public void sendPlayerChatMessage(PlayerChatMessage message, ChatType.Bound params) {
@@ -1271,7 +1270,7 @@
          this.send(new ClientboundPlayerChatPacket(message.link().sender(), message.link().index(), message.signature(), message.signedBody().pack(this.messageSignatureCache), message.unsignedContent(), message.filterMask(), params));
          this.addPendingMessage(message);
      }
-@@ -1703,6 +2433,13 @@
+@@ -1703,6 +2434,13 @@
          return this.connection.getRemoteAddress();
      }
  
@@ -1285,7 +1284,7 @@
      public void switchToConfig() {
          this.waitingForSwitchToConfig = true;
          this.removePlayerFromWorld();
-@@ -1718,9 +2455,17 @@
+@@ -1718,9 +2456,17 @@
      @Override
      public void handleInteract(ServerboundInteractPacket packet) {
          PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@@ -1303,7 +1302,7 @@
  
              this.player.resetLastActionTime();
              this.player.setShiftKeyDown(packet.isUsingSecondaryAction());
-@@ -1733,20 +2478,58 @@
+@@ -1733,20 +2479,58 @@
  
                  if (this.player.canInteractWithEntity(axisalignedbb, 3.0D)) {
                      packet.dispatch(new ServerboundInteractPacket.Handler() {
@@ -1366,7 +1365,7 @@
                                      }
                                  }
  
-@@ -1755,19 +2538,20 @@
+@@ -1755,19 +2539,20 @@
  
                          @Override
                          public void onInteraction(InteractionHand hand) {
@@ -1390,7 +1389,7 @@
                                  label23:
                                  {
                                      if (entity instanceof AbstractArrow) {
-@@ -1785,6 +2569,11 @@
+@@ -1785,6 +2570,11 @@
                                      }
  
                                      ServerGamePacketListenerImpl.this.player.attack(entity);
@@ -1402,7 +1401,7 @@
                                      return;
                                  }
                              }
-@@ -1809,7 +2598,7 @@
+@@ -1809,7 +2599,7 @@
              case PERFORM_RESPAWN:
                  if (this.player.wonGame) {
                      this.player.wonGame = false;
@@ -1411,7 +1410,7 @@
                      this.resetPosition();
                      CriteriaTriggers.CHANGED_DIMENSION.trigger(this.player, Level.END, Level.OVERWORLD);
                  } else {
-@@ -1817,11 +2606,11 @@
+@@ -1817,11 +2607,11 @@
                          return;
                      }
  
@@ -1425,7 +1424,7 @@
                      }
                  }
                  break;
-@@ -1834,15 +2623,21 @@
+@@ -1834,15 +2624,21 @@
      @Override
      public void handleContainerClose(ServerboundContainerClosePacket packet) {
          PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@@ -1449,7 +1448,7 @@
                  this.player.containerMenu.sendAllDataToRemote();
              } else if (!this.player.containerMenu.stillValid(this.player)) {
                  ServerGamePacketListenerImpl.LOGGER.debug("Player {} interacted with invalid menu {}", this.player, this.player.containerMenu);
-@@ -1855,7 +2650,284 @@
+@@ -1855,7 +2651,284 @@
                      boolean flag = packet.getStateId() != this.player.containerMenu.getStateId();
  
                      this.player.containerMenu.suppressRemoteUpdates();
@@ -1735,7 +1734,7 @@
                      ObjectIterator objectiterator = Int2ObjectMaps.fastIterable(packet.getChangedSlots()).iterator();
  
                      while (objectiterator.hasNext()) {
-@@ -1901,8 +2973,22 @@
+@@ -1901,8 +2974,22 @@
                                  return;
                              }
  
@@ -1759,7 +1758,7 @@
                              if (containerrecipebook_a == RecipeBookMenu.PostPlaceAction.PLACE_GHOST_RECIPE) {
                                  this.player.connection.send(new ClientboundPlaceGhostRecipePacket(this.player.containerMenu.containerId, craftingmanager_d.display().display()));
                              }
-@@ -1917,6 +3003,7 @@
+@@ -1917,6 +3004,7 @@
      @Override
      public void handleContainerButtonClick(ServerboundContainerButtonClickPacket packet) {
          PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@@ -1767,7 +1766,7 @@
          this.player.resetLastActionTime();
          if (this.player.containerMenu.containerId == packet.containerId() && !this.player.isSpectator()) {
              if (!this.player.containerMenu.stillValid(this.player)) {
-@@ -1945,6 +3032,43 @@
+@@ -1945,6 +3033,43 @@
  
              boolean flag1 = packet.slotNum() >= 1 && packet.slotNum() <= 45;
              boolean flag2 = itemstack.isEmpty() || itemstack.getCount() <= itemstack.getMaxStackSize();
@@ -1811,7 +1810,7 @@
  
              if (flag1 && flag2) {
                  this.player.inventoryMenu.getSlot(packet.slotNum()).setByPlayer(itemstack);
-@@ -1972,6 +3096,7 @@
+@@ -1972,6 +3097,7 @@
      }
  
      private void updateSignText(ServerboundSignUpdatePacket packet, List<FilteredText> signText) {
@@ -1819,7 +1818,7 @@
          this.player.resetLastActionTime();
          ServerLevel worldserver = this.player.serverLevel();
          BlockPos blockposition = packet.getPos();
-@@ -1993,7 +3118,17 @@
+@@ -1993,7 +3119,17 @@
      @Override
      public void handlePlayerAbilities(ServerboundPlayerAbilitiesPacket packet) {
          PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@@ -1838,7 +1837,7 @@
      }
  
      @Override
-@@ -2002,6 +3137,7 @@
+@@ -2002,6 +3138,7 @@
          boolean flag = this.player.isModelPartShown(PlayerModelPart.HAT);
  
          this.player.updateOptions(packet.information());
@@ -1846,7 +1845,7 @@
          if (this.player.isModelPartShown(PlayerModelPart.HAT) != flag) {
              this.server.getPlayerList().broadcastAll(new ClientboundPlayerInfoUpdatePacket(ClientboundPlayerInfoUpdatePacket.Action.UPDATE_HAT, this.player));
          }
-@@ -2058,7 +3194,7 @@
+@@ -2058,7 +3195,7 @@
          if (!this.waitingForSwitchToConfig) {
              throw new IllegalStateException("Client acknowledged config, but none was requested");
          } else {
@@ -1855,7 +1854,7 @@
          }
      }
  
-@@ -2083,8 +3219,10 @@
+@@ -2083,8 +3220,10 @@
          });
      }
  

paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java

@@ -936,7 +936,7 @@ public final class CraftServer implements Server {
     public boolean dispatchCommand(CommandSender sender, String commandLine) {
         Preconditions.checkArgument(sender != null, "sender cannot be null");
         Preconditions.checkArgument(commandLine != null, "commandLine cannot be null");
-        org.spigotmc.AsyncCatcher.catchOp("command dispatch"); // Spigot
+        org.spigotmc.AsyncCatcher.catchOp("Command Dispatched Async: " + commandLine); // Spigot // Paper - Include command in error message
 
         if (this.commandMap.dispatch(sender, commandLine)) {
             return true;

paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java

@@ -563,7 +563,20 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
 
         if (this.getHandle().connection == null) return;
 
-        this.getHandle().connection.chat(msg, PlayerChatMessage.system(msg), false);
+        // Paper start - Improve chat handling
+        if (ServerGamePacketListenerImpl.isChatMessageIllegal(msg)) {
+            this.getHandle().connection.disconnect(Component.translatable("multiplayer.disconnect.illegal_characters"));
+        } else {
+            if (msg.startsWith("/")) {
+                this.getHandle().connection.handleCommand(msg);
+            } else {
+                final PlayerChatMessage playerChatMessage = PlayerChatMessage.system(msg).withUnsignedContent(Component.literal(msg));
+                // TODO chat decorating
+                // TODO text filtering
+                this.getHandle().connection.chat(msg, playerChatMessage, false);
+            }
+        }
+        // Paper end - Improve chat handling
     }
 
     @Override