From 750e171398a22caa2a208a0b9bd4265029641042 Mon Sep 17 00:00:00 2001 From: SendableMetatype <263203301+SendableMetatype@users.noreply.github.com> Date: Mon, 11 May 2026 20:58:25 +0200 Subject: [PATCH 1/2] Fix handshake hostname length limit and plugin message drop during join --- .../connection/client/ClientPlaySessionHandler.java | 11 +++-------- .../proxy/protocol/packet/HandshakePacket.java | 9 ++------- 2 files changed, 5 insertions(+), 15 deletions(-) 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 beecf3ac32..2f824938ab 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 @@ -38,7 +38,6 @@ import com.velocitypowered.proxy.connection.backend.BackendConnectionPhases; import com.velocitypowered.proxy.connection.backend.BungeeCordMessageResponder; import com.velocitypowered.proxy.connection.backend.VelocityServerConnection; -import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants; import com.velocitypowered.proxy.connection.player.resourcepack.ResourcePackResponseBundle; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.StateRegistry; @@ -357,13 +356,9 @@ public boolean handle(TabCompleteRequestPacket packet) { @Override public boolean handle(PluginMessagePacket packet) { - // 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.getConnectionInFlight() : player.getConnectedServer(); + final VelocityServerConnection connected = player.getConnectedServer(); + final VelocityServerConnection serverConn = + connected != null ? connected : player.getConnectionInFlight(); MinecraftConnection backendConn = serverConn != null ? serverConn.getConnection() : null; if (serverConn != null && backendConn != null) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HandshakePacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HandshakePacket.java index 88cb3688bd..3f65f3c0ab 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HandshakePacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HandshakePacket.java @@ -17,8 +17,6 @@ package com.velocitypowered.proxy.protocol.packet; -import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN; - import com.velocitypowered.api.network.HandshakeIntent; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; @@ -29,9 +27,6 @@ public class HandshakePacket implements MinecraftPacket { - // This size was chosen to ensure Forge clients can still connect even with very long hostnames. - // While DNS technically allows any character to be used, in practice ASCII is used. - private static final int MAXIMUM_HOSTNAME_LENGTH = 255 + HANDSHAKE_HOSTNAME_TOKEN.length() + 1; private ProtocolVersion protocolVersion; private String serverAddress = ""; private int port; @@ -89,7 +84,7 @@ public String toString() { public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion ignored) { int realProtocolVersion = ProtocolUtils.readVarInt(buf); this.protocolVersion = ProtocolVersion.getProtocolVersion(realProtocolVersion); - this.serverAddress = ProtocolUtils.readString(buf, MAXIMUM_HOSTNAME_LENGTH); + this.serverAddress = ProtocolUtils.readString(buf, Short.MAX_VALUE); this.port = buf.readUnsignedShort(); this.nextStatus = ProtocolUtils.readVarInt(buf); this.intent = HandshakeIntent.getById(nextStatus); @@ -117,7 +112,7 @@ public int decodeExpectedMinLength(ByteBuf buf, ProtocolUtils.Direction directio @Override public int decodeExpectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { - return 9 + (MAXIMUM_HOSTNAME_LENGTH * 3); + return 9 + (Short.MAX_VALUE * 3); } @Override From 835299b87948cbc2c022a2c73795465913c607c5 Mon Sep 17 00:00:00 2001 From: SendableMetatype <263203301+SendableMetatype@users.noreply.github.com> Date: Mon, 11 May 2026 20:58:30 +0200 Subject: [PATCH 2/2] Fix LegacyPingDecoder false positive on 0xFE varint length --- .../protocol/netty/LegacyPingDecoder.java | 83 +++++++++++++++++-- 1 file changed, 77 insertions(+), 6 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java index a6c2c065f5..3637624d07 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java @@ -28,6 +28,8 @@ import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; /** * Decodes Minecraft 1.3-1.6.4 server ping requests. @@ -35,6 +37,12 @@ public class LegacyPingDecoder extends ByteToMessageDecoder { private static final String MC_1_6_CHANNEL = "MC|PingHost"; + // FE and FE 01 are complete legacy pings, but also the beginning of a modern 254-byte frame. + // Give a fragmented modern frame a chance to supply its packet ID before choosing legacy. + private static final long LEGACY_PING_GRACE_PERIOD_MILLIS = 100; + + private ScheduledFuture pendingLegacyPing; + private int pendingLegacyPingLength; @Override protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) throws Exception { @@ -52,27 +60,90 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) t if (first == 0xfe) { // possibly a ping if (!in.isReadable()) { - out.add(new LegacyPingPacket(LegacyMinecraftPingVersion.MINECRAFT_1_3)); + in.readerIndex(originalReaderIndex); + scheduleLegacyPing(ctx, 1, LegacyMinecraftPingVersion.MINECRAFT_1_3); return; } short next = in.readUnsignedByte(); - if (next == 1 && !in.isReadable()) { - out.add(new LegacyPingPacket(LegacyMinecraftPingVersion.MINECRAFT_1_4)); - return; + if (next == 1) { + if (!in.isReadable()) { + in.readerIndex(originalReaderIndex); + scheduleLegacyPing(ctx, 2, LegacyMinecraftPingVersion.MINECRAFT_1_4); + return; + } + if (in.getUnsignedByte(in.readerIndex()) == 0xFA) { + cancelPendingLegacyPing(); + out.add(readExtended16Data(in)); + return; + } } - // We got a 1.6.x ping. Let's chomp off the stuff we don't need. - out.add(readExtended16Data(in)); + // Not a legacy ping. Reset and let the modern decoder handle it. + cancelPendingLegacyPing(); + in.readerIndex(originalReaderIndex); + ctx.pipeline().remove(this); } else if (first == 0x02 && in.isReadable()) { + cancelPendingLegacyPing(); in.skipBytes(in.readableBytes()); out.add(new LegacyHandshakePacket()); } else { + cancelPendingLegacyPing(); in.readerIndex(originalReaderIndex); ctx.pipeline().remove(this); } } + private void scheduleLegacyPing(ChannelHandlerContext ctx, int length, + LegacyMinecraftPingVersion version) { + if (pendingLegacyPing != null && pendingLegacyPingLength == length) { + return; + } + + cancelPendingLegacyPing(); + pendingLegacyPingLength = length; + pendingLegacyPing = ctx.executor().schedule(() -> { + pendingLegacyPing = null; + if (!ctx.channel().isActive() || pendingLegacyPingLength != length) { + return; + } + + ByteBuf buffer = internalBuffer(); + if (buffer.readableBytes() != length) { + pendingLegacyPingLength = 0; + return; + } + + buffer.skipBytes(length); + pendingLegacyPingLength = 0; + ctx.fireChannelRead(new LegacyPingPacket(version)); + ctx.fireChannelReadComplete(); + if (ctx.pipeline().context(this) != null) { + ctx.pipeline().remove(this); + } + }, LEGACY_PING_GRACE_PERIOD_MILLIS, TimeUnit.MILLISECONDS); + } + + private void cancelPendingLegacyPing() { + if (pendingLegacyPing != null) { + pendingLegacyPing.cancel(false); + pendingLegacyPing = null; + } + pendingLegacyPingLength = 0; + } + + @Override + protected void handlerRemoved0(ChannelHandlerContext ctx) throws Exception { + cancelPendingLegacyPing(); + super.handlerRemoved0(ctx); + } + + @Override + public void channelInactive(ChannelHandlerContext ctx) throws Exception { + cancelPendingLegacyPing(); + super.channelInactive(ctx); + } + private static LegacyPingPacket readExtended16Data(ByteBuf in) { in.skipBytes(1); String channelName = readLegacyString(in);