Compare commits

...
Author SHA1 Message Date
Nassim Jahnke 065a01eb16 Move event down a little 2025-05-28 16:41:29 +02:00
Nassim Jahnke 3c257d3639 give up 2025-05-25 10:16:05 +02:00
Nassim Jahnke eb180d5aeb move early checks 2025-05-25 09:47:57 +02:00
Nassim Jahnke 5f210bb2d6 Put signed message decoding through the message chain
While this disrupts signed command handling a bit, general behavior stays the same, and most importantly chat can continue to be handled off-main. It is necessary for the message decoding to happen in order, which previously was not guaranteed with the method call being in the netty event loop for chat and on main for signed commands.
2025-05-25 09:47:56 +02:00
@@ -1395,7 +1395,7 @@
throw new IllegalArgumentException("Expected packet sequence nr >= 0"); throw new IllegalArgumentException("Expected packet sequence nr >= 0");
} else { } else {
this.ackBlockChangesUpTo = Math.max(sequence, this.ackBlockChangesUpTo); this.ackBlockChangesUpTo = Math.max(sequence, this.ackBlockChangesUpTo);
@@ -1356,20 +_,38 @@ @@ -1356,23 +_,37 @@
@Override @Override
public void handleSetCarriedItem(ServerboundSetCarriedItemPacket packet) { public void handleSetCarriedItem(ServerboundSetCarriedItemPacket packet) {
PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel()); PacketUtils.ensureRunningOnSameThread(packet, this, this.player.serverLevel());
@@ -1425,16 +1425,15 @@
@Override @Override
public void handleChat(ServerboundChatPacket packet) { public void handleChat(ServerboundChatPacket packet) {
+ // CraftBukkit start - async chat + if (this.server.isStopped()) return; // CraftBukkit - SPIGOT-3638
+ // SPIGOT-3638
+ if (this.server.isStopped()) {
+ return;
+ }
+ // CraftBukkit end
Optional<LastSeenMessages> optional = this.unpackAndApplyLastSeen(packet.lastSeenMessages()); Optional<LastSeenMessages> optional = this.unpackAndApplyLastSeen(packet.lastSeenMessages());
if (!optional.isEmpty()) { if (!optional.isEmpty()) {
this.tryHandleChat(packet.message(), () -> { this.tryHandleChat(packet.message(), () -> {
@@ -1381,25 +_,45 @@ + if (this.player.hasDisconnected()) return; // CraftBukkit
PlayerChatMessage signedMessage;
try {
signedMessage = this.getSignedMessage(packet, optional.get());
@@ -1381,10 +_,14 @@
return; return;
} }
@@ -1442,31 +1441,30 @@
- Component component = this.server.getChatDecorator().decorate(this.player, signedMessage.decoratedContent()); - Component component = this.server.getChatDecorator().decorate(this.player, signedMessage.decoratedContent());
- this.chatMessageChain.append(completableFuture, filteredText -> { - this.chatMessageChain.append(completableFuture, filteredText -> {
- PlayerChatMessage playerChatMessage = signedMessage.withUnsignedContent(component).filter(filteredText.mask()); - PlayerChatMessage playerChatMessage = signedMessage.withUnsignedContent(component).filter(filteredText.mask());
+ CompletableFuture<FilteredText> completableFuture = this.filterTextPacket(signedMessage.signedContent()).thenApplyAsync(java.util.function.Function.identity(), this.server.chatExecutor); // CraftBukkit - async chat + // Paper start - Adventure
+ CompletableFuture<Component> componentFuture = this.server.getChatDecorator().decorate(this.player, null, signedMessage.decoratedContent()); // Paper - Adventure + // Make decoration async too, just message decoding has to stay in order, unfortunately blocked by commands
+ CompletableFuture<FilteredText> completableFuture = this.filterTextPacket(signedMessage.signedContent()).thenApplyAsync(java.util.function.Function.identity(), this.server.chatExecutor);
+ CompletableFuture<Component> componentFuture = this.server.getChatDecorator().decorate(this.player, null, signedMessage.decoratedContent());
+ +
+ this.chatMessageChain.append(CompletableFuture.allOf(completableFuture, componentFuture), ($) -> { // Paper - Adventure + this.chatMessageChain.append(CompletableFuture.allOf(completableFuture, componentFuture), ($) -> {
+ PlayerChatMessage playerChatMessage = signedMessage.withUnsignedContent(componentFuture.join()).filter(completableFuture.join().mask()); // Paper - Adventure + PlayerChatMessage playerChatMessage = signedMessage.withUnsignedContent(componentFuture.join()).filter(completableFuture.join().mask());
+ // Paper end - Adventure
this.broadcastChatMessage(playerChatMessage); this.broadcastChatMessage(playerChatMessage);
}); });
- }); });
+ }, false); // CraftBukkit - async chat @@ -1394,12 +_,31 @@
}
}
@Override @Override
public void handleChatCommand(ServerboundChatCommandPacket packet) { public void handleChatCommand(ServerboundChatCommandPacket packet) {
this.tryHandleChat(packet.command(), () -> { this.tryHandleChat(packet.command(), () -> {
+ // CraftBukkit start - SPIGOT-7346: Prevent disconnected players from executing commands + // CraftBukkit start - SPIGOT-7346: Prevent disconnected players from executing commands
+ if (this.player.hasDisconnected()) { + if (this.player.hasDisconnected() || this.player.isRemoved()) {
+ return; + return;
+ } + }
+ // CraftBukkit end + // CraftBukkit end
this.performUnsignedChatCommand(packet.command()); this.performUnsignedChatCommand(packet.command());
- this.detectRateSpam(); - this.detectRateSpam();
- });
+ this.detectRateSpam("/" + packet.command()); // Spigot + this.detectRateSpam("/" + packet.command()); // Spigot
+ }, true); // CraftBukkit - sync commands });
} }
private void performUnsignedChatCommand(String command) { private void performUnsignedChatCommand(String command) {
@@ -1478,101 +1476,79 @@
+ +
+ PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(this.getCraftPlayer(), prefixedCommand, new org.bukkit.craftbukkit.util.LazyPlayerSet(this.server)); + PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(this.getCraftPlayer(), prefixedCommand, new org.bukkit.craftbukkit.util.LazyPlayerSet(this.server));
+ this.cserver.getPluginManager().callEvent(event); + this.cserver.getPluginManager().callEvent(event);
+
+ if (event.isCancelled()) { + if (event.isCancelled()) {
+ return; + return;
+ } + }
+
+ command = event.getMessage().substring(1); + command = event.getMessage().substring(1);
+ // CraftBukkit end + // CraftBukkit end
ParseResults<CommandSourceStack> parseResults = this.parseCommand(command); ParseResults<CommandSourceStack> parseResults = this.parseCommand(command);
if (this.server.enforceSecureProfile() && SignableCommand.hasSignableArguments(parseResults)) { if (this.server.enforceSecureProfile() && SignableCommand.hasSignableArguments(parseResults)) {
LOGGER.error( LOGGER.error(
@@ -1416,28 +_,57 @@ @@ -1416,13 +_,21 @@
Optional<LastSeenMessages> optional = this.unpackAndApplyLastSeen(packet.lastSeenMessages()); Optional<LastSeenMessages> optional = this.unpackAndApplyLastSeen(packet.lastSeenMessages());
if (!optional.isEmpty()) { if (!optional.isEmpty()) {
this.tryHandleChat(packet.command(), () -> { this.tryHandleChat(packet.command(), () -> {
+ // CraftBukkit start - SPIGOT-7346: Prevent disconnected players from executing commands + if (this.player.hasDisconnected()) return; // CraftBukkit - SPIGOT-7346: Prevent disconnected players from executing commands
+ if (this.player.hasDisconnected()) {
+ return;
+ }
+ // CraftBukkit end
this.performSignedChatCommand(packet, optional.get()); this.performSignedChatCommand(packet, optional.get());
- this.detectRateSpam(); - this.detectRateSpam();
- });
+ this.detectRateSpam("/" + packet.command()); // Spigot + this.detectRateSpam("/" + packet.command()); // Spigot
+ }, true); // CraftBukkit - sync commands });
} }
} }
private void performSignedChatCommand(ServerboundChatCommandSignedPacket packet, LastSeenMessages lastSeenMessages) { private void performSignedChatCommand(ServerboundChatCommandSignedPacket packet, LastSeenMessages lastSeenMessages) {
+ // CraftBukkit start + // Paper start
+ String command = "/" + packet.command(); + final String slashCommand = "/" + packet.command();
+ if (org.spigotmc.SpigotConfig.logCommands) { // Paper - Add missing SpigotConfig logCommands check + if (org.spigotmc.SpigotConfig.logCommands) {
+ LOGGER.info("{} issued server command: {}", this.player.getScoreboardName(), command); + LOGGER.info("{} issued server command: {}", this.player.getScoreboardName(), slashCommand);
+ } // Paper - Add missing SpigotConfig logCommands check + }
+ + // Paper end
+ PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(this.getCraftPlayer(), command, new org.bukkit.craftbukkit.util.LazyPlayerSet(this.server));
+ this.cserver.getPluginManager().callEvent(event);
+ command = event.getMessage().substring(1);
+ +
ParseResults<CommandSourceStack> parseResults = this.parseCommand(packet.command()); ParseResults<CommandSourceStack> parseResults = this.parseCommand(packet.command());
Map<String, PlayerChatMessage> map; Map<String, PlayerChatMessage> map;
try { @@ -1433,11 +_,30 @@
+ // Paper - Always parse the original command to add to the chat chain
map = this.collectSignedArguments(packet, SignableCommand.of(parseResults), lastSeenMessages);
} catch (SignedMessageChain.DecodeException var6) {
this.handleMessageDecodeFailure(var6);
return; return;
} }
+ // Paper start - Fix cancellation and message changing + // Paper start
+ if (event.isCancelled()) { + // Parsing commands to be on main :cryingandshaking: - if that weren't the case, both this and chat could be pushed off to the chat message chain immediately
+ // Only now are we actually good to return + // Always decode message arguments before cancelling, else the chat chain will be broken
+ if (this.player.isRemoved()) {
+ return; + return;
+ } + }
+ +
+ // Remove signed parts if the command was changed + final org.bukkit.event.player.PlayerCommandPreprocessEvent event = new org.bukkit.event.player.PlayerCommandPreprocessEvent(this.getCraftPlayer(), slashCommand, new org.bukkit.craftbukkit.util.LazyPlayerSet(this.server));
+ if (!command.equals(packet.command())) { + if (!event.callEvent()) {
+ parseResults = this.parseCommand(command); + return;
+ }
+
+ final String updatedCommand = event.getMessage().substring(1);
+ if (!updatedCommand.equals(packet.command())) {
+ // Remove signed parts if the command was changed
+ parseResults = this.parseCommand(updatedCommand);
+ map = Collections.emptyMap(); + map = Collections.emptyMap();
+ } + }
+ // Paper end - Fix cancellation and message changing + // Paper end
+
CommandSigningContext commandSigningContext = new CommandSigningContext.SignedArguments(map); CommandSigningContext commandSigningContext = new CommandSigningContext.SignedArguments(map);
parseResults = Commands.mapSource( parseResults = Commands.mapSource(
parseResults, commandSourceStack -> commandSourceStack.withSigningContext(commandSigningContext, this.chatMessageChain) parseResults, commandSourceStack -> commandSourceStack.withSigningContext(commandSigningContext, this.chatMessageChain)
); );
- this.server.getCommands().performCommand(parseResults, packet.command()); - this.server.getCommands().performCommand(parseResults, packet.command());
+ this.server.getCommands().performCommand(parseResults, command); // CraftBukkit + this.server.getCommands().performCommand(parseResults, updatedCommand); // Paper
} }
private void handleMessageDecodeFailure(SignedMessageChain.DecodeException exception) { private void handleMessageDecodeFailure(SignedMessageChain.DecodeException exception) {
@@ -1501,14 +_,20 @@ @@ -1503,7 +_,7 @@
return dispatcher.parse(command, this.player.createCommandSourceStack());
}
- private void tryHandleChat(String message, Runnable handler) { private void tryHandleChat(String message, Runnable handler) {
+ private void tryHandleChat(String message, Runnable handler, boolean sync) { // CraftBukkit
if (isChatMessageIllegal(message)) { if (isChatMessageIllegal(message)) {
- this.disconnect(Component.translatable("multiplayer.disconnect.illegal_characters")); - this.disconnect(Component.translatable("multiplayer.disconnect.illegal_characters"));
- } else if (this.player.getChatVisibility() == ChatVisiblity.HIDDEN) {
+ this.disconnectAsync(Component.translatable("multiplayer.disconnect.illegal_characters"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_CHARACTERS); // Paper - add proper async disconnect + this.disconnectAsync(Component.translatable("multiplayer.disconnect.illegal_characters"), org.bukkit.event.player.PlayerKickEvent.Cause.ILLEGAL_CHARACTERS); // Paper - add proper async disconnect
+ } else if (this.player.isRemoved() || this.player.getChatVisibility() == ChatVisiblity.HIDDEN) { // CraftBukkit - dead men tell no tales } else if (this.player.getChatVisibility() == ChatVisiblity.HIDDEN) {
this.send(new ClientboundSystemChatPacket(Component.translatable("chat.disabled.options").withStyle(ChatFormatting.RED), false)); this.send(new ClientboundSystemChatPacket(Component.translatable("chat.disabled.options").withStyle(ChatFormatting.RED), false));
} else { } else {
this.player.resetLastActionTime();
- this.server.execute(handler);
+ // CraftBukkit start
+ if (sync) {
+ this.server.execute(handler);
+ } else {
+ handler.run();
+ }
+ // CraftBukkit end
}
}
@@ -1520,7 +_,7 @@ @@ -1520,7 +_,7 @@
var10000 = Optional.of(lastSeenMessages); var10000 = Optional.of(lastSeenMessages);
} catch (LastSeenMessagesValidator.ValidationException var5) { } catch (LastSeenMessagesValidator.ValidationException var5) {
@@ -2607,7 +2583,7 @@
this.chatMessageChain this.chatMessageChain
.append( .append(
() -> { () -> {
+ server.executeBlocking(() -> { // Paper - Broadcast chat session update sync + server.executeBlocking(() -> { // Paper - Broadcast chat session update sync, the used broadcast method isn't thread safe
this.player.setChatSession(chatSession); this.player.setChatSession(chatSession);
this.server this.server
.getPlayerList() .getPlayerList()