fix: ignore the auth packet sent by client if there are NaN values in…

… it, and now setting the motion/location of an entity to a vector which contains NaN will result in an exception

CHANGELOG.md

@@ -47,11 +47,14 @@ Unless otherwise specified, any version comparison below is the comparison of se
 - `Pos`, `Motion` and `Rotation` in entity nbt are now saved as list tag instead of compound tag to match vanilla.
   This also fixed the bug that entities being spawned in incorrect position when placing structure using `/structure`
   command. Please note that this change is not backward compatible and will break the old world and player data.
-- Fixed several NaNs caused by `Vector3fc#normalize` methods in the physics engine, and now setting the motion of an
-  entity to a vector which contains NaN will result in an error.
+- Fixed several NaNs caused by `Vector3fc#normalize` methods in the physics engine, and now setting the motion/location
+  of an
+  entity to a vector which contains NaN will result in an exception.
 - EntityItem now won't have drowning damage when it is in water, this bug causes entity item died after a period of time
   in water.
 - `ServerboundLoadingScreenPacket` won't spam warnings in the console when switching dimension now.
+- Fixed the bug that sometimes there may be `NaN` values in `PlayerAuthInputPacket`, this bug is also confirmed in
+  df-mc ([issue#425](https://github.com/df-mc/dragonfly/issues/425)).
 
 ## 0.1.1 (API 0.2.0)
 

api/src/main/java/org/allaymc/api/math/MathUtils.java

@@ -1,5 +1,7 @@
 package org.allaymc.api.math;
 
+import org.allaymc.api.math.location.Location3dc;
+import org.allaymc.api.math.location.Location3fc;
 import org.joml.*;
 
 import java.lang.Math;
@@ -237,6 +239,30 @@ public static boolean hasNaN(Vector3dc v) {
         return Double.isNaN(v.x()) || Double.isNaN(v.y()) || Double.isNaN(v.z());
     }
 
+    /**
+     * Check if the location contains NaN.
+     *
+     * @param l the location to check.
+     *
+     * @return {@code true} if the vector contains NaN, {@code false} otherwise.
+     */
+    public static boolean hasNaN(Location3fc l) {
+        return Float.isNaN(l.x()) || Float.isNaN(l.y()) || Float.isNaN(l.z()) ||
+               Double.isNaN(l.pitch()) || Double.isNaN(l.yaw()) || Double.isNaN(l.headYaw());
+    }
+
+    /**
+     * Check if the location contains NaN.
+     *
+     * @param l the location to check.
+     *
+     * @return {@code true} if the vector contains NaN, {@code false} otherwise.
+     */
+    public static boolean hasNaN(Location3dc l) {
+        return Double.isNaN(l.x()) || Double.isNaN(l.y()) || Double.isNaN(l.z()) ||
+               Double.isNaN(l.pitch()) || Double.isNaN(l.yaw()) || Double.isNaN(l.headYaw());
+    }
+
     /**
      * Normalize the vector if it is not zero.
      * <p>

server/src/main/java/org/allaymc/server/entity/component/EntityBaseComponentImpl.java

@@ -290,6 +290,9 @@ public void setLocationBeforeSpawn(Location3fc location) {
     }
 
     protected void setLocation(Location3fc location, boolean calculateFallDistance) {
+        if (MathUtils.hasNaN(location)) {
+            throw new IllegalArgumentException("Trying to set the location of entity " + runtimeId + " to a new location which contains NaN: " + location);
+        }
         if (calculateFallDistance && !this.onGround) {
             if (this.fallDistance < 0) {
                 // Entity start falling
@@ -494,11 +497,7 @@ public Map<Long, EntityPlayer> getViewers() {
     @Override
     public void setMotion(Vector3fc motion) {
         if (MathUtils.hasNaN(motion)) {
-            // Sometimes there may be bugs in the physics engine, which will cause the motion to be NaN.
-            // This check help us find the bug quickly as we usually can't realize that a strange bug
-            // is caused by NaN motion.
-            log.error("Entity {} is set by a motion which contains NaN: {}", runtimeId, motion);
-            return;
+            throw new IllegalArgumentException("Trying to set the motion of entity " + runtimeId + " to a new motion which contains NaN: " + motion);
         }
         this.lastMotion = this.motion;
         this.motion = new Vector3f(motion);

server/src/main/java/org/allaymc/server/entity/component/player/EntityPlayerBaseComponentImpl.java

@@ -350,7 +350,7 @@ protected void teleportOverDimension(Location3fc target, EntityTeleportEvent.Rea
         location.dimension().removePlayer(thisPlayer, () -> {
             targetDim.getChunkService().getOrLoadChunkSync((int) target.x() >> 4, (int) target.z() >> 4);
             setLocationBeforeSpawn(target);
-            sendLocationToSelf(EntityTeleportEvent.Reason.UNKNOWN);
+            sendLocationToSelf(reason);
             if (currentDim.getDimensionInfo().dimensionId() != targetDim.getDimensionInfo().dimensionId()) {
                 var packet = new ChangeDimensionPacket();
                 packet.setDimension(targetDim.getDimensionInfo().dimensionId());

server/src/main/java/org/allaymc/server/network/processor/impl/ingame/PlayerAuthInputPacketProcessor.java

@@ -287,6 +287,11 @@ public PacketSignal handleAsync(EntityPlayer player, PlayerAuthInputPacket packe
             return PacketSignal.HANDLED;
         }
 
+        if (!validateClientLocation(player, packet.getPosition(), packet.getRotation())) {
+            // Ignore this auth packet if the pos provided by client is not valid
+            return PacketSignal.HANDLED;
+        }
+
         // The pos which client sends to the server is higher than the actual coordinates (one base offset)
         handleMovement(player, packet.getPosition().sub(0, player.getBaseOffset(), 0), packet.getRotation());
         handleBlockAction(player, packet.getPlayerActions(), receiveTime);
@@ -297,6 +302,18 @@ public PacketSignal handleAsync(EntityPlayer player, PlayerAuthInputPacket packe
         return PacketSignal.UNHANDLED;
     }
 
+    protected boolean validateClientLocation(EntityPlayer player, Vector3f pos, Vector3f rot) {
+        var valid = !Float.isNaN(pos.getX()) && !Float.isNaN(pos.getY()) && !Float.isNaN(pos.getZ()) &&
+                    !Float.isNaN(rot.getX()) && !Float.isNaN(rot.getY()) && !Float.isNaN(rot.getZ());
+        if (!valid) {
+            // Sometimes, the PlayerAuthInput packet is in fact sent with NaN/INF after being teleported (to another
+            // world). For this reason, we don't actually return an error if this happens, because this will result
+            // in the player being kicked. Just log it and replace the NaN value with the one we have tracked server-side.
+            log.debug("Found NaN in PlayerAuthInputPacket sent by player {}", player.getOriginName());
+        }
+        return valid;
+    }
+
     protected void handleSingleItemStackRequest(EntityPlayer player, ItemStackRequest request, long receiveTime) {
         // We had no idea why the client still use PlayerAuthInputPacket to hold ItemStackRequest
         // Instead of using ItemStackRequestPacket