From aa4f24c9159ab586ba7f4843e4d36bf1700cdafd Mon Sep 17 00:00:00 2001 From: Take Weiland Date: Tue, 8 Jun 2021 21:47:37 +0200 Subject: [PATCH] Fix vanilla packet splitter remote detection and add advancement packet splitting (#7802) --- .../fml/network/NetworkHooks.java | 2 + .../fml/server/ServerLifecycleHooks.java | 2 - .../network/ForgeConnectionNetworkFilter.java | 40 +++++++++++--- .../network/NetworkFilters.java | 10 ++-- .../network/VanillaPacketSplitter.java | 55 ++++++++++++++----- 5 files changed, 80 insertions(+), 29 deletions(-) diff --git a/src/main/java/net/minecraftforge/fml/network/NetworkHooks.java b/src/main/java/net/minecraftforge/fml/network/NetworkHooks.java index 775a56d9fb..a493d4331e 100644 --- a/src/main/java/net/minecraftforge/fml/network/NetworkHooks.java +++ b/src/main/java/net/minecraftforge/fml/network/NetworkHooks.java @@ -52,6 +52,7 @@ import net.minecraft.util.math.BlockPos; import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.event.entity.player.PlayerContainerEvent; import net.minecraftforge.fml.config.ConfigTracker; +import net.minecraftforge.network.NetworkFilters; import javax.annotation.Nullable; @@ -117,6 +118,7 @@ public class NetworkHooks } public synchronized static void sendMCRegistryPackets(NetworkManager manager, String direction) { + NetworkFilters.injectIfNecessary(manager); final Set resourceLocations = NetworkRegistry.buildChannelVersions().keySet().stream(). filter(rl -> !Objects.equals(rl.getNamespace(), "minecraft")). collect(Collectors.toSet()); diff --git a/src/main/java/net/minecraftforge/fml/server/ServerLifecycleHooks.java b/src/main/java/net/minecraftforge/fml/server/ServerLifecycleHooks.java index 8dba60d8b8..edd51f1ba7 100644 --- a/src/main/java/net/minecraftforge/fml/server/ServerLifecycleHooks.java +++ b/src/main/java/net/minecraftforge/fml/server/ServerLifecycleHooks.java @@ -68,7 +68,6 @@ import net.minecraftforge.fml.loading.moddiscovery.ModFile; import net.minecraftforge.fml.packs.ModFileResourcePack; import net.minecraftforge.fml.packs.ResourcePackLoader; import net.minecraftforge.forgespi.language.IModInfo; -import net.minecraftforge.network.NetworkFilters; import net.minecraftforge.registries.GameData; public class ServerLifecycleHooks @@ -169,7 +168,6 @@ public class ServerLifecycleHooks if (packet.getIntention() == ProtocolType.STATUS) return true; NetworkHooks.registerServerLoginChannel(manager, packet); - NetworkFilters.injectIfNecessary(manager); return true; } diff --git a/src/main/java/net/minecraftforge/network/ForgeConnectionNetworkFilter.java b/src/main/java/net/minecraftforge/network/ForgeConnectionNetworkFilter.java index fa3474c731..efa50d08ee 100644 --- a/src/main/java/net/minecraftforge/network/ForgeConnectionNetworkFilter.java +++ b/src/main/java/net/minecraftforge/network/ForgeConnectionNetworkFilter.java @@ -20,15 +20,19 @@ package net.minecraftforge.network; import java.util.List; +import java.util.Map; +import java.util.function.BiConsumer; + +import javax.annotation.Nullable; import io.netty.channel.ChannelHandler; import net.minecraft.network.IPacket; import net.minecraft.network.NetworkManager; import net.minecraft.network.PacketDirection; import net.minecraft.network.ProtocolType; +import net.minecraft.network.play.server.SAdvancementInfoPacket; import net.minecraft.network.play.server.STagsListPacket; import net.minecraft.network.play.server.SUpdateRecipesPacket; -import net.minecraftforge.fml.network.NetworkHooks; import com.google.common.collect.ImmutableMap; @@ -39,21 +43,41 @@ import com.google.common.collect.ImmutableMap; public class ForgeConnectionNetworkFilter extends VanillaPacketFilter { + @Deprecated public ForgeConnectionNetworkFilter() { - super( - ImmutableMap.of( - SUpdateRecipesPacket.class, ForgeConnectionNetworkFilter::splitPacket, - STagsListPacket.class, ForgeConnectionNetworkFilter::splitPacket - ) - ); + this(null); + } + + public ForgeConnectionNetworkFilter(@Nullable NetworkManager manager) + { + super(buildHandlers(manager)); + } + + private static Map>, BiConsumer, List>>> buildHandlers(@Nullable NetworkManager manager) + { + + VanillaPacketSplitter.RemoteCompatibility compatibility = manager == null ? VanillaPacketSplitter.RemoteCompatibility.ABSENT : VanillaPacketSplitter.getRemoteCompatibility(manager); + if (compatibility == VanillaPacketSplitter.RemoteCompatibility.ABSENT) + { + return ImmutableMap.of(); + } + ImmutableMap.Builder>, BiConsumer, List>>> builder = ImmutableMap.>, BiConsumer, List>>>builder() + .put(SUpdateRecipesPacket.class, ForgeConnectionNetworkFilter::splitPacket) + .put(STagsListPacket.class, ForgeConnectionNetworkFilter::splitPacket); + + if (compatibility == VanillaPacketSplitter.RemoteCompatibility.V11) + { + builder.put(SAdvancementInfoPacket.class, ForgeConnectionNetworkFilter::splitPacket); + } + return builder.build(); } @Override protected boolean isNecessary(NetworkManager manager) { // not needed on local connections, because packets are not encoded to bytes there - return !manager.isMemoryConnection() && !VanillaPacketSplitter.isRemoteCompatible(manager); + return !manager.isMemoryConnection() && VanillaPacketSplitter.isRemoteCompatible(manager); } private static void splitPacket(IPacket packet, List> out) diff --git a/src/main/java/net/minecraftforge/network/NetworkFilters.java b/src/main/java/net/minecraftforge/network/NetworkFilters.java index 92a2de9a74..199135f734 100644 --- a/src/main/java/net/minecraftforge/network/NetworkFilters.java +++ b/src/main/java/net/minecraftforge/network/NetworkFilters.java @@ -20,6 +20,7 @@ package net.minecraftforge.network; import java.util.Map; +import java.util.function.Function; import net.minecraft.network.NetworkManager; @@ -33,14 +34,15 @@ public class NetworkFilters private static final Logger LOGGER = LogManager.getLogger(); - private static final Map instances = ImmutableMap.of( - "forge:vanilla_filter", new VanillaConnectionNetworkFilter(), - "forge:forge_fixes", new ForgeConnectionNetworkFilter() + private static final Map> instances = ImmutableMap.of( + "forge:vanilla_filter", manager -> new VanillaConnectionNetworkFilter(), + "forge:forge_fixes", ForgeConnectionNetworkFilter::new ); public static void injectIfNecessary(NetworkManager manager) { - instances.forEach((key, filter) -> { + instances.forEach((key, filterFactory) -> { + VanillaPacketFilter filter = filterFactory.apply(manager); if (filter.isNecessary(manager)) { manager.channel().pipeline().addBefore("packet_handler", key, filter); diff --git a/src/main/java/net/minecraftforge/network/VanillaPacketSplitter.java b/src/main/java/net/minecraftforge/network/VanillaPacketSplitter.java index d036514e8a..1dfe195cc9 100644 --- a/src/main/java/net/minecraftforge/network/VanillaPacketSplitter.java +++ b/src/main/java/net/minecraftforge/network/VanillaPacketSplitter.java @@ -32,9 +32,8 @@ import net.minecraft.client.Minecraft; import net.minecraft.client.network.play.ClientPlayNetHandler; import net.minecraft.network.*; import net.minecraft.network.play.server.SCustomPayloadPlayPacket; -import net.minecraft.network.play.server.STagsListPacket; -import net.minecraft.network.play.server.SUpdateRecipesPacket; import net.minecraft.util.ResourceLocation; +import net.minecraftforge.fml.network.FMLConnectionData; import net.minecraftforge.fml.network.NetworkDirection; import net.minecraftforge.fml.network.NetworkEvent; import net.minecraftforge.fml.network.NetworkHooks; @@ -56,12 +55,13 @@ public class VanillaPacketSplitter private static final Logger LOGGER = LogManager.getLogger(); private static final ResourceLocation CHANNEL = new ResourceLocation("forge", "split"); + // TODO 1.17: bump this to version 1.1 and remove dummy channel + // Version 1.1 removed client-side whitelist + // we don't bump actual channel version because that would prevent old clients from connecting + // but we still need to detect new clients, so we add a dummy channel just for that. + private static final ResourceLocation V11_DUMMY_CHANNEL = new ResourceLocation("forge", "split_11"); private static final String VERSION = "1.0"; - - private static final Set>> ALLOWED_PACKETS = ImmutableSet.of( - SUpdateRecipesPacket.class, - STagsListPacket.class - ); + private static final String VERSION_11 = "1.1"; private static final int PROTOCOL_MAX = 2097152; @@ -72,13 +72,14 @@ public class VanillaPacketSplitter private static final byte STATE_FIRST = 1; private static final byte STATE_LAST = 2; - private static EventNetworkChannel channel; - public static void register() { Predicate versionCheck = NetworkRegistry.acceptMissingOr(VERSION); - channel = NetworkRegistry.newEventChannel(CHANNEL, () -> VERSION, versionCheck, versionCheck); + EventNetworkChannel channel = NetworkRegistry.newEventChannel(CHANNEL, () -> VERSION, versionCheck, versionCheck); channel.addListener(VanillaPacketSplitter::onClientPacket); + + Predicate version11Check = NetworkRegistry.acceptMissingOr(VERSION_11); + NetworkRegistry.newEventChannel(V11_DUMMY_CHANNEL, () -> VERSION_11, version11Check, version11Check); } /** @@ -184,10 +185,6 @@ public class VanillaPacketSplitter { LOGGER.error("Received invalid packet ID {} in forge:split", packetId); } - else if (!ALLOWED_PACKETS.contains(packet.getClass())) - { - LOGGER.error("Received not allowed packet type {} in forge:split", packet); - } else { try @@ -206,8 +203,36 @@ public class VanillaPacketSplitter } } + public enum RemoteCompatibility + { + ABSENT, + V10_LEGACY, + V11 + } + + public static RemoteCompatibility getRemoteCompatibility(NetworkManager manager) + { + FMLConnectionData connectionData = NetworkHooks.getConnectionData(manager); + if (connectionData == null) + { + return RemoteCompatibility.ABSENT; + } + else if (connectionData.getChannels().containsKey(V11_DUMMY_CHANNEL)) + { + return RemoteCompatibility.V11; + } + else if (connectionData.getChannels().containsKey(CHANNEL)) + { + return RemoteCompatibility.V10_LEGACY; + } + else + { + return RemoteCompatibility.ABSENT; + } + } + public static boolean isRemoteCompatible(NetworkManager manager) { - return !NetworkHooks.isVanillaConnection(manager) && channel.isRemotePresent(manager); + return getRemoteCompatibility(manager) != RemoteCompatibility.ABSENT; } }