diff --git a/common/query_server/examples/demo.rs b/common/query_server/examples/demo.rs index a649ed683d..7f36e8be30 100644 --- a/common/query_server/examples/demo.rs +++ b/common/query_server/examples/demo.rs @@ -14,7 +14,7 @@ use veloren_query_server::{ const DEFAULT_SERVER_INFO: ServerInfo = ServerInfo { git_hash: 0, - git_version: 0, + git_timestamp: 0, players_count: 100, player_cap: 300, battlemode: ServerBattleMode::GlobalPvE, @@ -41,7 +41,7 @@ async fn main() { let start = Instant::now(); for _i in 0..10000 { - if let Err(error) = client.ping().await { + if let Err(error) = client.server_info().await { error!(?error, "Server info request error"); } } diff --git a/common/query_server/src/client.rs b/common/query_server/src/client.rs index 4c83a1cbbe..13587c93db 100644 --- a/common/query_server/src/client.rs +++ b/common/query_server/src/client.rs @@ -6,11 +6,11 @@ use std::{ use protocol::Parcel; use tokio::{net::UdpSocket, time::timeout}; -use tracing::{trace, warn}; +use tracing::trace; use crate::proto::{ QueryServerRequest, QueryServerResponse, RawQueryServerRequest, RawQueryServerResponse, - ServerInfo, MAX_REQUEST_SIZE, MAX_RESPONSE_SIZE, VELOREN_HEADER, VERSION, + ServerInfo, MAX_RESPONSE_SIZE, }; // This must be at least 2 for the client to get a value for the `p` field. @@ -23,7 +23,6 @@ pub enum QueryClientError { InvalidResponse, Timeout, ChallengeFailed, - RequestTooLarge, } struct ClientInitData { @@ -46,6 +45,7 @@ impl QueryClient { self.send_query(QueryServerRequest::ServerInfo) .await .and_then(|(response, duration)| { + #[allow(irrefutable_let_patterns)] if let QueryServerResponse::ServerInfo(info) = response { Ok((info, duration)) } else { @@ -54,18 +54,6 @@ impl QueryClient { }) } - pub async fn ping(&mut self) -> Result { - self.send_query(QueryServerRequest::Ping) - .await - .and_then(|(response, duration)| { - if let QueryServerResponse::Pong = response { - Ok(duration) - } else { - Err(QueryClientError::InvalidResponse) - } - }) - } - async fn send_query( &mut self, request: QueryServerRequest, @@ -73,42 +61,20 @@ impl QueryClient { let socket = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0)).await?; for _ in 0..MAX_REQUEST_RETRIES { - let mut buf = Vec::with_capacity(VELOREN_HEADER.len() + 2 + MAX_REQUEST_SIZE); - - // 2 extra bytes for version information, currently unused - buf.extend(VERSION.to_le_bytes()); - buf.extend({ - let request_data = if let Some(init) = &self.init { - // TODO: Use the maximum version supported by both the client and server once - // new protocol versions are added - ::raw_bytes( - &RawQueryServerRequest { p: init.p, request }, - &Default::default(), - )? - } else { - // TODO: Use the legacy version here once new protocol versions are added - ::raw_bytes( - &RawQueryServerRequest { p: 0, request }, - &Default::default(), - )? - }; - if request_data.len() > MAX_REQUEST_SIZE { - warn!( - ?request, - ?MAX_REQUEST_SIZE, - "Attempted to send request larger than the max size ({})", - request_data.len() - ); - Err(QueryClientError::RequestTooLarge)? + let request = if let Some(init) = &self.init { + // TODO: Use the maximum version supported by both the client and server once + // new protocol versions are added + RawQueryServerRequest { p: init.p, request } + } else { + // TODO: Use the legacy version here once new protocol versions are added + RawQueryServerRequest { + p: 0, + request: QueryServerRequest::Init, } - request_data - }); - buf.resize(2 + MAX_RESPONSE_SIZE, 0); - buf.extend(VELOREN_HEADER); - + }; + let buf = request.serialize()?; let query_sent = Instant::now(); socket.send_to(&buf, self.addr).await?; - let mut buf = vec![0; MAX_RESPONSE_SIZE]; let (buf_len, _) = timeout(Duration::from_secs(2), socket.recv_from(&mut buf)) .await @@ -119,7 +85,6 @@ impl QueryClient { } let packet = ::read( - // TODO: Remove this padding once version information is added to packets &mut io::Cursor::new(&buf[..buf_len]), &Default::default(), )?; diff --git a/common/query_server/src/lib.rs b/common/query_server/src/lib.rs index 6df1b6ff37..8f57862ab8 100644 --- a/common/query_server/src/lib.rs +++ b/common/query_server/src/lib.rs @@ -1,4 +1,4 @@ #[cfg(feature = "client")] pub mod client; pub mod proto; -mod ratelimit; +#[cfg(feature = "server")] mod ratelimit; #[cfg(feature = "server")] pub mod server; diff --git a/common/query_server/src/proto.rs b/common/query_server/src/proto.rs index 159d842437..b65d6f67fa 100644 --- a/common/query_server/src/proto.rs +++ b/common/query_server/src/proto.rs @@ -2,11 +2,10 @@ use protocol::Protocol; pub(crate) const VERSION: u16 = 0; pub(crate) const VELOREN_HEADER: [u8; 7] = [b'v', b'e', b'l', b'o', b'r', b'e', b'n']; -// The actual maximum size of packets will be `MAX_REQUEST_SIZE + -// VELOREN_HEADER.len() + 2` (2 added for currently unused version). +pub(crate) const MAX_REQUEST_CONTENT_SIZE: usize = 300; // NOTE: The actual maximum size must never exceed 1200 or we risk getting near // MTU limits for some networks. -pub(crate) const MAX_REQUEST_SIZE: usize = 300; +pub(crate) const MAX_REQUEST_SIZE: usize = MAX_REQUEST_CONTENT_SIZE + VELOREN_HEADER.len() + 2; pub(crate) const MAX_RESPONSE_SIZE: usize = 256; #[derive(Protocol, Debug, Clone, Copy)] @@ -21,9 +20,17 @@ pub(crate) struct RawQueryServerRequest { #[protocol(discriminator(u8))] #[allow(clippy::large_enum_variant)] pub enum QueryServerRequest { + /// This requests exists mostly for backwards-compatibilty reasons. As the + /// first message sent to the server should always be in the V0 version + /// of the protocol, if future versions of the protocol have more + /// requests than server info it may be confusing to request `P` and the max + /// version with a `QueryServerRequest::ServerInfo` request (the request + /// will still be dropped as the supplied `P` value is invalid). + Init, ServerInfo, - Ping, - // New requests should be added at the end to prevent breakage + // New requests should be added at the end to prevent breakage. + // NOTE: Any new (sub-)variants must be added to the `check_request_sizes` test at the end of + // this file } #[derive(Protocol, Debug, Clone, Copy)] @@ -56,14 +63,13 @@ pub(crate) enum RawQueryServerResponse { #[protocol(discriminator(u8))] pub enum QueryServerResponse { ServerInfo(ServerInfo), - Pong, // New responses should be added at the end to prevent breakage } #[derive(Protocol, Debug, Clone, Copy, PartialEq, Eq)] pub struct ServerInfo { pub git_hash: u32, - pub git_version: i64, + pub git_timestamp: i64, pub players_count: u16, pub player_cap: u16, pub battlemode: ServerBattleMode, @@ -78,3 +84,49 @@ pub enum ServerBattleMode { GlobalPvE, PerPlayer, } + +impl RawQueryServerRequest { + #[cfg(any(feature = "client", test))] + pub fn serialize(&self) -> Result, protocol::Error> { + use protocol::Parcel; + + let mut buf = Vec::with_capacity(MAX_REQUEST_SIZE); + + // 2 extra bytes for version information, currently unused + buf.extend(VERSION.to_le_bytes()); + buf.extend({ + let request_data = + ::raw_bytes(self, &Default::default())?; + if request_data.len() > MAX_REQUEST_CONTENT_SIZE { + panic!( + "Attempted to send request larger than the max size (size: {}, max size: \ + {MAX_REQUEST_CONTENT_SIZE}, request: {self:?})", + request_data.len() + ); + } + request_data + }); + const _: () = assert!(MAX_RESPONSE_SIZE + VELOREN_HEADER.len() <= MAX_REQUEST_SIZE); + buf.resize(MAX_RESPONSE_SIZE.max(buf.len()), 0); + buf.extend(VELOREN_HEADER); + Ok(buf) + } +} + +#[cfg(test)] +mod tests { + use super::{QueryServerRequest, RawQueryServerRequest}; + + #[test] + fn check_request_sizes() { + const ALL_REQUESTS: &[QueryServerRequest] = + &[QueryServerRequest::ServerInfo, QueryServerRequest::Init]; + for request in ALL_REQUESTS { + let request = RawQueryServerRequest { + p: 0, + request: *request, + }; + request.serialize().unwrap(); // This will panic if the size is above MAX_REQUEST_SIZE + } + } +} diff --git a/common/query_server/src/server.rs b/common/query_server/src/server.rs index a89cd7ad48..194a722a5a 100644 --- a/common/query_server/src/server.rs +++ b/common/query_server/src/server.rs @@ -37,7 +37,7 @@ pub struct Metrics { pub invalid_packets: u32, pub proccessing_errors: u32, pub info_requests: u32, - pub ping_requests: u32, + pub init_requests: u32, pub sent_responses: u32, pub failed_responses: u32, pub timed_out_responses: u32, @@ -129,13 +129,13 @@ impl QueryServer { if len < MAX_RESPONSE_SIZE.max(VELOREN_HEADER.len() + 2) { trace!(?len, "Datagram too short"); false - } else if len > (MAX_REQUEST_SIZE + VELOREN_HEADER.len() + 2) { + } else if len > MAX_REQUEST_SIZE { trace!(?len, "Datagram too large"); false } else if data[(len - VELOREN_HEADER.len())..] != VELOREN_HEADER { trace!(?len, "Datagram header invalid"); false - // FIXME: Allow lower versions once proper versioning is added. + // TODO: Allow lower versions once proper versioning is added. } else if u16::from_ne_bytes(data[..2].try_into().unwrap()) != VERSION { trace!( "Datagram has invalid version {:?}, current {VERSION:?}", @@ -205,21 +205,24 @@ impl QueryServer { } match request { - QueryServerRequest::ServerInfo => { - metrics.info_requests += 1; - let server_info = *self.server_info.borrow(); + QueryServerRequest::Init => { + metrics.init_requests += 1; Self::send_response( - RawQueryServerResponse::Response(QueryServerResponse::ServerInfo(server_info)), + RawQueryServerResponse::Init(Init { + p: real_p, + max_supported_version: VERSION, + }), remote, socket, metrics, ) .await; }, - QueryServerRequest::Ping => { - metrics.ping_requests += 1; + QueryServerRequest::ServerInfo => { + metrics.info_requests += 1; + let server_info = *self.server_info.borrow(); Self::send_response( - RawQueryServerResponse::Response(QueryServerResponse::Pong), + RawQueryServerResponse::Response(QueryServerResponse::ServerInfo(server_info)), remote, socket, metrics, @@ -283,7 +286,7 @@ impl std::ops::AddAssign for Metrics { invalid_packets, proccessing_errors, info_requests, - ping_requests, + init_requests, sent_responses, failed_responses, timed_out_responses, @@ -295,7 +298,7 @@ impl std::ops::AddAssign for Metrics { self.invalid_packets += invalid_packets; self.proccessing_errors += proccessing_errors; self.info_requests += info_requests; - self.ping_requests += ping_requests; + self.init_requests += init_requests; self.sent_responses += sent_responses; self.failed_responses += failed_responses; self.timed_out_responses += timed_out_responses; diff --git a/common/src/util/mod.rs b/common/src/util/mod.rs index 8bded12a1e..3a282c5952 100644 --- a/common/src/util/mod.rs +++ b/common/src/util/mod.rs @@ -23,7 +23,7 @@ lazy_static::lazy_static! { pub static ref GIT_DATE: String = GIT_DATETIME.split('-').take(3).collect::>().join("-"); pub static ref GIT_TIME: &'static str = GIT_DATETIME.split('-').nth(3).expect("failed to retrieve git_time!"); pub static ref GIT_DATE_TIMESTAMP: i64 = - NaiveDateTime::parse_from_str(dbg!(&*GIT_DATETIME), "%Y-%m-%d-%H:%M") + NaiveDateTime::parse_from_str(*GIT_DATETIME, "%Y-%m-%d-%H:%M") .expect("Invalid date") .and_utc().timestamp(); pub static ref DISPLAY_VERSION: String = if GIT_TAG.is_empty() { diff --git a/server/Cargo.toml b/server/Cargo.toml index 23ba5bd05c..df5978e7e5 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -18,7 +18,7 @@ default = ["worldgen", "plugins", "persistent_world", "simd"] [dependencies] common = { package = "veloren-common", path = "../common" } common-base = { package = "veloren-common-base", path = "../common/base" } -veloren-query-server = { package = "veloren-query-server", path = "../common/query_server", features = ["server"] } +veloren-query-server = { package = "veloren-query-server", path = "../common/query_server", default-features = false, features = ["server"] } common-ecs = { package = "veloren-common-ecs", path = "../common/ecs" } common-state = { package = "veloren-common-state", path = "../common/state" } common-systems = { package = "veloren-common-systems", path = "../common/systems" } diff --git a/server/src/lib.rs b/server/src/lib.rs index 86bdafb1d0..824256ff0a 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -609,7 +609,7 @@ impl Server { let (query_server_info_tx, query_server_info_rx) = tokio::sync::watch::channel(ServerInfo { git_hash: *sys::server_info::GIT_HASH, - git_version: *GIT_DATE_TIMESTAMP, + git_timestamp: *GIT_DATE_TIMESTAMP, players_count: 0, player_cap: settings.max_players, battlemode: settings.gameplay.battle_mode.into(), diff --git a/server/src/metrics.rs b/server/src/metrics.rs index 41b5da54b0..64a5e25774 100644 --- a/server/src/metrics.rs +++ b/server/src/metrics.rs @@ -76,7 +76,7 @@ pub struct QueryServerMetrics { pub invalid_packets: IntCounter, pub proccessing_errors: IntCounter, pub info_requests: IntCounter, - pub ping_requests: IntCounter, + pub init_requests: IntCounter, pub sent_responses: IntCounter, pub failed_responses: IntCounter, pub timed_out_responses: IntCounter, @@ -461,9 +461,9 @@ impl QueryServerMetrics { "query_server::info_requests", "Amount of server info requests received by the query server", ))?; - let ping_requests = IntCounter::with_opts(Opts::new( + let init_requests = IntCounter::with_opts(Opts::new( "query_server::ping_requests", - "Amount of server ping requests received by the query server", + "Amount of init requests received by the query server", ))?; let sent_responses = IntCounter::with_opts(Opts::new( "query_server::sent_responses", @@ -487,7 +487,7 @@ impl QueryServerMetrics { registry.register(Box::new(invalid_packets.clone()))?; registry.register(Box::new(proccessing_errors.clone()))?; registry.register(Box::new(info_requests.clone()))?; - registry.register(Box::new(ping_requests.clone()))?; + registry.register(Box::new(init_requests.clone()))?; registry.register(Box::new(sent_responses.clone()))?; registry.register(Box::new(failed_responses.clone()))?; registry.register(Box::new(timed_out_responses.clone()))?; @@ -499,7 +499,7 @@ impl QueryServerMetrics { invalid_packets, proccessing_errors, info_requests, - ping_requests, + init_requests, sent_responses, failed_responses, timed_out_responses, @@ -515,7 +515,7 @@ impl QueryServerMetrics { invalid_packets, proccessing_errors, info_requests, - ping_requests, + init_requests, sent_responses, failed_responses, timed_out_responses, @@ -527,7 +527,7 @@ impl QueryServerMetrics { self.invalid_packets.inc_by(invalid_packets as u64); self.proccessing_errors.inc_by(proccessing_errors as u64); self.info_requests.inc_by(info_requests as u64); - self.ping_requests.inc_by(ping_requests as u64); + self.init_requests.inc_by(init_requests as u64); self.sent_responses.inc_by(sent_responses as u64); self.failed_responses.inc_by(failed_responses as u64); self.timed_out_responses.inc_by(timed_out_responses as u64); diff --git a/server/src/sys/server_info.rs b/server/src/sys/server_info.rs index 92ccb00937..e89d67153e 100644 --- a/server/src/sys/server_info.rs +++ b/server/src/sys/server_info.rs @@ -37,7 +37,7 @@ impl<'a> System<'a> for Sys { let count = players.count().try_into().unwrap_or(u16::MAX); if let Err(error) = sender.send(ServerInfo { git_hash: *GIT_HASH, - git_version: *GIT_DATE_TIMESTAMP, + git_timestamp: *GIT_DATE_TIMESTAMP, players_count: count, player_cap: settings.max_players, battlemode: settings.gameplay.battle_mode.into(),