diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java index 6655e3fbf..a74f7c56a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java @@ -137,12 +137,14 @@ public class TransitionSessionHandler implements MinecraftSessionHandler { smc.setActiveSessionHandler(StateRegistry.PLAY, new BackendPlaySessionHandler(server, serverConn)); - // Clean up disabling auto-read while the connected event was being processed. - smc.setAutoReading(true); - // Now set the connected server. serverConn.getPlayer().setConnectedServer(serverConn); + // Clean up disabling auto-read while the connected event was being processed. + // Do this after setting the connection, so no incoming packets are processed before + // the API knows which server the player is connected to. + smc.setAutoReading(true); + // Send client settings. In 1.20.2+ this is done in the config state. if (smc.getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_20_2) && player.getClientSettingsPacket() != null) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index a76f054e7..f2df78a7d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -54,6 +54,7 @@ import com.velocitypowered.proxy.protocol.packet.ServerboundCookieResponsePacket import com.velocitypowered.proxy.protocol.packet.TabCompleteRequestPacket; import com.velocitypowered.proxy.protocol.packet.TabCompleteResponsePacket; import com.velocitypowered.proxy.protocol.packet.TabCompleteResponsePacket.Offer; +import com.velocitypowered.proxy.protocol.packet.chat.ChatAcknowledgementPacket; import com.velocitypowered.proxy.protocol.packet.chat.ChatHandler; import com.velocitypowered.proxy.protocol.packet.chat.ChatTimeKeeper; import com.velocitypowered.proxy.protocol.packet.chat.CommandHandler; @@ -423,6 +424,15 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { return true; } + @Override + public boolean handle(ChatAcknowledgementPacket packet) { + if (player.getCurrentServer().isEmpty()) { + return true; + } + player.getChatQueue().handleAcknowledgement(packet.offset()); + return true; + } + @Override public boolean handle(ServerboundCookieResponsePacket packet) { server.getEventManager() diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 8522dfce7..df6769ffd 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java @@ -83,6 +83,7 @@ import com.velocitypowered.proxy.protocol.packet.chat.ChatType; import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder; import com.velocitypowered.proxy.protocol.packet.chat.PlayerChatCompletionPacket; import com.velocitypowered.proxy.protocol.packet.chat.builder.ChatBuilderFactory; +import com.velocitypowered.proxy.protocol.packet.chat.builder.ChatBuilderV2; import com.velocitypowered.proxy.protocol.packet.chat.legacy.LegacyChatPacket; import com.velocitypowered.proxy.protocol.packet.config.ClientboundServerLinksPacket; import com.velocitypowered.proxy.protocol.packet.config.StartUpdatePacket; @@ -1108,11 +1109,12 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, "input cannot be greater than " + LegacyChatPacket.MAX_SERVERBOUND_MESSAGE_LENGTH + " characters in length"); if (getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_19)) { - this.chatQueue.hijack(getChatBuilderFactory().builder().asPlayer(this).message(input), - (instant, item) -> { - item.setTimestamp(instant); - return item.toServer(); - }); + ChatBuilderV2 message = getChatBuilderFactory().builder().asPlayer(this).message(input); + this.chatQueue.queuePacket(chatState -> { + message.setTimestamp(chatState.lastTimestamp); + message.setLastSeenMessages(chatState.createLastSeen()); + return message.toServer(); + }); } else { ensureBackendConnection().write(getChatBuilderFactory().builder() .asPlayer(this).message(input).toServer()); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatAcknowledgementPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatAcknowledgementPacket.java index 6ca648a73..b0718090e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatAcknowledgementPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatAcknowledgementPacket.java @@ -54,4 +54,8 @@ public class ChatAcknowledgementPacket implements MinecraftPacket { "offset=" + offset + '}'; } + + public int offset() { + return offset; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatQueue.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatQueue.java index 2ddcff17d..5928bf36f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatQueue.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/ChatQueue.java @@ -23,7 +23,9 @@ import com.velocitypowered.proxy.protocol.MinecraftPacket; import io.netty.channel.ChannelFuture; import org.checkerframework.checker.nullness.qual.Nullable; import java.time.Instant; +import java.util.BitSet; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; /** @@ -32,9 +34,10 @@ import java.util.function.Function; */ public class ChatQueue { - private final Object internalLock; + private final Object internalLock = new Object(); private final ConnectedPlayer player; - private CompletableFuture packetFuture; + private final ChatState chatState = new ChatState(); + private CompletableFuture head = CompletableFuture.completedFuture(null); /** * Instantiates a {@link ChatQueue} for a specific {@link ConnectedPlayer}. @@ -43,8 +46,19 @@ public class ChatQueue { */ public ChatQueue(ConnectedPlayer player) { this.player = player; - this.packetFuture = CompletableFuture.completedFuture(new WrappedPacket(Instant.EPOCH, null)); - this.internalLock = new Object(); + } + + private void queueTask(Task task) { + synchronized (internalLock) { + MinecraftConnection smc = player.ensureAndGetCurrentServer().ensureConnected(); + head = head.thenCompose(v -> { + try { + return task.update(chatState, smc).exceptionally(ignored -> null); + } catch (Throwable ignored) { + return CompletableFuture.completedFuture(null); + } + }); + } } /** @@ -52,121 +66,115 @@ public class ChatQueue { * packets. This maintains order on the server-level for the client insertions of commands * and messages. All entries are locked through an internal object lock. * - * @param nextPacket the {@link CompletableFuture} which will provide the next-processed packet. - * @param timestamp the {@link Instant} timestamp of this packet so we can allow piggybacking. + * @param nextPacket a function mapping {@link LastSeenMessages} state to a {@link CompletableFuture} that will + * provide the next-processed packet. This should include the fixed {@link LastSeenMessages}. + * @param timestamp the new {@link Instant} timestamp of this packet to update the internal chat state. + * @param lastSeenMessages the new {@link LastSeenMessages} last seen messages to update the internal chat state. */ - public void queuePacket(CompletableFuture nextPacket, Instant timestamp) { - synchronized (internalLock) { // wait for the lock to resolve - we don't want to drop packets - MinecraftConnection smc = player.ensureAndGetCurrentServer().ensureConnected(); - - CompletableFuture nextInLine = WrappedPacket.wrap(timestamp, nextPacket); - this.packetFuture = awaitChat(smc, this.packetFuture, - nextInLine); // we await chat, binding `this.packetFuture` -> `nextInLine` - } + public void queuePacket(Function> nextPacket, @Nullable Instant timestamp, @Nullable LastSeenMessages lastSeenMessages) { + queueTask((chatState, smc) -> { + LastSeenMessages newLastSeenMessages = chatState.updateFromMessage(timestamp, lastSeenMessages); + return nextPacket.apply(newLastSeenMessages).thenCompose(packet -> writePacket(packet, smc)); + }); } /** - * Hijacks the latest sent packet's timestamp to provide an in-order packet without polling the + * Hijacks the latest sent packet's chat state to provide an in-order packet without polling the * physical, or prior packets sent through the stream. * - * @param packet the {@link MinecraftPacket} to send. - * @param instantMapper the {@link InstantPacketMapper} which maps the prior timestamp and current - * packet to a new packet. - * @param the type of base to expect when mapping the packet. - * @param the type of packet for instantMapper type-checking. + * @param packetFunction a function that maps the prior {@link ChatState} into a new packet. + * @param the type of packet to send. */ - public void hijack(K packet, - InstantPacketMapper instantMapper) { - synchronized (internalLock) { - CompletableFuture trueFuture = CompletableFuture.completedFuture(packet); - MinecraftConnection smc = player.ensureAndGetCurrentServer().ensureConnected(); - - this.packetFuture = hijackCurrentPacket(smc, this.packetFuture, trueFuture, instantMapper); - } + public void queuePacket(Function packetFunction) { + queueTask((chatState, smc) -> { + T packet = packetFunction.apply(chatState); + return writePacket(packet, smc); + }); } - private static Function writePacket(MinecraftConnection connection) { - return wrappedPacket -> { - if (!connection.isClosed()) { - ChannelFuture future = wrappedPacket.write(connection); + public void handleAcknowledgement(int offset) { + queueTask((chatState, smc) -> { + int ackCountToForward = chatState.accumulateAckCount(offset); + if (ackCountToForward > 0) { + return writePacket(new ChatAcknowledgementPacket(ackCountToForward), smc); + } + return CompletableFuture.completedFuture(null); + }); + } + + private static CompletableFuture writePacket(T packet, MinecraftConnection smc) { + return CompletableFuture.runAsync(() -> { + if (!smc.isClosed()) { + ChannelFuture future = smc.write(packet); if (future != null) { future.awaitUninterruptibly(); } } - - return wrappedPacket; - }; + }, smc.eventLoop()); } - private static CompletableFuture awaitChat( - MinecraftConnection connection, - CompletableFuture binder, - CompletableFuture future - ) { - // the binder will run -> then the future will get the `write packet` caller - return binder.thenCompose(ignored -> future.thenApply(writePacket(connection))); - } - - private static CompletableFuture hijackCurrentPacket( - MinecraftConnection connection, - CompletableFuture binder, - CompletableFuture future, - InstantPacketMapper packetMapper - ) { - CompletableFuture awaitedFuture = new CompletableFuture<>(); - // the binder will complete -> then the future will get the `write packet` caller - binder.whenComplete((previous, ignored) -> { - // map the new packet into a better "designed" packet with the hijacked packet's timestamp - WrappedPacket.wrap(previous.timestamp, - future.thenApply(item -> packetMapper.map(previous.timestamp, item))) - .thenApplyAsync(writePacket(connection), connection.eventLoop()) - .whenComplete( - (packet, throwable) -> awaitedFuture.complete(throwable != null ? null : packet)); - }); - return awaitedFuture; + private interface Task { + CompletableFuture update(ChatState chatState, MinecraftConnection smc); } /** - * Provides an {@link Instant} based timestamp mapper from an existing object to create a packet. + * Tracks the last Secure Chat state that we received from the client. This is important to always have a valid 'last + * seen' state that is consistent with future and past updates from the client (which may be signed). This state is + * used to construct 'spoofed' command packets from the proxy to the server. + *
    + *
  • If we last forwarded a chat or command packet from the client, we have a known 'last seen' that we can + * reuse.
    • + *
  • If we last forwarded a {@link ChatAcknowledgementPacket}, the previous 'last seen' cannot be reused. We + * cannot predict an up-to-date 'last seen', as we do not know which messages the client actually saw.
    • + *
  • Therefore, we need to hold back any acknowledgement packets so that we can continue to reuse the last valid + * 'last seen' state.
    • + *
  • However, there is a limit to the number of messages that can remain unacknowledged on the server.
    • + *
  • To address this, we know that if the client has moved its 'last seen' window far enough, we can fill in the + * gap with dummy 'last seen', and it will never be checked.
    • + *
* - * @param The base object type to map. - * @param The resulting packet type. + * Note that this is effectively unused for 1.20.5+ clients, as commands without any signature do not send 'last seen' + * updates. */ - public interface InstantPacketMapper { + public static class ChatState { + private static final int MINIMUM_DELAYED_ACK_COUNT = LastSeenMessages.WINDOW_SIZE; + private static final BitSet DUMMY_LAST_SEEN_MESSAGES = new BitSet(); - /** - * Maps a value into a packet with it and a timestamp. - * - * @param nextInstant the {@link Instant} timestamp to use for tracking. - * @param currentObject the current item to map to the packet. - * @return The resulting packet from the mapping. - */ - V map(Instant nextInstant, K currentObject); - } + public volatile Instant lastTimestamp = Instant.EPOCH; + private volatile BitSet lastSeenMessages = new BitSet(); + private final AtomicInteger delayedAckCount = new AtomicInteger(); - private static class WrappedPacket { - - private final Instant timestamp; - private final MinecraftPacket packet; - - private WrappedPacket(Instant timestamp, MinecraftPacket packet) { - this.timestamp = timestamp; - this.packet = packet; + private ChatState() { } @Nullable - public ChannelFuture write(MinecraftConnection connection) { - if (packet != null) { - return connection.write(packet); + public LastSeenMessages updateFromMessage(@Nullable Instant timestamp, @Nullable LastSeenMessages lastSeenMessages) { + if (timestamp != null) { + this.lastTimestamp = timestamp; + } + if (lastSeenMessages != null) { + // We held back some acknowledged messages, so flush that out now that we have a known 'last seen' state again + int delayedAckCount = this.delayedAckCount.getAndSet(0); + this.lastSeenMessages = lastSeenMessages.getAcknowledged(); + return lastSeenMessages.offset(delayedAckCount); } return null; } - private static CompletableFuture wrap(Instant timestamp, - CompletableFuture nextPacket) { - return nextPacket - .thenApply(pkt -> new WrappedPacket(timestamp, pkt)) - .exceptionally(ignored -> new WrappedPacket(timestamp, null)); + public int accumulateAckCount(int ackCount) { + int delayedAckCount = this.delayedAckCount.addAndGet(ackCount); + int ackCountToForward = delayedAckCount - MINIMUM_DELAYED_ACK_COUNT; + if (ackCountToForward >= LastSeenMessages.WINDOW_SIZE) { + // Because we only forward acknowledgements above the window size, we don't have to shift the previous 'last seen' state + this.lastSeenMessages = DUMMY_LAST_SEEN_MESSAGES; + this.delayedAckCount.set(MINIMUM_DELAYED_ACK_COUNT); + return ackCountToForward; + } + return 0; + } + + public LastSeenMessages createLastSeen() { + return new LastSeenMessages(0, lastSeenMessages); } } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/CommandHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/CommandHandler.java index c778eecd0..9786fe145 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/CommandHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/CommandHandler.java @@ -23,11 +23,13 @@ import com.velocitypowered.proxy.connection.client.ConnectedPlayer; import com.velocitypowered.proxy.protocol.MinecraftPacket; import java.time.Instant; import java.util.concurrent.CompletableFuture; +import java.util.function.BiFunction; import java.util.function.Function; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.checkerframework.checker.nullness.qual.Nullable; public interface CommandHandler { @@ -53,11 +55,12 @@ public interface CommandHandler { } default void queueCommandResult(VelocityServer server, ConnectedPlayer player, - Function> futurePacketCreator, - String message, Instant timestamp) { - player.getChatQueue().queuePacket( - server.getCommandManager().callCommandEvent(player, message) - .thenComposeAsync(futurePacketCreator) + BiFunction> futurePacketCreator, + String message, Instant timestamp, @Nullable LastSeenMessages lastSeenMessages) { + CompletableFuture eventFuture = server.getCommandManager().callCommandEvent(player, message); + player.getChatQueue().queuePacket( + newLastSeenMessages -> eventFuture + .thenComposeAsync(event -> futurePacketCreator.apply(event, newLastSeenMessages)) .thenApply(pkt -> { if (server.getConfiguration().isLogCommandExecutions()) { logger.info("{} -> executed command /{}", player, message); @@ -68,6 +71,6 @@ public interface CommandHandler { player.sendMessage( Component.translatable("velocity.command.generic-error", NamedTextColor.RED)); return null; - }), timestamp); + }), timestamp, lastSeenMessages); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java index a1ed539d6..18a743c82 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/LastSeenMessages.java @@ -24,7 +24,8 @@ import java.util.BitSet; public class LastSeenMessages { - private static final int DIV_FLOOR = -Math.floorDiv(-20, 8); + public static final int WINDOW_SIZE = 20; + private static final int DIV_FLOOR = -Math.floorDiv(-WINDOW_SIZE, 8); private int offset; private BitSet acknowledged; @@ -33,6 +34,11 @@ public class LastSeenMessages { this.acknowledged = new BitSet(); } + public LastSeenMessages(int offset, BitSet acknowledged) { + this.offset = offset; + this.acknowledged = acknowledged; + } + public LastSeenMessages(ByteBuf buf) { this.offset = ProtocolUtils.readVarInt(buf); @@ -46,14 +52,18 @@ public class LastSeenMessages { buf.writeBytes(Arrays.copyOf(acknowledged.toByteArray(), DIV_FLOOR)); } - public boolean isEmpty() { - return acknowledged.isEmpty(); - } - public int getOffset() { return this.offset; } + public BitSet getAcknowledged() { + return acknowledged; + } + + public LastSeenMessages offset(final int offset) { + return new LastSeenMessages(this.offset + offset, acknowledged); + } + @Override public String toString() { return "LastSeenMessages{" + diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/builder/ChatBuilderV2.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/builder/ChatBuilderV2.java index e9e24c2a0..9ac028646 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/builder/ChatBuilderV2.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/builder/ChatBuilderV2.java @@ -21,6 +21,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.api.proxy.Player; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.packet.chat.ChatType; +import com.velocitypowered.proxy.protocol.packet.chat.LastSeenMessages; import java.time.Instant; import net.kyori.adventure.identity.Identity; import net.kyori.adventure.text.Component; @@ -36,6 +37,7 @@ public abstract class ChatBuilderV2 { protected @Nullable Identity senderIdentity; protected Instant timestamp; protected ChatType type = ChatType.CHAT; + protected @Nullable LastSeenMessages lastSeenMessages; protected ChatBuilderV2(ProtocolVersion version) { this.version = version; @@ -77,6 +79,11 @@ public abstract class ChatBuilderV2 { return this; } + public ChatBuilderV2 setLastSeenMessages(LastSeenMessages lastSeenMessages) { + this.lastSeenMessages = lastSeenMessages; + return this; + } + public abstract MinecraftPacket toClient(); public abstract MinecraftPacket toServer(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedChatHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedChatHandler.java index f4964e6c2..f8fc906ab 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedChatHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedChatHandler.java @@ -91,11 +91,12 @@ public class KeyedChatHandler implements }); } chatQueue.queuePacket( - chatFuture.exceptionally((ex) -> { + newLastSeen -> chatFuture.exceptionally((ex) -> { logger.error("Exception while handling player chat for {}", player, ex); return null; }), - packet.getExpiry() + packet.getExpiry(), + null ); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java index bef75247c..1d3751e45 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/keyed/KeyedCommandHandler.java @@ -43,7 +43,7 @@ public class KeyedCommandHandler implements CommandHandler { + queueCommandResult(this.server, this.player, (event, newLastSeenMessages) -> { CommandExecuteEvent.CommandResult result = event.getResult(); IdentifiedKey playerKey = player.getIdentifiedKey(); if (result == CommandExecuteEvent.CommandResult.denied()) { @@ -111,6 +111,6 @@ public class KeyedCommandHandler implements CommandHandler { @Override public void handlePlayerCommandInternal(LegacyChatPacket packet) { String command = packet.getMessage().substring(1); - queueCommandResult(this.server, this.player, event -> { + queueCommandResult(this.server, this.player, (event, newLastSeenMessages) -> { CommandExecuteEvent.CommandResult result = event.getResult(); if (result == CommandExecuteEvent.CommandResult.denied()) { return CompletableFuture.completedFuture(null); @@ -62,6 +62,6 @@ public class LegacyCommandHandler implements CommandHandler { } return null; }); - }, command, Instant.now()); + }, command, Instant.now(), null); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatBuilder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatBuilder.java index 9a4fdc725..eb74123fc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatBuilder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatBuilder.java @@ -41,6 +41,7 @@ public class SessionChatBuilder extends ChatBuilderV2 { @Override public MinecraftPacket toServer() { + LastSeenMessages lastSeenMessages = this.lastSeenMessages != null ? this.lastSeenMessages : new LastSeenMessages(); if (message.startsWith("/")) { if (version.noLessThan(ProtocolVersion.MINECRAFT_1_20_5)) { UnsignedPlayerCommandPacket command = new UnsignedPlayerCommandPacket(); @@ -52,7 +53,7 @@ public class SessionChatBuilder extends ChatBuilderV2 { command.salt = 0L; command.timeStamp = timestamp; command.argumentSignatures = new SessionPlayerCommandPacket.ArgumentSignatures(); - command.lastSeenMessages = new LastSeenMessages(); + command.lastSeenMessages = lastSeenMessages; return command; } } else { @@ -62,8 +63,8 @@ public class SessionChatBuilder extends ChatBuilderV2 { chat.signature = new byte[0]; chat.timestamp = timestamp; chat.salt = 0L; - chat.lastSeenMessages = new LastSeenMessages(); + chat.lastSeenMessages = lastSeenMessages; return chat; } } -} \ No newline at end of file +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatHandler.java index 20268f901..0731f64ed 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionChatHandler.java @@ -29,6 +29,8 @@ import com.velocitypowered.proxy.protocol.packet.chat.ChatQueue; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.concurrent.CompletableFuture; + public class SessionChatHandler implements ChatHandler { private static final Logger logger = LogManager.getLogger(SessionChatHandler.class); @@ -51,8 +53,9 @@ public class SessionChatHandler implements ChatHandler ChatQueue chatQueue = this.player.getChatQueue(); EventManager eventManager = this.server.getEventManager(); PlayerChatEvent toSend = new PlayerChatEvent(player, packet.getMessage()); + CompletableFuture eventFuture = eventManager.fire(toSend); chatQueue.queuePacket( - eventManager.fire(toSend) + newLastSeenMessages -> eventFuture .thenApply(pme -> { PlayerChatEvent.ChatResult chatResult = pme.getResult(); if (!chatResult.isAllowed()) { @@ -70,15 +73,17 @@ public class SessionChatHandler implements ChatHandler } return this.player.getChatBuilderFactory().builder().message(packet.message) .setTimestamp(packet.timestamp) + .setLastSeenMessages(newLastSeenMessages) .toServer(); } - return packet; + return packet.withLastSeenMessages(newLastSeenMessages); }) .exceptionally((ex) -> { logger.error("Exception while handling player chat for {}", player, ex); return null; }), - packet.getTimestamp() + packet.getTimestamp(), + packet.getLastSeenMessages() ); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java index 6984970b3..0e47feede 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionCommandHandler.java @@ -18,13 +18,14 @@ package com.velocitypowered.proxy.protocol.packet.chat.session; import com.velocitypowered.api.event.command.CommandExecuteEvent; -import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.client.ConnectedPlayer; +import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.packet.chat.ChatAcknowledgementPacket; import com.velocitypowered.proxy.protocol.packet.chat.CommandHandler; import java.util.concurrent.CompletableFuture; import net.kyori.adventure.text.Component; +import org.checkerframework.checker.nullness.qual.Nullable; public class SessionCommandHandler implements CommandHandler { @@ -41,78 +42,81 @@ public class SessionCommandHandler implements CommandHandler { + queueCommandResult(this.server, this.player, (event, newLastSeenMessages) -> { + SessionPlayerCommandPacket fixedPacket = packet.withLastSeenMessages(newLastSeenMessages); + CommandExecuteEvent.CommandResult result = event.getResult(); if (result == CommandExecuteEvent.CommandResult.denied()) { - if (packet.isSigned()) { - logger.fatal("A plugin tried to deny a command with signable component(s). " - + "This is not supported. " - + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet); - player.disconnect(Component.text( - "A proxy plugin caused an illegal protocol state. " - + "Contact your network administrator.")); - } - // We seemingly can't actually do this if signed args exist, if not, we can probs keep stuff happy - if (player.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_19_3) && packet.lastSeenMessages != null) { - return CompletableFuture.completedFuture(new ChatAcknowledgementPacket(packet.lastSeenMessages.getOffset())); - } - return CompletableFuture.completedFuture(null); + return CompletableFuture.completedFuture(consumeCommand(fixedPacket)); } - String commandToRun = result.getCommand().orElse(packet.command); + String commandToRun = result.getCommand().orElse(fixedPacket.command); if (result.isForwardToServer()) { - if (packet.isSigned() && commandToRun.equals(packet.command)) { - return CompletableFuture.completedFuture(packet); - } else { - if (packet.isSigned()) { - logger.fatal("A plugin tried to change a command with signed component(s). " - + "This is not supported. " - + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet); - player.disconnect(Component.text( - "A proxy plugin caused an illegal protocol state. " - + "Contact your network administrator.")); - return CompletableFuture.completedFuture(null); - } - - return CompletableFuture.completedFuture(this.player.getChatBuilderFactory() - .builder() - .setTimestamp(packet.timeStamp) - .asPlayer(this.player) - .message("/" + commandToRun) - .toServer()); - } + return CompletableFuture.completedFuture(forwardCommand(fixedPacket, commandToRun)); } return runCommand(this.server, this.player, commandToRun, hasRun -> { - if (!hasRun) { - if (packet.isSigned() && commandToRun.equals(packet.command)) { - return packet; - } else { - if (packet.isSigned()) { - logger.fatal("A plugin tried to change a command with signed component(s). " - + "This is not supported. " - + "Disconnecting player " + player.getUsername() + ". Command packet: " + packet); - player.disconnect(Component.text( - "A proxy plugin caused an illegal protocol state. " - + "Contact your network administrator.")); - return null; - } - - return this.player.getChatBuilderFactory() - .builder() - .setTimestamp(packet.timeStamp) - .asPlayer(this.player) - .message("/" + commandToRun) - .toServer(); - } + if (hasRun) { + return consumeCommand(fixedPacket); } - if (player.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_19_3) && packet.lastSeenMessages != null) { - return new ChatAcknowledgementPacket(packet.lastSeenMessages.getOffset()); - } - return null; + return forwardCommand(fixedPacket, commandToRun); }); - }, packet.command, packet.timeStamp); + }, packet.command, packet.timeStamp, packet.lastSeenMessages); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerChatPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerChatPacket.java index c49170fd9..41f373667 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerChatPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerChatPacket.java @@ -99,4 +99,15 @@ public class SessionPlayerChatPacket implements MinecraftPacket { buf.readBytes(signature); return signature; } + + public SessionPlayerChatPacket withLastSeenMessages(LastSeenMessages lastSeenMessages) { + SessionPlayerChatPacket packet = new SessionPlayerChatPacket(); + packet.message = message; + packet.timestamp = timestamp; + packet.salt = salt; + packet.signed = signed; + packet.signature = signature; + packet.lastSeenMessages = lastSeenMessages; + return packet; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommandPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommandPacket.java index c3b1ae3c0..07374c626 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommandPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/SessionPlayerCommandPacket.java @@ -25,6 +25,8 @@ import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.packet.chat.LastSeenMessages; import com.velocitypowered.proxy.util.except.QuietDecoderException; import io.netty.buffer.ByteBuf; +import org.checkerframework.checker.nullness.qual.Nullable; + import java.time.Instant; import java.util.List; @@ -63,8 +65,7 @@ public class SessionPlayerCommandPacket implements MinecraftPacket { } public boolean isSigned() { - if (salt == 0) return false; - return !lastSeenMessages.isEmpty() || !argumentSignatures.isEmpty(); + return !argumentSignatures.isEmpty(); } @Override @@ -83,6 +84,21 @@ public class SessionPlayerCommandPacket implements MinecraftPacket { '}'; } + public SessionPlayerCommandPacket withLastSeenMessages(@Nullable LastSeenMessages lastSeenMessages) { + if (lastSeenMessages == null) { + UnsignedPlayerCommandPacket packet = new UnsignedPlayerCommandPacket(); + packet.command = command; + return packet; + } + SessionPlayerCommandPacket packet = new SessionPlayerCommandPacket(); + packet.command = command; + packet.timeStamp = timeStamp; + packet.salt = salt; + packet.argumentSignatures = argumentSignatures; + packet.lastSeenMessages = lastSeenMessages; + return packet; + } + public static class ArgumentSignatures { private final List entries; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/UnsignedPlayerCommandPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/UnsignedPlayerCommandPacket.java index dd59e3ff2..b4e26fe07 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/UnsignedPlayerCommandPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/chat/session/UnsignedPlayerCommandPacket.java @@ -19,7 +19,9 @@ package com.velocitypowered.proxy.protocol.packet.chat.session; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.protocol.ProtocolUtils; +import com.velocitypowered.proxy.protocol.packet.chat.LastSeenMessages; import io.netty.buffer.ByteBuf; +import org.checkerframework.checker.nullness.qual.Nullable; public class UnsignedPlayerCommandPacket extends SessionPlayerCommandPacket { @@ -33,6 +35,11 @@ public class UnsignedPlayerCommandPacket extends SessionPlayerCommandPacket { ProtocolUtils.writeString(buf, this.command); } + @Override + public SessionPlayerCommandPacket withLastSeenMessages(@Nullable LastSeenMessages lastSeenMessages) { + return this; + } + public boolean isSigned() { return false; }