Made commands a distinct ClientMsg to avoid possible sanitisation problems for clients

Cargo.lock

@@ -707,6 +707,12 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "comma"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "96677551532ffe910f470bd767a9a7daf9ba53b1f5532e0891dba6c735f692e5"
+
 [[package]]
 name = "concurrent-queue"
 version = "1.2.2"
@@ -6049,6 +6055,7 @@ dependencies = [
  "bincode",
  "bytemuck",
  "chrono",
+ "comma",
  "conrod_core",
  "conrod_winit",
  "copy_dir",

client/src/lib.rs

@@ -808,9 +808,9 @@ impl Client {
                     //Only in game, terrain
                     ClientGeneral::TerrainChunkRequest { .. } => &mut self.terrain_stream,
                     //Always possible
-                    ClientGeneral::ChatMsg(_) | ClientGeneral::Terminate => {
-                        &mut self.general_stream
-                    },
+                    ClientGeneral::ChatMsg(_)
+                    | ClientGeneral::Command(_, _)
+                    | ClientGeneral::Terminate => &mut self.general_stream,
                 };
                 stream.send(msg)
             },
@@ -1370,6 +1370,11 @@ impl Client {
         }
     }
 
+    /// Send a command to the server.
+    pub fn send_command(&mut self, name: String, args: Vec<String>) {
+        self.send_msg(ClientGeneral::Command(name, args));
+    }
+
     /// Remove all cached terrain
     pub fn clear_terrain(&mut self) {
         self.state.clear_terrain();

common/net/src/msg/client.rs

@@ -80,6 +80,7 @@ pub enum ClientGeneral {
     },
     //Always possible
     ChatMsg(String),
+    Command(String, Vec<String>),
     Terminate,
     RequestPlayerPhysics {
         server_authoritative: bool,
@@ -129,7 +130,9 @@ impl ClientMsg {
                             c_type == ClientType::Game && presence.is_some()
                         },
                         //Always possible
-                        ClientGeneral::ChatMsg(_) | ClientGeneral::Terminate => true,
+                        ClientGeneral::ChatMsg(_)
+                        | ClientGeneral::Command(_, _)
+                        | ClientGeneral::Terminate => true,
                     }
             },
             ClientMsg::Ping(_) => true,

common/src/event.rs

@@ -140,7 +140,7 @@ pub enum ServerEvent {
     ClientDisconnect(EcsEntity, DisconnectReason),
     ClientDisconnectWithoutPersistence(EcsEntity),
     ChunkRequest(EcsEntity, Vec2<i32>),
-    ChatCmd(EcsEntity, String),
+    ChatCmd(EcsEntity, String, Vec<String>),
     /// Send a chat message to the player from an npc or other player
     Chat(comp::UnresolvedChatMsg),
     Aura {

server/src/cmd.rs

@@ -52,11 +52,15 @@ use scan_fmt::{scan_fmt, scan_fmt_some};
 use tracing::{error, info, warn};
 
 pub trait ChatCommandExt {
-    fn execute(&self, server: &mut Server, entity: EcsEntity, args: String);
+    fn execute(&self, server: &mut Server, entity: EcsEntity, args: Vec<String>);
 }
 impl ChatCommandExt for ChatCommand {
     #[allow(clippy::needless_return)] // TODO: Pending review in #587
-    fn execute(&self, server: &mut Server, entity: EcsEntity, args: String) {
+    fn execute(&self, server: &mut Server, entity: EcsEntity, args: Vec<String>) {
+        // TODO: Pass arguments to commands as Vec<String>, not String, to support
+        // proper parsing.
+        let args = args.join(" ");
+
         if let Err(err) = do_command(server, entity, entity, args, self) {
             server.notify_client(
                 entity,
@@ -101,6 +105,7 @@ fn do_command(
             cmd.keyword()
         ));
     }
+
     let handler: CommandHandler = match cmd {
         ChatCommand::Adminify => handle_adminify,
         ChatCommand::Airship => handle_spawn_airship,

server/src/events/mod.rs

@@ -188,8 +188,8 @@ impl Server {
                 ServerEvent::ChunkRequest(entity, key) => {
                     requested_chunks.push((entity, key));
                 },
-                ServerEvent::ChatCmd(entity, cmd) => {
-                    chat_commands.push((entity, cmd));
+                ServerEvent::ChatCmd(entity, name, args) => {
+                    chat_commands.push((entity, name, args));
                 },
                 ServerEvent::Chat(msg) => {
                     chat_messages.push(msg);
@@ -229,8 +229,8 @@ impl Server {
             self.generate_chunk(entity, key);
         }
 
-        for (entity, cmd) in chat_commands {
-            self.process_chat_cmd(entity, cmd);
+        for (entity, name, args) in chat_commands {
+            self.process_chat_cmd(entity, name, args);
         }
 
         for msg in chat_messages {

server/src/lib.rs

@@ -983,16 +983,9 @@ impl Server {
         );
     }
 
-    fn process_chat_cmd(&mut self, entity: EcsEntity, cmd: String) {
-        // Separate string into keyword and arguments.
-        let sep = cmd.find(' ');
-        let (kwd, args) = match sep {
-            Some(i) => (cmd[..i].to_string(), cmd[(i + 1)..].to_string()),
-            None => (cmd, "".to_string()),
-        };
-
+    fn process_chat_cmd(&mut self, entity: EcsEntity, name: String, args: Vec<String>) {
         // Find the command object and run its handler.
-        if let Ok(command) = kwd.parse::<ChatCommand>() {
+        if let Ok(command) = name.parse::<ChatCommand>() {
             command.execute(self, entity, args);
         } else {
             #[cfg(feature = "plugins")]
@@ -1020,8 +1013,8 @@ impl Server {
                 let rs = plugin_manager.execute_event(
                     &ecs_world,
                     &plugin_api::event::ChatCommandEvent {
-                        command: kwd.clone(),
-                        command_args: args.split(' ').map(|x| x.to_owned()).collect(),
+                        command: name.clone(),
+                        command_args: args.clone(),
                         player: plugin_api::event::Player { id: uid },
                     },
                 );
@@ -1035,7 +1028,7 @@ impl Server {
                                     format!(
                                         "Unknown command '/{}'.\nType '/help' for available \
                                          commands",
-                                        kwd
+                                        name
                                     ),
                                 ),
                             );
@@ -1059,7 +1052,7 @@ impl Server {
                                             comp::ChatType::CommandError,
                                             format!(
                                                 "Error occurred while executing command '/{}'.\n{}",
-                                                kwd, e
+                                                name, e
                                             ),
                                         ),
                                     );
@@ -1068,7 +1061,7 @@ impl Server {
                         }
                     },
                     Err(e) => {
-                        error!(?e, "Can't execute command {} {}", kwd, args);
+                        error!(?e, "Can't execute command {} {:?}", name, args);
                         self.notify_client(
                             entity,
                             ServerGeneral::server_msg(
@@ -1076,7 +1069,7 @@ impl Server {
                                 format!(
                                     "Internal error while executing '/{}'.\nContact the server \
                                      administrator",
-                                    kwd
+                                    name
                                 ),
                             ),
                         );

server/src/sys/msg/general.rs

@@ -29,12 +29,7 @@ impl Sys {
                 if player.is_some() {
                     match validate_chat_msg(&message) {
                         Ok(()) => {
-                            if let Some(message) = message.strip_prefix('/') {
-                                if !message.is_empty() {
-                                    let argv = String::from(message);
-                                    server_emitter.emit(ServerEvent::ChatCmd(entity, argv));
-                                }
-                            } else if let Some(from) = uids.get(entity) {
+                            if let Some(from) = uids.get(entity) {
                                 const CHAT_MODE_DEFAULT: &ChatMode = &ChatMode::default();
                                 let mode = chat_modes.get(entity).unwrap_or(CHAT_MODE_DEFAULT);
                                 // Send chat message
@@ -52,6 +47,11 @@ impl Sys {
                     }
                 }
             },
+            ClientGeneral::Command(name, args) => {
+                if player.is_some() {
+                    server_emitter.emit(ServerEvent::ChatCmd(entity, name, args));
+                }
+            },
             ClientGeneral::Terminate => {
                 debug!(?entity, "Client send message to terminate session");
                 server_emitter.emit(ServerEvent::ClientDisconnect(

voxygen/Cargo.toml

@@ -78,6 +78,7 @@ backtrace = "0.3.40"
 bincode = "1.3.1"
 chrono = { version = "0.4.9", features = ["serde"] }
 cpal = "0.13"
+comma = "0.1"
 copy_dir = "0.1.2"
 crossbeam-utils = "0.8.1"
 crossbeam-channel = "0.5"

voxygen/src/hud/chat.rs

@@ -168,6 +168,7 @@ pub struct State {
 pub enum Event {
     TabCompletionStart(String),
     SendMessage(String),
+    SendCommand(String, Vec<String>),
     Focus(Id),
     ChangeChatTab(Option<usize>),
     ShowChatTabSettings(usize),
@@ -645,7 +646,17 @@ impl<'a> Widget for Chat<'a> {
                     s.history.truncate(self.history_max);
                 }
             });
-            events.push(Event::SendMessage(msg));
+            if let Some(msg) = msg.strip_prefix('/') {
+                match msg.parse::<comma::Command>() {
+                    Ok(cmd) => events.push(Event::SendCommand(cmd.name, cmd.arguments)),
+                    Err(err) => self.new_messages.push_back(ChatMsg {
+                        chat_type: ChatType::CommandError,
+                        message: err.to_string(),
+                    }),
+                }
+            } else {
+                events.push(Event::SendMessage(msg));
+            }
         }
         events
     }

voxygen/src/hud/mod.rs

@@ -375,6 +375,7 @@ pub struct HudInfo {
 #[derive(Clone)]
 pub enum Event {
     SendMessage(String),
+    SendCommand(String, Vec<String>),
 
     CharacterSelection,
     UseSlot {
@@ -2722,6 +2723,9 @@ impl Hud {
                 chat::Event::SendMessage(message) => {
                     events.push(Event::SendMessage(message));
                 },
+                chat::Event::SendCommand(name, args) => {
+                    events.push(Event::SendCommand(name, args));
+                },
                 chat::Event::Focus(focus_id) => {
                     self.to_focus = Some(Some(focus_id));
                 },

voxygen/src/session/mod.rs

@@ -1032,6 +1032,9 @@ impl PlayState for SessionState {
                         // TODO: Handle result
                         self.client.borrow_mut().send_chat(msg);
                     },
+                    HudEvent::SendCommand(name, args) => {
+                        self.client.borrow_mut().send_command(name, args);
+                    },
                     HudEvent::CharacterSelection => {
                         self.client.borrow_mut().request_remove_character()
                     },