add parser error handling
authoralfadur
Fri, 12 Apr 2019 22:36:54 +0300
changeset 14816 add191d825f4
parent 14815 fc2cfec95d86
child 14817 f5d43f007970
add parser error handling
rust/hedgewars-server/src/protocol.rs
rust/hedgewars-server/src/protocol/messages.rs
rust/hedgewars-server/src/protocol/parser.rs
rust/hedgewars-server/src/protocol/test.rs
rust/hedgewars-server/src/server/handlers.rs
rust/hedgewars-server/src/server/network.rs
--- a/rust/hedgewars-server/src/protocol.rs	Fri Apr 12 19:26:44 2019 +0300
+++ b/rust/hedgewars-server/src/protocol.rs	Fri Apr 12 22:36:54 2019 +0300
@@ -1,3 +1,5 @@
+use crate::protocol::parser::message;
+use log::*;
 use netbuf;
 use nom::{Err, ErrorKind, IResult};
 use std::io::{Read, Result};
@@ -10,6 +12,7 @@
 pub struct ProtocolDecoder {
     buf: netbuf::Buf,
     consumed: usize,
+    is_recovering: bool,
 }
 
 impl ProtocolDecoder {
@@ -17,26 +20,55 @@
         ProtocolDecoder {
             buf: netbuf::Buf::new(),
             consumed: 0,
+            is_recovering: false,
         }
     }
 
+    fn recover(&mut self) -> bool {
+        self.is_recovering = match parser::malformed_message(&self.buf[..]) {
+            Ok((tail, ())) => {
+                self.buf.consume(self.buf.len() - tail.len());
+                false
+            }
+            _ => {
+                self.buf.consume(self.buf.len());
+                true
+            }
+        };
+        !self.is_recovering
+    }
+
     pub fn read_from<R: Read>(&mut self, stream: &mut R) -> Result<usize> {
-        self.buf.read_from(stream)
+        let count = self.buf.read_from(stream)?;
+        if count > 0 && self.is_recovering {
+            self.recover();
+        }
+        Ok(count)
     }
 
     pub fn extract_messages(&mut self) -> Vec<messages::HWProtocolMessage> {
-        let parse_result = parser::extract_messages(&self.buf[..]);
-        match parse_result {
-            Ok((tail, msgs)) => {
-                self.consumed = self.buf.len() - self.consumed - tail.len();
-                msgs
+        let mut messages = vec![];
+        let mut consumed = 0;
+        if !self.is_recovering {
+            loop {
+                match parser::message(&self.buf[consumed..]) {
+                    Ok((tail, message)) => {
+                        messages.push(message);
+                        consumed += self.buf.len() - tail.len();
+                    }
+                    Err(nom::Err::Incomplete(_)) => break,
+                    Err(nom::Err::Failure(e)) | Err(nom::Err::Error(e)) => {
+                        debug!("Invalid message: {:?}", e);
+                        self.buf.consume(consumed);
+                        consumed = 0;
+                        if !self.recover() || self.buf.is_empty() {
+                            break;
+                        }
+                    }
+                }
             }
-            _ => unreachable!(),
         }
-    }
-
-    pub fn sweep(&mut self) {
-        self.buf.consume(self.consumed);
-        self.consumed = 0;
+        self.buf.consume(consumed);
+        messages
     }
 }
--- a/rust/hedgewars-server/src/protocol/messages.rs	Fri Apr 12 19:26:44 2019 +0300
+++ b/rust/hedgewars-server/src/protocol/messages.rs	Fri Apr 12 22:36:54 2019 +0300
@@ -62,8 +62,6 @@
     Delete(String),
     SaveRoom(String),
     LoadRoom(String),
-    Malformed,
-    Empty,
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -341,8 +339,6 @@
             Delete(name) => msg!["CMD", format!("DELETE {}", name)],
             SaveRoom(name) => msg!["CMD", format!("SAVEROOM {}", name)],
             LoadRoom(name) => msg!["CMD", format!("LOADROOM {}", name)],
-            Malformed => msg!["A", "QUICK", "BROWN", "HOG", "JUMPS", "OVER", "THE", "LAZY", "DOG"],
-            Empty => msg![""],
             _ => panic!("Protocol message not yet implemented"),
         }
     }
--- a/rust/hedgewars-server/src/protocol/parser.rs	Fri Apr 12 19:26:44 2019 +0300
+++ b/rust/hedgewars-server/src/protocol/parser.rs	Fri Apr 12 22:36:54 2019 +0300
@@ -531,18 +531,15 @@
     ))(input)
 }
 
-fn empty_message(input: &[u8]) -> HWResult<HWProtocolMessage> {
-    let (i, _) = alt((end_of_message, eol))(input)?;
-    Ok((i, Empty))
+pub fn malformed_message(input: &[u8]) -> HWResult<()> {
+    let (i, _) = terminatedc(input, |i| take_until(&b"\n\n"[..])(i), end_of_message)?;
+    Ok((i, ()))
 }
 
-fn malformed_message(input: &[u8]) -> HWResult<HWProtocolMessage> {
-    let (i, _) = separated_listc(input, eol, a_line)?;
-    Ok((i, Malformed))
-}
-
-fn message(input: &[u8]) -> HWResult<HWProtocolMessage> {
-    alt((
+pub fn message(input: &[u8]) -> HWResult<HWProtocolMessage> {
+    precededc(
+        input,
+        |i| take_while(|c| c == b'\n')(i),
         |i| {
             terminatedc(
                 i,
@@ -557,18 +554,17 @@
                 end_of_message,
             )
         },
-        |i| terminatedc(i, malformed_message, end_of_message),
-        empty_message,
-    ))(input)
+    )
 }
 
-pub fn extract_messages<'a>(input: &'a [u8]) -> HWResult<Vec<HWProtocolMessage>> {
-    many0(|i: &'a [u8]| complete!(i, message))(input)
+fn extract_messages(input: &[u8]) -> HWResult<Vec<HWProtocolMessage>> {
+    many0(message)(input)
 }
 
 #[cfg(test)]
 mod test {
     use super::{extract_messages, message};
+    use crate::protocol::parser::HWProtocolError;
     use crate::protocol::{messages::HWProtocolMessage::*, test::gen_proto_msg};
     use proptest::{proptest, proptest_helper};
 
@@ -596,8 +592,8 @@
         );
         assert_eq!(message(b"QUIT\n\n"), Ok((&b""[..], Quit(None))));
         assert_eq!(
-            message(b"CMD\nwatch demo\n\n"),
-            Ok((&b""[..], Watch("demo".to_string())))
+            message(b"CMD\nwatch 49471\n\n"),
+            Ok((&b""[..], Watch(49471)))
         );
         assert_eq!(
             message(b"BAN\nme\nbad\n77\n\n"),
@@ -617,25 +613,17 @@
         );
 
         assert_eq!(
-            extract_messages(b"QUIT\n1\n2\n\n"),
-            Ok((&b""[..], vec![Malformed]))
+            message(b"QUIT\n1\n2\n\n"),
+            Err(nom::Err::Error(HWProtocolError::new()))
         );
 
         assert_eq!(
-            extract_messages(b"PING\n\nPING\n\nP"),
-            Ok((&b"P"[..], vec![Ping, Ping]))
-        );
-        assert_eq!(
-            extract_messages(b"SING\n\nPING\n\n"),
-            Ok((&b""[..], vec![Malformed, Ping]))
-        );
-        assert_eq!(
             extract_messages(b"\n\n\n\nPING\n\n"),
-            Ok((&b""[..], vec![Empty, Empty, Ping]))
+            Ok((&b""[..], vec![Ping]))
         );
         assert_eq!(
             extract_messages(b"\n\n\nPING\n\n"),
-            Ok((&b""[..], vec![Empty, Empty, Ping]))
+            Ok((&b""[..], vec![Ping]))
         );
     }
 }
--- a/rust/hedgewars-server/src/protocol/test.rs	Fri Apr 12 19:26:44 2019 +0300
+++ b/rust/hedgewars-server/src/protocol/test.rs	Fri Apr 12 22:36:54 2019 +0300
@@ -129,6 +129,7 @@
                     hog(8),
                 ];
                 TeamInfo {
+                    owner: String::new(),
                     name,
                     color,
                     grave,
@@ -150,7 +151,7 @@
     type Parameters = ();
 
     fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
-        (0..2)
+        (0..=2)
             .no_shrink()
             .prop_flat_map(|i| {
                 proto_msg_match!(i, def = ServerVar::LatestProto(0),
@@ -166,14 +167,13 @@
 }
 
 pub fn gen_proto_msg() -> BoxedStrategy<HWProtocolMessage> where {
-    let res = (0..58).no_shrink().prop_flat_map(|i| {
-        proto_msg_match!(i, def = Malformed,
+    let res = (0..=55).no_shrink().prop_flat_map(|i| {
+        proto_msg_match!(i, def = Ping,
             0 => Ping(),
             1 => Pong(),
             2 => Quit(Option<Ascii>),
-            //3 => Cmd
             4 => Global(Ascii),
-            5 => Watch(Ascii),
+            5 => Watch(u32),
             6 => ToggleServerRegisteredOnly(),
             7 => SuperPower(),
             8 => Info(Ascii),
@@ -223,9 +223,7 @@
             52 => Save(Ascii, Ascii),
             53 => Delete(Ascii),
             54 => SaveRoom(Ascii),
-            55 => LoadRoom(Ascii),
-            56 => Malformed(),
-            57 => Empty()
+            55 => LoadRoom(Ascii)
         )
     });
     res.boxed()
--- a/rust/hedgewars-server/src/server/handlers.rs	Fri Apr 12 19:26:44 2019 +0300
+++ b/rust/hedgewars-server/src/server/handlers.rs	Fri Apr 12 22:36:54 2019 +0300
@@ -192,8 +192,6 @@
 ) {
     match message {
         HWProtocolMessage::Ping => response.add(Pong.send_self()),
-        HWProtocolMessage::Malformed => warn!("Malformed/unknown message"),
-        HWProtocolMessage::Empty => warn!("Empty message"),
         _ => {
             if server.anteroom.clients.contains(client_id) {
                 match loggingin::handle(server, client_id, response, message) {
--- a/rust/hedgewars-server/src/server/network.rs	Fri Apr 12 19:26:44 2019 +0300
+++ b/rust/hedgewars-server/src/server/network.rs	Fri Apr 12 22:36:54 2019 +0300
@@ -155,7 +155,6 @@
                 Err(error) => break Err(error),
             }
         };
-        decoder.sweep();
         result
     }