From d7799eaf607a7bffdd2de3dcbbb7f6b2ed8d8f33 Mon Sep 17 00:00:00 2001 From: Adrian <68704415+4drian3d@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:15:14 -0500 Subject: [PATCH] Ensure that we send a response to the correct server when in flight (#1234) * Ensure that we send a response to the correct server when in flight * The rest of the codebase treats the hash as a String... As does Mojang. * Support sending the resource packet response to the in-process connection in the ModernResourcePackHandler --------- Co-authored-by: Shane Freeder --- .../client/ClientConfigSessionHandler.java | 4 +++- .../client/ClientPlaySessionHandler.java | 14 ++++++++------ .../resourcepack/LegacyResourcePackHandler.java | 4 ++-- .../resourcepack/ModernResourcePackHandler.java | 10 ++-------- .../resourcepack/ResourcePackHandler.java | 17 +++++++++++++++++ .../ResourcePackResponseBundle.java | 3 ++- 6 files changed, 34 insertions(+), 18 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java index 32ce19c0d..1d566748b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java @@ -99,7 +99,9 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler { player.getConnectionInFlight().ensureConnected().write(packet); } return player.resourcePackHandler().onResourcePackResponse( - new ResourcePackResponseBundle(packet.getId(), packet.getStatus()) + new ResourcePackResponseBundle(packet.getId(), + packet.getHash(), + packet.getStatus()) ); } 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 cd2db3546..ef769b168 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 @@ -302,9 +302,9 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // Handling edge case when packet with FML client handshake (state COMPLETE) // arrives after JoinGame packet from destination server VelocityServerConnection serverConn = - (player.getConnectedServer() == null - && packet.getChannel().equals( - LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) + (player.getConnectedServer() == null + && packet.getChannel().equals( + LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL)) ? player.getConnectionInFlight() : player.getConnectedServer(); MinecraftConnection backendConn = serverConn != null ? serverConn.getConnection() : null; @@ -393,7 +393,9 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { @Override public boolean handle(ResourcePackResponsePacket packet) { return player.resourcePackHandler().onResourcePackResponse( - new ResourcePackResponseBundle(packet.getId(), packet.getStatus())); + new ResourcePackResponseBundle(packet.getId(), + packet.getHash(), + packet.getStatus())); } @Override @@ -552,7 +554,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // Tell the server about the proxy's plugin message channels. ProtocolVersion serverVersion = serverMc.getProtocolVersion(); final Collection channels = server.getChannelRegistrar() - .getChannelsForProtocol(serverMc.getProtocolVersion()); + .getChannelsForProtocol(serverMc.getProtocolVersion()); if (!channels.isEmpty()) { serverMc.delayedWrite(constructChannelsPacket(serverVersion, channels)); } @@ -717,7 +719,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { if (suggestion.getTooltip() != null && suggestion.getTooltip() instanceof VelocityBrigadierMessage) { tooltip = new ComponentHolder(player.getProtocolVersion(), - ((VelocityBrigadierMessage) suggestion.getTooltip()).asComponent()); + ((VelocityBrigadierMessage) suggestion.getTooltip()).asComponent()); } response.getOffers().add(new Offer(offer, tooltip)); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java index 3b211bca5..b3797e751 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java @@ -108,6 +108,7 @@ public sealed class LegacyResourcePackHandler extends ResourcePackHandler break; } onResourcePackResponse(new ResourcePackResponseBundle(queued.getId(), + new String(queued.getHash()), PlayerResourcePackStatusEvent.Status.DECLINED)); queued = null; } @@ -165,8 +166,7 @@ public sealed class LegacyResourcePackHandler extends ResourcePackHandler player.getConnection().eventLoop().execute(this::tickResourcePackQueue); } - return queued != null - && queued.getOriginalOrigin() != ResourcePackInfo.Origin.DOWNSTREAM_SERVER; + return handleResponseResult(queued, bundle); } protected boolean shouldDisconnectForForcePack(final PlayerResourcePackStatusEvent event) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java index 1fb5aef91..f41895125 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java @@ -33,14 +33,11 @@ import net.kyori.adventure.resource.ResourcePackRequest; import net.kyori.adventure.text.Component; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Modern (Minecraft 1.20.3+) ResourcePackHandler */ public final class ModernResourcePackHandler extends ResourcePackHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(ModernResourcePackHandler.class); private final ListMultimap outstandingResourcePacks = Multimaps.newListMultimap(new ConcurrentHashMap<>(), LinkedList::new); private final Map pendingResourcePacks = new ConcurrentHashMap<>(); @@ -102,9 +99,7 @@ public final class ModernResourcePackHandler extends ResourcePackHandler { @Override public void queueResourcePack(final @NotNull ResourcePackRequest request) { if (request.packs().size() > 1) { - player.getBundleHandler().bundlePackets(() -> { - super.queueResourcePack(request); - }); + player.getBundleHandler().bundlePackets(() -> super.queueResourcePack(request)); } else { super.queueResourcePack(request); } @@ -170,7 +165,6 @@ public final class ModernResourcePackHandler extends ResourcePackHandler { player.getConnection().eventLoop().execute(() -> tickResourcePackQueue(uuid)); } - return queued != null - && queued.getOriginalOrigin() != ResourcePackInfo.Origin.DOWNSTREAM_SERVER; + return handleResponseResult(queued, bundle); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java index d779ca297..42a525b00 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java @@ -23,6 +23,7 @@ import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.client.ConnectedPlayer; import com.velocitypowered.proxy.connection.player.VelocityResourcePackInfo; import com.velocitypowered.proxy.protocol.packet.ResourcePackRequestPacket; +import com.velocitypowered.proxy.protocol.packet.ResourcePackResponsePacket; import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder; import io.netty.buffer.ByteBufUtil; import java.util.Collection; @@ -139,4 +140,20 @@ public abstract sealed class ResourcePackHandler */ public abstract boolean onResourcePackResponse( final @NotNull ResourcePackResponseBundle bundle); + + protected boolean handleResponseResult( + final @Nullable ResourcePackInfo queued, + final @NotNull ResourcePackResponseBundle bundle + ) { + final boolean handled = queued != null + && queued.getOriginalOrigin() != ResourcePackInfo.Origin.DOWNSTREAM_SERVER; + if (!handled) { + if (player.getConnectionInFlight() != null + && player.getConnectionInFlight().getConnection() != null) { + player.getConnectionInFlight().getConnection().write(new ResourcePackResponsePacket( + bundle.uuid(), bundle.hash(), bundle.status())); + } + } + return handled; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackResponseBundle.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackResponseBundle.java index 9465c3be3..052f7080a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackResponseBundle.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackResponseBundle.java @@ -21,5 +21,6 @@ import com.velocitypowered.api.event.player.PlayerResourcePackStatusEvent; import java.util.UUID; @SuppressWarnings("checkstyle:MissingJavadocType") -public record ResourcePackResponseBundle(UUID uuid, PlayerResourcePackStatusEvent.Status status) { +public record ResourcePackResponseBundle(UUID uuid, String hash, + PlayerResourcePackStatusEvent.Status status) { }