`force-set-room-state` does not force forcefully enough #852

New issue

Closed

opened 2025-06-15 23:01:25 +00:00 by nex · 3 comments

nex commented

2025-06-15 23:01:25 +00:00

Owner

 		let pdu = PduEvent::from_id_val(&event_id, value.clone()).map_err(|e| {
 			debug_error!("Invalid PDU in fetching remote room state PDUs response: {value:#?}");
 			err!(BadServerResponse(debug_error!("Invalid PDU in send_join response: {e:?}")))
 		})?;

It seems that this line can cause us to trip up, for example, if we have local events that the remote we're pulling from does not. I'm not convinced invalid PDUs are being skipped appropriately.

https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/5d44653e3a9bc278f9dbe3084e2e34a9a0799e1f/src/admin/debug/commands.rs#L590-L593 It seems that this line can cause us to trip up, for example, if we have local events that the remote we're pulling from does not. I'm not convinced invalid PDUs are being skipped appropriately.

nex added the

Bug

Matrix/Administration

Status

Needs Investigation

labels 2025-06-15 23:01:25 +00:00

nex commented

2025-06-15 23:02:54 +00:00

Author

Owner

I suppose the argument is, do we even want to skip "invalid" PDUs? is it safe to do that? It probably is in most cases, but are there scenarios where skipping invalid PDUs does more harm than just having a broken room?

nex commented

2025-06-15 23:05:28 +00:00

Author

Owner

e.g: Answer from envs.net: [404 / M_NOT_FOUND] Could not find event $7-Pi9-Rq0e3L_pwv7abXJeSiE1asEwirTGpusb-9HIk (where the event it cannot find is a local event). This error actually looks unrelated to the line linked above 🧐

e.g: `Answer from envs.net: [404 / M_NOT_FOUND] Could not find event $7-Pi9-Rq0e3L_pwv7abXJeSiE1asEwirTGpusb-9HIk` (where the event it cannot find is a local event). This error actually looks unrelated to the line linked above 🧐

nex commented

2025-06-15 23:09:50 +00:00

Author

Owner

 	let first_pdu = self
 		.services
 		.rooms
 		.timeline
 		.latest_pdu_in_room(&room_id)
 		.await
 		.map_err(|_| err!(Database("Failed to find the latest PDU in database")))?;
 	let room_version = self.services.rooms.state.get_room_version(&room_id).await?;
 	let mut state: HashMap<u64, OwnedEventId> = HashMap::new();
 	let remote_state_response = self
 		.services
 		.sending
 		.send_federation_request(&server_name, get_room_state::v1::Request {
 			room_id: room_id.clone(),
 			event_id: first_pdu.event_id.clone(),
 		})
 		.await?;

These lines sus me out now

https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/5d44653e3a9bc278f9dbe3084e2e34a9a0799e1f/src/admin/debug/commands.rs#L543-L562 These lines sus me out now

nex added reference nex/force-time-travel

2025-06-15 23:28:53 +00:00

nex added

and removed

labels 2025-06-16 01:05:41 +00:00

nex referenced this issue from a pull request that will close it,

2025-06-16 15:42:49 +00:00

feat: allow overriding the "most recent event" when forcing a state download #853