From 59829799d8c267b258d41cab3daa2ad4959de2ce Mon Sep 17 00:00:00 2001 From: Mathias Hartveit Date: Thu, 15 Oct 2026 13:47:32 +0200 Subject: [PATCH] fix(gateway): send the vanilla Transfer abort reason --- docs/benilla-compatibility.md | 5 ++++ gateway/src/codec/mod.rs | 4 +-- gateway/src/codec/movement.rs | 25 ++++++++++------ gateway/src/world/benilla_tests.rs | 46 ++++++++++++++++++++++++++++++ gateway/src/world/entry.rs | 34 ++++------------------ gateway/src/world/session_tests.rs | 45 +++++++++-------------------- 6 files changed, 88 insertions(+), 71 deletions(-) diff --git a/docs/benilla-compatibility.md b/docs/benilla-compatibility.md index 7b240156f..f9f888524 100644 --- a/docs/benilla-compatibility.md +++ b/docs/benilla-compatibility.md @@ -202,6 +202,11 @@ runs them in CI. They cover: ## Additional protocol corrections +`SMSG_TRANSFER_ABORTED` carries one reason byte in build 5875. The Gateway sends it +even when the failed Transfer has no usable destination map. A Benilla scenario checks +the failure path for known, unknown and missing destinations, including full body +consumption. The retail client's loading-screen recovery still needs Verification. + The Gateway owns these build-5875 layouts where the protocol library differs: | Request | Benilla layout | diff --git a/gateway/src/codec/mod.rs b/gateway/src/codec/mod.rs index 35e3a90b8..faf777077 100644 --- a/gateway/src/codec/mod.rs +++ b/gateway/src/codec/mod.rs @@ -112,8 +112,8 @@ use wow_world_messages::vanilla::{ SMSG_QUESTUPDATE_FAILEDTIMER, SMSG_RESURRECT_REQUEST, SMSG_SET_PROFICIENCY, SMSG_SET_REST_START, SMSG_SHOW_BANK, SMSG_SPELLNONMELEEDAMAGELOG, SMSG_SPELL_COOLDOWN, SMSG_SPELL_DELAYED, SMSG_SPELL_FAILURE, SMSG_SPELL_GO, SMSG_SPELL_START, - SMSG_TRAINER_BUY_FAILED, SMSG_TRAINER_BUY_SUCCEEDED, SMSG_TRAINER_LIST, SMSG_TRANSFER_ABORTED, - SMSG_TRANSFER_PENDING, SMSG_TUTORIAL_FLAGS, SMSG_UPDATE_AURA_DURATION, SMSG_UPDATE_OBJECT, + SMSG_TRAINER_BUY_FAILED, SMSG_TRAINER_BUY_SUCCEEDED, SMSG_TRAINER_LIST, SMSG_TRANSFER_PENDING, + SMSG_TUTORIAL_FLAGS, SMSG_UPDATE_AURA_DURATION, SMSG_UPDATE_OBJECT, }; use wow_world_messages::Guid; diff --git a/gateway/src/codec/movement.rs b/gateway/src/codec/movement.rs index 5b738315c..d40e6e1ee 100644 --- a/gateway/src/codec/movement.rs +++ b/gateway/src/codec/movement.rs @@ -238,15 +238,9 @@ pub fn build_cross_map_teleport( ]) } -pub fn build_transfer_aborted( - map_id: u32, - reason: TransferAbortReason, -) -> Result { - Ok(SMSG_TRANSFER_ABORTED { - map: Map::try_from(map_id).map_err(|_| anyhow!("invalid map {map_id}"))?, - reason, - argument: 0, - }) +/// Build-5875 aborts carry only a reason byte; the typed writer adds later fields. +pub fn build_transfer_aborted(reason: TransferAbortReason) -> (u16, Vec) { + (0x0040, vec![reason.as_int()]) } pub fn build_force_run_speed(guid: u64, speed: f32) -> SMSG_FORCE_RUN_SPEED_CHANGE { @@ -440,6 +434,19 @@ pub fn build_monster_move_facing( } } +#[cfg(test)] +mod transfer_abort_tests { + use super::*; + + #[test] + fn vanilla_transfer_abort_contains_only_the_reason() { + assert_eq!( + build_transfer_aborted(TransferAbortReason::NotFound), + (0x0040, vec![0x02]) + ); + } +} + #[cfg(test)] mod ground_path_tests { use super::*; diff --git a/gateway/src/world/benilla_tests.rs b/gateway/src/world/benilla_tests.rs index c3bba8e98..c52d442d3 100644 --- a/gateway/src/world/benilla_tests.rs +++ b/gateway/src/world/benilla_tests.rs @@ -279,6 +279,52 @@ fn benilla_login_clock_ping_logout_and_reconnect() { } } +#[test] +fn benilla_failed_transfer_reports_reason_without_a_destination_map() { + for destination_map in [Some(36), Some(u32::MAX), None] { + let mut store = tester_store(7); + store.session.entity_in_world = false; + store.session.login_entity = Some(warrior_entity()); + store.characters = vec![super::party_tests::character(1, "Tester")]; + store.transfer.xdb = destination_map.map(|map_id| { + super::FakeShardDb::with_character( + 1, + super::FakeChar { + map_id, + instance_id: 7, + payload: "gear+spells".into(), + }, + ) + }); + store.topology.settle_error = Some("destination Shard unreachable".into()); + store.topology.settle_ok_calls = 1; + let store = Arc::new(store); + let (socket, gateway_socket) = world_session_socket_pair(); + let gateway_store = store.clone(); + let gateway = std::thread::spawn(move || run_world_session(gateway_socket, gateway_store)); + let mut client = Client::connect(socket, super::K); + client.enter_world(); + client.send(opcode::MSG_MOVE_WORLDPORT_ACK, &[]); + + // Client::recv requires Benilla to consume every byte in each frame. + assert!(matches!( + client.until("Transfer abort", |packet| { + matches!(packet, ServerPacket::TransferAborted { .. }) + }), + ServerPacket::TransferAborted { reason: 0x02 } + )); + let error = gateway + .join() + .unwrap() + .expect_err("a failed Transfer must end the World Session"); + assert!(format!("{error:#}").contains("destination Shard unreachable")); + assert!(store + .session + .logout_called + .load(std::sync::atomic::Ordering::SeqCst)); + } +} + pub(crate) struct MovementWorld { view: Arc, relay: fn(&crate::stdb::world_view::WorldView, &crate::stdb::bindings::EntityMotion), diff --git a/gateway/src/world/entry.rs b/gateway/src/world/entry.rs index 340714dec..2492302d0 100644 --- a/gateway/src/world/entry.rs +++ b/gateway/src/world/entry.rs @@ -68,45 +68,21 @@ pub(super) fn world_port_ack( ported = enter_world(tx, &*home, conn, character, codec::WorldEntry::WorldPort); } if let Err(error) = ported { - abort_pending_transfer(tx, store, character_guid, &error); + abort_pending_transfer(tx, character_guid, &error); return Err(error); } Ok(()) } -fn abort_pending_transfer( - tx: &SessionTx, - store: &St, - character_guid: u64, - cause: &anyhow::Error, -) { - let dest_map = store - .character_destination(character_guid) - .map(|p| p.dest_map_id); +fn abort_pending_transfer(tx: &SessionTx, character_guid: u64, cause: &anyhow::Error) { log::error!( "world: world-port for guid {character_guid} cannot complete ({cause:#}); aborting the \ - client's transfer to map {dest_map:?}. The character is unharmed: the escrow is idempotent \ + client's Transfer. The Character is unharmed: the Escrow is idempotent \ and the next login re-drives it." ); - let Some(map_id) = dest_map else { - log::warn!( - "world: no durable destination for guid {character_guid}; the client gets no \ - SMSG_TRANSFER_ABORTED and will need to reconnect" - ); - return; - }; use wow_world_messages::vanilla::TransferAbortReason; - match codec::build_transfer_aborted(map_id, TransferAbortReason::NotFound) { - Ok(msg) => { - let _ = send( - tx, - Outbound::One(ServerOpcodeMessage::SMSG_TRANSFER_ABORTED(msg)), - ); - } - Err(e) => { - log::warn!("world: could not build SMSG_TRANSFER_ABORTED for map {map_id}: {e:#}") - } - } + let (opcode, body) = codec::build_transfer_aborted(TransferAbortReason::NotFound); + let _ = send(tx, Outbound::Raw { opcode, body }); } // Queue self CREATE before registration so initial AOI Relays follow it. diff --git a/gateway/src/world/session_tests.rs b/gateway/src/world/session_tests.rs index 489a4ccd2..78ca78eff 100644 --- a/gateway/src/world/session_tests.rs +++ b/gateway/src/world/session_tests.rs @@ -1408,23 +1408,16 @@ fn a_world_port_whose_transfer_cannot_be_driven_aborts_the_clients_loading_scree .write_encrypted_client(&mut client, &mut c_enc) .unwrap(); - let aborted = ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).expect( - "the client must receive SMSG_TRANSFER_ABORTED — silence here is the loading-screen hang this fixes", + assert_eq!( + read_raw_frame(&mut client, &mut c_dec), + (0x0040, vec![0x02]), + "SMSG_TRANSFER_ABORTED must contain only the NotFound reason" ); - match aborted { - ServerOpcodeMessage::SMSG_TRANSFER_ABORTED(m) => { - assert_eq!( - m.map, - Map::Deadmines, - "the abort must name the map the client is loading" - ); - } - other => panic!("expected SMSG_TRANSFER_ABORTED, got {other}"), - } drop(client); - let err = server.join().unwrap().expect_err( - "a half-driven transfer must still end the session rather than enter the world", - ); + let err = server + .join() + .unwrap() + .expect_err("a failed Transfer must end the World Session"); assert!( format!("{err:#}").contains("instances shard unreachable"), "{err:#}" @@ -1432,7 +1425,7 @@ fn a_world_port_whose_transfer_cannot_be_driven_aborts_the_clients_loading_scree assert!( view.viewer_of_owner(crate::stdb::world_view::OwnerGuid(1)) .is_none(), - "a failed transfer terminates the session and must not restore its source viewer" + "a failed Transfer ends the World Session and must not restore its source viewer" ); assert!( store @@ -1483,27 +1476,17 @@ fn a_world_port_whose_world_entry_fails_also_aborts_the_clients_loading_screen() .write_encrypted_client(&mut client, &mut c_enc) .unwrap(); - let mut aborted = None; - while let Ok(msg) = ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec) { - if let ServerOpcodeMessage::SMSG_TRANSFER_ABORTED(m) = msg { - aborted = Some(m); - break; - } - } - let m = aborted.expect( - "a world-port whose ENTRY fails must still abort the client's loading screen — silence \ - here is the infinite loading bar this abort exists to kill", - ); + // World entry fails before it queues any arrival messages. assert_eq!( - m.map, - Map::Deadmines, - "the abort must name the map the client is loading" + read_raw_frame(&mut client, &mut c_dec), + (0x0040, vec![0x02]), + "SMSG_TRANSFER_ABORTED must contain only the NotFound reason" ); drop(client); let err = server .join() .unwrap() - .expect_err("a half-entered world-port must still end the session"); + .expect_err("a failed world-port must end the World Session"); assert!(format!("{err:#}").contains("stranded"), "{err:#}"); }