diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f97b930..508f3f725 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/api/src/main/java/org/allaymc/api/math/MathUtils.java b/api/src/main/java/org/allaymc/api/math/MathUtils.java index 54624522b..0137e225d 100644 --- a/api/src/main/java/org/allaymc/api/math/MathUtils.java +++ b/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. *

diff --git a/server/src/main/java/org/allaymc/server/entity/component/EntityBaseComponentImpl.java b/server/src/main/java/org/allaymc/server/entity/component/EntityBaseComponentImpl.java index 235edfa45..eb2be8579 100644 --- a/server/src/main/java/org/allaymc/server/entity/component/EntityBaseComponentImpl.java +++ b/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 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); diff --git a/server/src/main/java/org/allaymc/server/entity/component/player/EntityPlayerBaseComponentImpl.java b/server/src/main/java/org/allaymc/server/entity/component/player/EntityPlayerBaseComponentImpl.java index dd7fa688f..88aa88f26 100644 --- a/server/src/main/java/org/allaymc/server/entity/component/player/EntityPlayerBaseComponentImpl.java +++ b/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()); diff --git a/server/src/main/java/org/allaymc/server/network/processor/impl/ingame/PlayerAuthInputPacketProcessor.java b/server/src/main/java/org/allaymc/server/network/processor/impl/ingame/PlayerAuthInputPacketProcessor.java index 689939300..bc4ae296a 100644 --- a/server/src/main/java/org/allaymc/server/network/processor/impl/ingame/PlayerAuthInputPacketProcessor.java +++ b/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