From 868f921b946c1153d061b7788ce2784478586f8d Mon Sep 17 00:00:00 2001
From: themode <themode@outlook.fr>
Date: Sat, 14 Nov 2020 05:33:24 +0100
Subject: [PATCH] Better warning message when a packet is not fully read

---
 .../extras/optifine/OptifineSupport.java      |  3 +++
 .../network/netty/channel/ClientChannel.java  | 11 +++++++---
 .../network/netty/codec/PacketFramer.java     |  6 +----
 .../network/player/PlayerConnection.java      | 22 +++++++++++++++++++
 4 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/main/java/net/minestom/server/extras/optifine/OptifineSupport.java b/src/main/java/net/minestom/server/extras/optifine/OptifineSupport.java
index 89c3d2044..108702440 100644
--- a/src/main/java/net/minestom/server/extras/optifine/OptifineSupport.java
+++ b/src/main/java/net/minestom/server/extras/optifine/OptifineSupport.java
@@ -16,6 +16,9 @@ public final class OptifineSupport {
 
     private static volatile boolean enabled;
 
+    /**
+     * Enables optifine support by registering the required biomes.
+     */
     public static void enable() {
         Check.stateCondition(enabled, "Optifine support is already enabled!");
         OptifineSupport.enabled = true;
diff --git a/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java b/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java
index 4b8a51f12..ac9d7573a 100644
--- a/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java
+++ b/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java
@@ -10,10 +10,14 @@ import net.minestom.server.network.PacketProcessor;
 import net.minestom.server.network.netty.packet.InboundPacket;
 import net.minestom.server.network.player.PlayerConnection;
 import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Slf4j
 public class ClientChannel extends SimpleChannelInboundHandler<InboundPacket> {
 
+    public final static Logger LOGGER = LoggerFactory.getLogger(ClientChannel.class);
+
     private final ConnectionManager connectionManager = MinecraftServer.getConnectionManager();
     private final PacketProcessor packetProcessor;
 
@@ -34,9 +38,10 @@ public class ClientChannel extends SimpleChannelInboundHandler<InboundPacket> {
             final int availableBytes = packet.body.readableBytes();
 
             if (availableBytes > 0) {
-                // TODO log4j2
-                System.err.println("WARNING: Packet 0x" + Integer.toHexString(packet.packetId)
-                        + " not fully read (" + availableBytes + " bytes left)");
+                final PlayerConnection playerConnection = packetProcessor.getPlayerConnection(ctx);
+
+                LOGGER.warn("WARNING: Packet 0x" + Integer.toHexString(packet.packetId)
+                        + " not fully read (" + availableBytes + " bytes left), " + playerConnection);
 
                 packet.body.skipBytes(availableBytes);
             }
diff --git a/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java b/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java
index f78c65942..62af408d0 100644
--- a/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java
+++ b/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java
@@ -5,7 +5,6 @@ import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageCodec;
 import io.netty.handler.codec.CorruptedFrameException;
 import net.minestom.server.MinecraftServer;
-import net.minestom.server.entity.Player;
 import net.minestom.server.network.PacketProcessor;
 import net.minestom.server.network.player.PlayerConnection;
 import net.minestom.server.utils.Utils;
@@ -60,10 +59,7 @@ public class PacketFramer extends ByteToMessageCodec<ByteBuf> {
                 if (packetSize >= MinecraftServer.getMaxPacketSize()) {
                     final PlayerConnection playerConnection = packetProcessor.getPlayerConnection(ctx);
                     if (playerConnection != null) {
-                        final Player player = playerConnection.getPlayer();
-                        final String identifier = player != null ?
-                                player.getUsername() :
-                                playerConnection.getRemoteAddress().toString();
+                        final String identifier = playerConnection.getIdentifier();
                         LOGGER.warn("An user (" + identifier + ") sent a packet over the maximum size (" + packetSize + ")");
                     } else {
                         LOGGER.warn("An unregistered user sent a packet over the maximum size (" + packetSize + ")");
diff --git a/src/main/java/net/minestom/server/network/player/PlayerConnection.java b/src/main/java/net/minestom/server/network/player/PlayerConnection.java
index 2b7dea130..b3a190173 100644
--- a/src/main/java/net/minestom/server/network/player/PlayerConnection.java
+++ b/src/main/java/net/minestom/server/network/player/PlayerConnection.java
@@ -73,6 +73,20 @@ public abstract class PlayerConnection {
         }
     }
 
+    /**
+     * Returns a printable identifier for this connection, will be the player username
+     * or the connection remote address.
+     *
+     * @return this connection identifier
+     */
+    @NotNull
+    public String getIdentifier() {
+        final Player player = getPlayer();
+        return player != null ?
+                player.getUsername() :
+                getRemoteAddress().toString();
+    }
+
     /**
      * Serializes the packet and send it to the client.
      * <p>
@@ -156,4 +170,12 @@ public abstract class PlayerConnection {
     public int getLastPacketCounter() {
         return lastPacketCounter.get();
     }
+
+    @Override
+    public String toString() {
+        return "PlayerConnection{" +
+                "connectionState=" + connectionState +
+                ", identifier=" + getIdentifier() +
+                '}';
+    }
 }