From d4ccc60986b4ea295c596e960519945551ff152e Mon Sep 17 00:00:00 2001
From: Minecrell <minecrell@minecrell.net>
Date: Fri, 13 Jul 2018 09:43:56 +0200
Subject: [PATCH] Avoid Netty buffer leak in LegacyPingHandler. Fixes #1200
 (#1201)

The extra buffer used to decode the strings sent by the client
in the legacy ping protocol was never released. However, creating
an extra copy of the buffer just to decode it to a string isn't
actually necessary: We can just call toString() directly on the
original buffer.

Additionally, free the buffer in handlerRemoved() to handle cases
where the client never sends enough bytes to form a valid legacy
ping request.
---
 ...ServerListPingEvent-for-legacy-pings.patch |  2 +-
 ...e-legacy-ping-handler-more-reliable.patch} | 48 ++++++++++++-------
 2 files changed, 33 insertions(+), 17 deletions(-)
 rename Spigot-Server-Patches/{Make-the-legacy-ping-handler-more-reliable.patch => Make-legacy-ping-handler-more-reliable.patch} (81%)

diff --git a/Spigot-Server-Patches/Call-PaperServerListPingEvent-for-legacy-pings.patch b/Spigot-Server-Patches/Call-PaperServerListPingEvent-for-legacy-pings.patch
index 60c89673d..f9483e001 100644
--- a/Spigot-Server-Patches/Call-PaperServerListPingEvent-for-legacy-pings.patch
+++ b/Spigot-Server-Patches/Call-PaperServerListPingEvent-for-legacy-pings.patch
@@ -84,7 +84,7 @@ index 000000000..74c012fd4
 +
 +}
 diff --git a/src/main/java/net/minecraft/server/LegacyPingHandler.java b/src/main/java/net/minecraft/server/LegacyPingHandler.java
-index f084a653a..39d19e91b 100644
+index a89a86e6d..2762bcc2e 100644
 --- a/src/main/java/net/minecraft/server/LegacyPingHandler.java
 +++ b/src/main/java/net/minecraft/server/LegacyPingHandler.java
 @@ -0,0 +0,0 @@ import java.net.InetSocketAddress;
diff --git a/Spigot-Server-Patches/Make-the-legacy-ping-handler-more-reliable.patch b/Spigot-Server-Patches/Make-legacy-ping-handler-more-reliable.patch
similarity index 81%
rename from Spigot-Server-Patches/Make-the-legacy-ping-handler-more-reliable.patch
rename to Spigot-Server-Patches/Make-legacy-ping-handler-more-reliable.patch
index cc6f768f8..fa0c89815 100644
--- a/Spigot-Server-Patches/Make-the-legacy-ping-handler-more-reliable.patch
+++ b/Spigot-Server-Patches/Make-legacy-ping-handler-more-reliable.patch
@@ -1,7 +1,7 @@
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Minecrell <minecrell@minecrell.net>
 Date: Wed, 11 Oct 2017 18:22:50 +0200
-Subject: [PATCH] Make the legacy ping handler more reliable
+Subject: [PATCH] Make legacy ping handler more reliable
 
 The Minecraft server often fails to respond to old ("legacy") pings
 from old Minecraft versions using the protocol used before the switch
@@ -28,7 +28,7 @@ respond to the request.
 [2]: https://netty.io/wiki/user-guide-for-4.x.html#wiki-h4-13
 
 diff --git a/src/main/java/net/minecraft/server/LegacyPingHandler.java b/src/main/java/net/minecraft/server/LegacyPingHandler.java
-index 4c1a0181a..f084a653a 100644
+index 4c1a0181a..a89a86e6d 100644
 --- a/src/main/java/net/minecraft/server/LegacyPingHandler.java
 +++ b/src/main/java/net/minecraft/server/LegacyPingHandler.java
 @@ -0,0 +0,0 @@ public class LegacyPingHandler extends ChannelInboundHandlerAdapter {
@@ -82,6 +82,17 @@ index 4c1a0181a..f084a653a 100644
      }
  
 +    // Paper start
++    private static String readLegacyString(ByteBuf buf) {
++        int size = buf.readShort() * Character.BYTES;
++        if (!buf.isReadable(size)) {
++            return null;
++        }
++
++        String result = buf.toString(buf.readerIndex(), size, StandardCharsets.UTF_16BE);
++        buf.skipBytes(size); // toString doesn't increase readerIndex automatically
++        return result;
++    }
++
 +    private void readLegacy1_6(ChannelHandlerContext ctx, ByteBuf part) {
 +        ByteBuf buf = this.buf;
 +
@@ -94,34 +105,31 @@ index 4c1a0181a..f084a653a 100644
 +
 +        buf.writeBytes(part);
 +
-+        // Short + Short + Byte + Short + Int
-+        if (!buf.isReadable(2 + 2 + 1 + 2 + 4)) {
++        if (!buf.isReadable(Short.BYTES + Short.BYTES + Byte.BYTES + Short.BYTES + Integer.BYTES)) {
 +            return;
 +        }
 +
-+        short length = buf.readShort();
-+        if (!buf.isReadable(length * 2)) {
++        String s = readLegacyString(buf);
++        if (s == null) {
 +            return;
 +        }
 +
-+        if (!buf.readBytes(length * 2).toString(StandardCharsets.UTF_16BE).equals("MC|PingHost")) {
++        if (!s.equals("MC|PingHost")) {
 +            removeHandler(ctx);
 +            return;
 +        }
 +
-+        if (!buf.isReadable(2)) {
-+            return;
-+        }
-+
-+        length = buf.readShort();
-+        if (!buf.isReadable(length)) {
++        if (!buf.isReadable(Short.BYTES) || !buf.isReadable(buf.readShort())) {
 +            return;
 +        }
 +
 +        MinecraftServer server = this.b.d();
 +        int protocolVersion = buf.readByte();
-+        length = buf.readShort();
-+        String host = buf.readBytes(length * 2).toString(StandardCharsets.UTF_16BE);
++        String host = readLegacyString(buf);
++        if (host == null) {
++            removeHandler(ctx);
++            return;
++        }
 +        int port = buf.readInt();
 +
 +        if (buf.isReadable()) {
@@ -144,9 +152,17 @@ index 4c1a0181a..f084a653a 100644
 +        this.buf = null;
 +
 +        buf.resetReaderIndex();
-+        ctx.pipeline().remove("legacy_query");
++        ctx.pipeline().remove(this);
 +        ctx.fireChannelRead(buf);
 +    }
++
++    @Override
++    public void handlerRemoved(ChannelHandlerContext ctx) {
++        if (this.buf != null) {
++            this.buf.release();
++            this.buf = null;
++        }
++    }
 +    // Paper end
 +
      private void a(ChannelHandlerContext channelhandlercontext, ByteBuf bytebuf) {