From 042d25816150f3c1ea9d7e2c656325cf8e1d2b4e Mon Sep 17 00:00:00 2001 From: InfRandomness Date: Sun, 27 Feb 2022 23:08:47 +0000 Subject: [PATCH] Modify message catch-all arms --- CHANGELOG.md | 1 + server/src/sys/msg/character_screen.rs | 8 +++++++- server/src/sys/msg/general.rs | 8 +++++++- server/src/sys/msg/in_game.rs | 8 +++++++- server/src/sys/msg/terrain.rs | 18 ++++++++++++++++-- 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a010a5b978..01bd5e395b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Improved site placement +- [Server] Kick clients who send messages on the wrong stream ### Removed diff --git a/server/src/sys/msg/character_screen.rs b/server/src/sys/msg/character_screen.rs index 77f6ff35c4..296abb3da0 100644 --- a/server/src/sys/msg/character_screen.rs +++ b/server/src/sys/msg/character_screen.rs @@ -175,7 +175,13 @@ impl Sys { ); } }, - _ => tracing::error!("not a client_character_screen msg"), + _ => { + debug!("Kicking possibly misbehaving client due to invalid character request"); + server_emitter.emit(ServerEvent::ClientDisconnect( + entity, + common::comp::DisconnectReason::NetworkError, + )); + }, } Ok(()) } diff --git a/server/src/sys/msg/general.rs b/server/src/sys/msg/general.rs index 9c56cc20ec..0527da7dcf 100644 --- a/server/src/sys/msg/general.rs +++ b/server/src/sys/msg/general.rs @@ -57,7 +57,13 @@ impl Sys { common::comp::DisconnectReason::ClientRequested, )); }, - _ => tracing::error!("not a client_general msg"), + _ => { + debug!("Kicking possible misbehaving client due to invalid message request"); + server_emitter.emit(ServerEvent::ClientDisconnect( + entity, + common::comp::DisconnectReason::NetworkError, + )); + }, } Ok(()) } diff --git a/server/src/sys/msg/in_game.rs b/server/src/sys/msg/in_game.rs index 5ee80a324a..0b1972eb1b 100644 --- a/server/src/sys/msg/in_game.rs +++ b/server/src/sys/msg/in_game.rs @@ -297,7 +297,13 @@ impl Sys { | ClientGeneral::TerrainChunkRequest { .. } | ClientGeneral::ChatMsg(_) | ClientGeneral::Command(..) - | ClientGeneral::Terminate => tracing::error!("not a client_in_game msg"), + | ClientGeneral::Terminate => { + debug!("Kicking possibly misbehaving client due to invalid client in game request"); + server_emitter.emit(ServerEvent::ClientDisconnect( + entity, + common::comp::DisconnectReason::NetworkError, + )); + }, } Ok(()) } diff --git a/server/src/sys/msg/terrain.rs b/server/src/sys/msg/terrain.rs index 13d495cddf..e6b39c1808 100644 --- a/server/src/sys/msg/terrain.rs +++ b/server/src/sys/msg/terrain.rs @@ -1,6 +1,7 @@ use crate::{client::Client, metrics::NetworkRequestMetrics, presence::Presence, ChunkRequest}; use common::{ comp::Pos, + event::{EventBus, ServerEvent}, spiral::Spiral2d, terrain::{TerrainChunkSize, TerrainGrid}, vol::RectVolSize, @@ -8,7 +9,7 @@ use common::{ use common_ecs::{Job, Origin, ParMode, Phase, System}; use common_net::msg::{ClientGeneral, SerializedTerrainChunk, ServerGeneral}; use rayon::iter::ParallelIterator; -use specs::{Entities, Join, ParJoin, ReadExpect, ReadStorage, Write}; +use specs::{Entities, Join, ParJoin, Read, ReadExpect, ReadStorage, Write}; use tracing::{debug, trace}; /// This system will handle new messages from clients @@ -17,6 +18,7 @@ pub struct Sys; impl<'a> System<'a> for Sys { type SystemData = ( Entities<'a>, + Read<'a, EventBus>, ReadExpect<'a, TerrainGrid>, ReadExpect<'a, NetworkRequestMetrics>, Write<'a, Vec>, @@ -33,6 +35,7 @@ impl<'a> System<'a> for Sys { job: &mut Job, ( entities, + server_event_bus, terrain, network_metrics, mut chunk_requests, @@ -47,6 +50,8 @@ impl<'a> System<'a> for Sys { .map(|(entity, client, maybe_presence)| { let mut chunk_requests = Vec::new(); let _ = super::try_recv_all(client, 5, |client, msg| { + // TODO: Refactor things (https://gitlab.com/veloren/veloren/-/merge_requests/3245#note_856538056) + let mut server_emitter = server_event_bus.emitter(); let presence = match maybe_presence { Some(g) => g, None => { @@ -96,7 +101,16 @@ impl<'a> System<'a> for Sys { network_metrics.chunks_request_dropped.inc(); } }, - _ => tracing::error!("not a client_terrain msg"), + _ => { + debug!( + "Kicking possibly misbehaving client due to invalud terrain \ + request" + ); + server_emitter.emit(ServerEvent::ClientDisconnect( + entity, + common::comp::DisconnectReason::NetworkError, + )); + }, } Ok(()) });