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 <theboyetronic@gmail.com>

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())
     );
   }
 

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
+        (player.getConnectedServer() == null
-                    && packet.getChannel().equals(
+            && packet.getChannel().equals(
-                            LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL))
+            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<String> 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));
             }

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
+    return handleResponseResult(queued, bundle);
-            && queued.getOriginalOrigin() != ResourcePackInfo.Origin.DOWNSTREAM_SERVER;
   }
 
   protected boolean shouldDisconnectForForcePack(final PlayerResourcePackStatusEvent event) {

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<UUID, ResourcePackInfo> outstandingResourcePacks =
       Multimaps.newListMultimap(new ConcurrentHashMap<>(), LinkedList::new);
   private final Map<UUID, ResourcePackInfo> 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(() -> {
+      player.getBundleHandler().bundlePackets(() -> super.queueResourcePack(request));
-        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
+    return handleResponseResult(queued, bundle);
-            && queued.getOriginalOrigin() != ResourcePackInfo.Origin.DOWNSTREAM_SERVER;
   }
 }

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;
+  }
 }

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) {
 }