player-import: refuse an overflowing rotation and a named position over a vehicle

Answers martipops' review of #8735:
- A rotation finite as f64 but past f32::MAX became an infinity (and a NaN pitch) after the
  cast; it is refused now, like any rotation that is not a number.
- A record with a vehicle still to restore (RootVehicle) seats the player back on it at the
  vehicle's own place, which undid a position the request named; the two are refused together.
- A retried import of the same hand-over, or a retry after the commit, answers the same
  `recentered` as the first reply (kept on the guest until it is placed).
- docs/server.md says the velocity is the server's only: the arrival's teleport carries no
  motion, as a vanilla login's does not.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/horizon-client/src/game/dedicated/transfer.rs

@@ -99,6 +99,7 @@ impl Game<'_> {
                 "handover": arrivals::handover_text(id),
                 "placed": true,
                 "announced": host.hub.guests[i].arrival_announce,
+                "recentered": host.hub.guests[i].arrival_recentered,
                 "retried": true,
             });
             self.lan = Some(host);

crates/horizon-client/src/lan/arrivals.rs

@@ -2689,9 +2689,16 @@ mod hub_tests {
     fn held_with_a_record_at(
         at: [f64; 3],
         center: Option<(i32, i32)>,
+    ) -> (serde_json::Value, Hub, Receiver<Out>) {
+        held_with_a_record_of(|g| g.position = at, center)
+    }
+
+    fn held_with_a_record_of(
+        source_player: impl FnOnce(&mut Guest),
+        center: Option<(i32, i32)>,
     ) -> (serde_json::Value, Hub, Receiver<Out>) {
         let (mut source, i) = exported_guest();
-        source.guests[i].position = at;
+        source_player(&mut source.guests[i]);
         let record = super::export_record(&mut source, i, true, Default::default()).unwrap();
         let mut hub = hub();
         assert_eq!(
@@ -2771,6 +2778,42 @@ mod hub_tests {
         );
     }
 
+    /// A retried import of the same hand-over (its reply was lost) finds the view already moved,
+    /// and still says the chunks around the new place are on their way.
+    #[test]
+    fn a_retried_import_still_reports_the_recentring() {
+        let (record, mut hub, _mine) = held_with_a_record_at([100.5, 70.0, -3.25], Some((0, 0)));
+        let request = serde_json::json!({ "record": record, "place": false });
+        for _ in 0..2 {
+            let done = import_player(&mut hub, UUID_N, &request).unwrap();
+            assert!(done.recentered);
+        }
+    }
+
+    /// A record whose vehicle is still to be restored puts the player back on it at the vehicle's
+    /// own place, so a position the request names would be undone: it is refused, and the record's
+    /// own place is still taken.
+    #[test]
+    fn a_named_position_is_refused_for_a_record_with_a_vehicle_to_restore() {
+        let (record, mut hub, _mine) = held_with_a_record_of(
+            |g| {
+                g.position = [10.5, 70.0, -3.25];
+                g.saved_vehicle = Some([1, 2, 3, 4]);
+            },
+            Some((0, 0)),
+        );
+        let named = serde_json::json!({
+            "record": record.clone(), "place": false, "position": [300.5, 64.0, 300.5],
+        });
+        let error = import_player(&mut hub, UUID_N, &named).unwrap_err();
+        assert!(error.contains("vehicle"), "{error}");
+        assert!(!hub.guests[0].imported);
+        let plain = serde_json::json!({ "record": record, "place": false });
+        import_player(&mut hub, UUID_N, &plain).unwrap();
+        assert_eq!(hub.guests[0].position, [10.5, 70.0, -3.25]);
+        assert_eq!(hub.guests[0].saved_vehicle, Some([1, 2, 3, 4]));
+    }
+
     #[test]
     fn placing_at_once_reports_the_recentring_too() {
         let (record, mut hub, mine) = held_with_a_record_at([100.5, 70.0, -3.25], Some((0, 0)));
@@ -2887,6 +2930,10 @@ mod hub_tests {
                 "rotation of one",
                 with(serde_json::json!({ "rotation": [1.0] })),
             ),
+            (
+                "a rotation past what a float holds",
+                with(serde_json::json!({ "rotation": [1e39, 1e39] })),
+            ),
             (
                 "a velocity that is not three numbers",
                 with(serde_json::json!({ "velocity": [0.25, "up", 0.0] })),

crates/horizon-client/src/lan/guest.rs

@@ -360,6 +360,9 @@ pub struct Guest {
     /// The hand-over its record came with (`player-import`): `player-confirm` and
     /// `player-drop` name it, and another hand-over's record is not taken.
     pub arrival_handover: Option<u128>,
+    /// The import of that hand-over moved the view to another chunk (`recentered` in its reply),
+    /// kept so a retried import or a confirm after it answers the same.
+    pub arrival_recentered: bool,
     /// Leaving for another server (`player-release moved`) without the "left the
     /// game" line.
     pub quiet_leave: bool,
@@ -548,6 +551,7 @@ impl Guest {
             imported: false,
             arrival_announce: true,
             arrival_handover: None,
+            arrival_recentered: false,
             quiet_leave: false,
             released: false,
             hidden_commands: Vec::new(),

crates/horizon-client/src/lan/import.rs

@@ -150,8 +150,14 @@ fn read_arrival(request: &Value) -> Result<Arrival, String> {
             // Entity.setXRot: the pitch is taken modulo a turn (`frem`, the sign of the
             // dividend, as Rust's `%`), then cut to a quarter turn each way; setYRot keeps the
             // yaw as it is.
-            let pitch = (pitch as f32 % 360.0).clamp(-90.0, 90.0);
-            Some([yaw as f32, pitch])
+            let (yaw, pitch) = (yaw as f32, pitch as f32);
+            // Finite as an f64 is not enough: past f32::MAX the cast gives an infinity (and
+            // the pitch's modulo a NaN), which Entity.load would refuse.
+            if !yaw.is_finite() || !pitch.is_finite() {
+                return Err("rotation holds only numbers a float can hold".into());
+            }
+            let pitch = (pitch % 360.0).clamp(-90.0, 90.0);
+            Some([yaw, pitch])
         }
         None => None,
     };
@@ -502,6 +508,17 @@ pub fn import_player(hub: &mut Hub, uuid: u128, request: &Value) -> Result<Impor
     let mut level = Level::default();
     level.read_player(&tag);
     check_values(&level)?;
+    // A record with a vehicle still to restore (`RootVehicle`) puts the player back on it, at
+    // the vehicle's own saved place: a position named by the request would be undone as soon
+    // as the vehicle is restored, so the two cannot be taken together.
+    if request.arrival.position.is_some()
+        && (level.root_vehicle.is_some() || tag.get("RootVehicle").is_some())
+    {
+        return Err(
+            "the record has a vehicle to restore (RootVehicle): a position cannot be named with it"
+                .into(),
+        );
+    }
     // What `playerdata::load` gives `apply` too: the whole compound, for the keys Horizon
     // does not model, the mods' attachments and the body's values.
     level.player_extra = Some(tag);
@@ -560,6 +577,10 @@ pub fn import_player(hub: &mut Hub, uuid: u128, request: &Value) -> Result<Impor
     // player as it will be; the player stays held until it is placed or dropped.
     let recentered = hub.send_arrival_state(i);
     let g = &mut hub.guests[i];
+    // A retry of this hand-over's import finds the view already moved: it still answers that
+    // the chunks around the new place are on their way, as the first reply did.
+    let recentered = recentered || (g.arrival_handover == Some(handover) && g.arrival_recentered);
+    g.arrival_recentered = recentered;
     g.imported = true;
     g.arrival_announce = request.announce;
     g.arrival_handover = Some(handover);

docs/server.md

@@ -348,9 +348,13 @@ takes four optional fields, which win over the compound:
 `position` is `[x, y, z]`, `rotation` is `[yaw, pitch]` in degrees, `on_ground` is a boolean and `velocity` is
 `[x, y, z]` in blocks a tick. The pitch is taken as vanilla's `Entity.setXRot` takes it: modulo 360, then cut to ±90;
 the yaw is kept as it is. A velocity component over 10 blocks a tick is taken as 0, as `Entity.load` takes a saved
-`Motion`. They belong to the `position` part: a request that gives one and does not import that part is refused. A
-position that is not finite, or farther than the record's own may be (30,000,512 blocks from the origin, 20,000,000 up
-or down), is refused, and so is a rotation or a velocity that is not finite numbers. The border, the blocks and the chunks are not checked,
+`Motion`. The velocity is the server's: the arrival's teleport carries no motion, as a vanilla login's does not, so
+the client starts still, and since a player's client moves itself, its first move replaces it. They belong to the `position` part: a
+request that gives one and does not import that part is refused. A position that is not finite, or farther than the
+record's own may be (30,000,512 blocks from the origin, 20,000,000 up or down), is refused, and so is a rotation or a
+velocity that is not finite numbers (a rotation past what a float holds too). A record with a vehicle still to restore
+(`RootVehicle`) puts the player back on it at the vehicle's own place, so a request that names a position for such a
+record is refused. The border, the blocks and the chunks are not checked,
 as a vanilla server does not check the place its player file names. The player is teleported to the position (the
 client confirms a new teleport id), so an anticheat never sees the arrival as a move, however far it is from the
 export.