Substantially-improved ancient server list ping support.

Velocity now flawlessly supports 1.6 and below server list pings along
with a notice to reconnect with a more modern version of Minecraft if
possible.

proxy/src/main/java/com/velocitypowered/proxy/connection/client/HandshakeSessionHandler.java

@@ -2,15 +2,11 @@ package com.velocitypowered.proxy.connection.client;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.velocitypowered.api.event.connection.ConnectionHandshakeEvent;
-import com.velocitypowered.api.event.proxy.ProxyPingEvent;
 import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.proxy.InboundConnection;
-import com.velocitypowered.api.proxy.server.ServerPing;
 import com.velocitypowered.proxy.VelocityServer;
 import com.velocitypowered.proxy.config.PlayerInfoForwarding;
-import com.velocitypowered.proxy.config.VelocityConfiguration;
 import com.velocitypowered.proxy.connection.ConnectionType;
 import com.velocitypowered.proxy.connection.ConnectionTypes;
 import com.velocitypowered.proxy.connection.MinecraftConnection;
@@ -23,7 +19,6 @@ import com.velocitypowered.proxy.protocol.packet.Handshake;
 import com.velocitypowered.proxy.protocol.packet.LegacyDisconnect;
 import com.velocitypowered.proxy.protocol.packet.LegacyHandshake;
 import com.velocitypowered.proxy.protocol.packet.LegacyPing;
-import com.velocitypowered.proxy.protocol.packet.LegacyPingResponse;
 import io.netty.buffer.ByteBuf;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -45,23 +40,10 @@ public class HandshakeSessionHandler implements MinecraftSessionHandler {
   @Override
   public boolean handle(LegacyPing packet) {
     connection.setProtocolVersion(ProtocolVersion.LEGACY);
-    VelocityConfiguration configuration = server.getConfiguration();
-    ServerPing ping = new ServerPing(
-        new ServerPing.Version(ProtocolVersion.MAXIMUM_VERSION.getProtocol(),
-            "Velocity " + ProtocolVersion.SUPPORTED_VERSION_STRING),
-        new ServerPing.Players(server.getPlayerCount(), configuration.getShowMaxPlayers(),
-            ImmutableList.of()),
-        configuration.getMotdComponent(),
-        null,
-        null
-    );
-    ProxyPingEvent event = new ProxyPingEvent(new LegacyInboundConnection(connection), ping);
-    server.getEventManager().fire(event)
-        .thenRunAsync(() -> {
-          // The disconnect packet is the same as the server response one.
-          LegacyPingResponse response = LegacyPingResponse.from(event.getPing());
-          connection.closeWith(LegacyDisconnect.fromPingResponse(response));
-        }, connection.eventLoop());
+    StatusSessionHandler handler = new StatusSessionHandler(server, connection,
+        new LegacyInboundConnection(connection, packet));
+    connection.setSessionHandler(handler);
+    handler.handle(packet);
     return true;
   }
 
@@ -178,9 +160,12 @@ public class HandshakeSessionHandler implements MinecraftSessionHandler {
   private static class LegacyInboundConnection implements InboundConnection {
 
     private final MinecraftConnection connection;
+    private final LegacyPing ping;
 
-    private LegacyInboundConnection(MinecraftConnection connection) {
+    private LegacyInboundConnection(MinecraftConnection connection,
+        LegacyPing ping) {
       this.connection = connection;
+      this.ping = ping;
     }
 
     @Override
@@ -190,7 +175,7 @@ public class HandshakeSessionHandler implements MinecraftSessionHandler {
 
     @Override
     public Optional<InetSocketAddress> getVirtualHost() {
-      return Optional.empty();
+      return Optional.ofNullable(ping.getVhost());
     }
 
     @Override

proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java

@@ -10,6 +10,8 @@ import com.velocitypowered.proxy.VelocityServer;
 import com.velocitypowered.proxy.config.VelocityConfiguration;
 import com.velocitypowered.proxy.connection.MinecraftConnection;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
+import com.velocitypowered.proxy.protocol.packet.LegacyDisconnect;
+import com.velocitypowered.proxy.protocol.packet.LegacyPing;
 import com.velocitypowered.proxy.protocol.packet.StatusPing;
 import com.velocitypowered.proxy.protocol.packet.StatusRequest;
 import com.velocitypowered.proxy.protocol.packet.StatusResponse;
@@ -28,19 +30,11 @@ public class StatusSessionHandler implements MinecraftSessionHandler {
     this.inboundWrapper = inboundWrapper;
   }
 
-  @Override
-  public boolean handle(StatusPing packet) {
-    connection.closeWith(packet);
-    return true;
-  }
-
-  @Override
-  public boolean handle(StatusRequest packet) {
+  private ServerPing createInitialPing() {
     VelocityConfiguration configuration = server.getConfiguration();
-
     ProtocolVersion shownVersion = ProtocolVersion.isSupported(connection.getProtocolVersion())
         ? connection.getProtocolVersion() : ProtocolVersion.MAXIMUM_VERSION;
-    ServerPing initialPing = new ServerPing(
+    return new ServerPing(
         new ServerPing.Version(shownVersion.getProtocol(),
             "Velocity " + ProtocolVersion.SUPPORTED_VERSION_STRING),
         new ServerPing.Players(server.getPlayerCount(), configuration.getShowMaxPlayers(),
@@ -49,7 +43,29 @@ public class StatusSessionHandler implements MinecraftSessionHandler {
         configuration.getFavicon().orElse(null),
         configuration.isAnnounceForge() ? ModInfo.DEFAULT : null
     );
+  }
 
+  @Override
+  public boolean handle(LegacyPing packet) {
+    ServerPing initialPing = createInitialPing();
+    ProxyPingEvent event = new ProxyPingEvent(inboundWrapper, initialPing);
+    server.getEventManager().fire(event)
+        .thenRunAsync(() -> {
+          connection.closeWith(LegacyDisconnect.fromServerPing(event.getPing(),
+              packet.getVersion()));
+        }, connection.eventLoop());
+    return true;
+  }
+
+  @Override
+  public boolean handle(StatusPing packet) {
+    connection.closeWith(packet);
+    return true;
+  }
+
+  @Override
+  public boolean handle(StatusRequest packet) {
+    ServerPing initialPing = createInitialPing();
     ProxyPingEvent event = new ProxyPingEvent(inboundWrapper, initialPing);
     server.getEventManager().fire(event)
         .thenRunAsync(

proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java

@@ -1,30 +1,73 @@
 package com.velocitypowered.proxy.protocol.netty;
 
+import static com.velocitypowered.proxy.protocol.util.NettyPreconditions.checkFrame;
+
 import com.velocitypowered.proxy.protocol.packet.LegacyHandshake;
 import com.velocitypowered.proxy.protocol.packet.LegacyPing;
+import com.velocitypowered.proxy.protocol.packet.legacyping.LegacyMinecraftPingVersion;
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 
 public class LegacyPingDecoder extends ByteToMessageDecoder {
 
+  private static final String MC_1_6_CHANNEL = "MC|PingHost";
+
   @Override
   protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-    if (in.readableBytes() < 2) {
+    if (!in.isReadable()) {
       return;
     }
 
-    short first = in.getUnsignedByte(in.readerIndex());
-    short second = in.getUnsignedByte(in.readerIndex() + 1);
-    if (first == 0xfe && second == 0x01) {
-      in.skipBytes(in.readableBytes());
-      out.add(new LegacyPing());
+    int originalReaderIndex = in.readerIndex();
+    short first = in.readUnsignedByte();
+    if (first == 0xfe) {
+      // possibly a ping
+      if (!in.isReadable()) {
+        out.add(new LegacyPing(LegacyMinecraftPingVersion.MINECRAFT_1_3));
+        return;
+      }
+
+      short next = in.readUnsignedByte();
+      if (next == 1 && !in.isReadable()) {
+        out.add(new LegacyPing(LegacyMinecraftPingVersion.MINECRAFT_1_4));
+        return;
+      }
+
+      // We got a 1.6.x ping. Let's chomp off the stuff we don't need.
+      out.add(readExtended16Data(in));
     } else if (first == 0x02) {
       in.skipBytes(in.readableBytes());
       out.add(new LegacyHandshake());
     } else {
+      in.readerIndex(originalReaderIndex);
       ctx.pipeline().remove(this);
     }
   }
+
+  private static LegacyPing readExtended16Data(ByteBuf in) {
+    in.skipBytes(1);
+    String channelName = readLegacyString(in);
+    if (!channelName.equals(MC_1_6_CHANNEL)) {
+      throw new IllegalArgumentException("Didn't find correct channel");
+    }
+    in.skipBytes(3);
+    String hostname = readLegacyString(in);
+    int port = in.readInt();
+
+    return new LegacyPing(LegacyMinecraftPingVersion.MINECRAFT_1_6, InetSocketAddress
+        .createUnresolved(hostname, port));
+  }
+
+  private static String readLegacyString(ByteBuf buf) {
+    int len = buf.readShort() * Character.BYTES;
+    checkFrame(buf.isReadable(len), "String length %s is too large for available bytes %d",
+        len, buf.readableBytes());
+    String str = buf.toString(buf.readerIndex(), len, StandardCharsets.UTF_16BE);
+    buf.skipBytes(len);
+    return str;
+  }
 }

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyDisconnect.java

@@ -1,10 +1,21 @@
 package com.velocitypowered.proxy.protocol.packet;
 
+import static net.kyori.text.serializer.ComponentSerializers.LEGACY;
+import static net.kyori.text.serializer.ComponentSerializers.PLAIN;
+
+import com.google.common.collect.ImmutableList;
+import com.velocitypowered.api.proxy.server.ServerPing;
+import com.velocitypowered.api.proxy.server.ServerPing.Players;
+import com.velocitypowered.proxy.protocol.packet.legacyping.LegacyMinecraftPingVersion;
 import net.kyori.text.TextComponent;
 import net.kyori.text.serializer.ComponentSerializers;
 
 public class LegacyDisconnect {
 
+  private static final ServerPing.Players FAKE_PLAYERS = new ServerPing.Players(0, 0,
+      ImmutableList.of());
+  private static final String LEGACY_COLOR_CODE = "\u00a7";
+
   private final String reason;
 
   private LegacyDisconnect(String reason) {
@@ -12,20 +23,48 @@ public class LegacyDisconnect {
   }
 
   /**
-   * Converts a legacy response into an legacy disconnect packet.
-   * @param response the response to convert
+   * Converts a modern server list ping response into an legacy disconnect packet.
+   * @param response the server ping to convert
+   * @param version the requesting clients' version
    * @return the disconnect packet
    */
-  public static LegacyDisconnect fromPingResponse(LegacyPingResponse response) {
-    String kickMessage = String.join("\0",
-        "§1",
-        Integer.toString(response.getProtocolVersion()),
-        response.getServerVersion(),
-        response.getMotd(),
-        Integer.toString(response.getPlayersOnline()),
-        Integer.toString(response.getPlayersMax())
-    );
-    return new LegacyDisconnect(kickMessage);
+  @SuppressWarnings("deprecation") // we use these on purpose to service older clients!
+  public static LegacyDisconnect fromServerPing(ServerPing response,
+      LegacyMinecraftPingVersion version) {
+    Players players = response.getPlayers().orElse(FAKE_PLAYERS);
+
+    switch (version) {
+      case MINECRAFT_1_3:
+        // Minecraft 1.3 and below use the section symbol as a delimiter. Accordingly, we must
+        // remove all section symbols, along with fetching just the first line of an (unformatted)
+        // MOTD.
+        return new LegacyDisconnect(String.join(LEGACY_COLOR_CODE,
+            cleanSectionSymbol(getFirstLine(PLAIN.serialize(response.getDescription()))),
+            Integer.toString(players.getOnline()),
+            Integer.toString(players.getMax())));
+      case MINECRAFT_1_4:
+      case MINECRAFT_1_6:
+        // Minecraft 1.4-1.6 provide support for more fields, and additionally support color codes.
+        return new LegacyDisconnect(String.join("\0",
+            LEGACY_COLOR_CODE + "1",
+            Integer.toString(response.getVersion().getProtocol()),
+            response.getVersion().getName(),
+            getFirstLine(LEGACY.serialize(response.getDescription())),
+            Integer.toString(players.getOnline()),
+            Integer.toString(players.getMax())
+        ));
+      default:
+        throw new IllegalArgumentException("Unknown version " + version);
+    }
+  }
+
+  private static String cleanSectionSymbol(String string) {
+    return string.replaceAll(LEGACY_COLOR_CODE, "");
+  }
+
+  private static String getFirstLine(String legacyMotd) {
+    int newline = legacyMotd.indexOf('\n');
+    return newline == -1 ? legacyMotd : legacyMotd.substring(0, newline);
   }
 
   /**

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyPing.java

@@ -4,10 +4,36 @@ import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.MinecraftPacket;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
+import com.velocitypowered.proxy.protocol.packet.legacyping.LegacyMinecraftPingVersion;
 import io.netty.buffer.ByteBuf;
+import java.net.InetSocketAddress;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class LegacyPing implements MinecraftPacket {
 
+  private final LegacyMinecraftPingVersion version;
+  @Nullable
+  private final InetSocketAddress vhost;
+
+  public LegacyPing(LegacyMinecraftPingVersion version) {
+    this.version = version;
+    this.vhost = null;
+  }
+
+  public LegacyPing(LegacyMinecraftPingVersion version, InetSocketAddress vhost) {
+    this.version = version;
+    this.vhost = vhost;
+  }
+
+  public LegacyMinecraftPingVersion getVersion() {
+    return version;
+  }
+
+  @Nullable
+  public InetSocketAddress getVhost() {
+    return vhost;
+  }
+
   @Override
   public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) {
     throw new UnsupportedOperationException();

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/LegacyPingResponse.java

@@ -1,70 +0,0 @@
-package com.velocitypowered.proxy.protocol.packet;
-
-import com.google.common.collect.ImmutableList;
-import com.velocitypowered.api.proxy.server.ServerPing;
-import com.velocitypowered.api.network.ProtocolVersion;
-import net.kyori.text.serializer.ComponentSerializers;
-
-public class LegacyPingResponse {
-
-  private static final ServerPing.Players FAKE_PLAYERS = new ServerPing.Players(0, 0,
-      ImmutableList.of());
-  private final int protocolVersion;
-  private final String serverVersion;
-  private final String motd;
-  private final int playersOnline;
-  private final int playersMax;
-
-  public LegacyPingResponse(int protocolVersion, String serverVersion, String motd,
-                            int playersOnline, int playersMax) {
-    this.protocolVersion = protocolVersion;
-    this.serverVersion = serverVersion;
-    this.motd = motd;
-    this.playersOnline = playersOnline;
-    this.playersMax = playersMax;
-  }
-
-  public int getProtocolVersion() {
-    return protocolVersion;
-  }
-
-  public String getServerVersion() {
-    return serverVersion;
-  }
-
-  public String getMotd() {
-    return motd;
-  }
-
-  public int getPlayersOnline() {
-    return playersOnline;
-  }
-
-  public int getPlayersMax() {
-    return playersMax;
-  }
-
-  @Override
-  public String toString() {
-    return "LegacyPingResponse{"
-        + "protocolVersion=" + protocolVersion
-        + ", serverVersion='" + serverVersion + '\''
-        + ", motd='" + motd + '\''
-        + ", playersOnline=" + playersOnline
-        + ", playersMax=" + playersMax
-        + '}';
-  }
-
-  /**
-   * Transforms a {@link ServerPing} into a legacy ping response.
-   * @param ping the response to transform
-   * @return the legacy ping response
-   */
-  public static LegacyPingResponse from(ServerPing ping) {
-    return new LegacyPingResponse(ping.getVersion().getProtocol(),
-        ping.getVersion().getName(),
-        ComponentSerializers.LEGACY.serialize(ping.getDescription()),
-        ping.getPlayers().orElse(FAKE_PLAYERS).getOnline(),
-        ping.getPlayers().orElse(FAKE_PLAYERS).getMax());
-  }
-}

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/legacyping/LegacyMinecraftPingVersion.java

@@ -0,0 +1,7 @@
+package com.velocitypowered.proxy.protocol.packet.legacyping;
+
+public enum LegacyMinecraftPingVersion {
+  MINECRAFT_1_3,
+  MINECRAFT_1_4,
+  MINECRAFT_1_6
+}