From bcc07d07b6115085a5a9511dfa975fde3bfcddfd Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 3 Mar 2016 01:17:12 -0600 Subject: [PATCH] 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 Co-authored-by: SoSeDiK --- .../ServerGamePacketListenerImpl.java.patch | 57 +++++++++---------- .../org/bukkit/craftbukkit/CraftServer.java | 2 +- .../craftbukkit/entity/CraftPlayer.java | 15 ++++- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch b/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch index 2181df53c7..b3bfe8b8af 100644 --- a/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch +++ b/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 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 @@ }); } diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java index e4335bfc98..a0a0fa7de4 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/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; diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java index 38dd2a6a6d..aa5755bd26 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java +++ b/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