more tests

Author: Byron

git-protocol/src/packet_line/decode.rs

@@ -22,12 +22,14 @@ quick_error! {
     }
 }
 
+#[derive(Debug, Clone)]
 pub enum Stream<'a> {
     Complete {
         line: packet_line::Borrowed<'a>,
         bytes_consumed: usize,
     },
     Incomplete {
+        /// The amount of additional bytes needed for the parsing to complete
         bytes_needed: usize,
     },
 }
@@ -50,18 +52,26 @@ pub fn streaming(data: &[u8]) -> Result<Stream, Error> {
     let mut buf = [0u8; U16_HEX_BYTES / 2];
     hex::decode_to_slice(hex_bytes, &mut buf)?;
     let wanted_bytes = u16::from_be_bytes(buf) as usize;
+    if wanted_bytes > MAX_LINE_LEN {
+        return Err(Error::DataLengthLimitExceeded(wanted_bytes));
+    }
     if data_len < wanted_bytes {
         return Ok(Stream::Incomplete {
-            bytes_needed: wanted_bytes,
+            bytes_needed: wanted_bytes - data_len,
         });
     }
-    if wanted_bytes > MAX_LINE_LEN {
-        return Err(Error::DataLengthLimitExceeded(wanted_bytes));
+
+    if wanted_bytes == 4 {
+        return Err(Error::DataIsEmpty);
     }
 
     // todo: error line
+    let mut data = &data[U16_HEX_BYTES..wanted_bytes];
+    if data[data.len() - 1] == b'\n' {
+        data = &data[..data.len() - 1];
+    }
     Ok(Stream::Complete {
-        line: packet_line::Borrowed::Data(&data[U16_HEX_BYTES..wanted_bytes]),
+        line: packet_line::Borrowed::Data(data),
         bytes_consumed: wanted_bytes,
     })
 }

git-protocol/src/packet_line/mod.rs

@@ -1,3 +1,4 @@
+use bstr::BStr;
 use std::io;
 
 pub(crate) const U16_HEX_BYTES: usize = 4;
@@ -26,6 +27,9 @@ impl<'a> Borrowed<'a> {
             Borrowed::Flush => &[],
         }
     }
+    pub fn as_bstr(&self) -> &BStr {
+        self.as_slice().into()
+    }
 }
 
 pub mod decode;

git-protocol/tests/packet_line/decode.rs

@@ -1,4 +1,5 @@
 mod streaming {
+    use crate::packet_line::assert_err_display;
     use git_protocol::{
         packet_line::decode::{self, streaming, Stream},
         PacketLine,
@@ -12,7 +13,7 @@ mod streaming {
         match res? {
             Stream::Complete { line, bytes_consumed } => {
                 assert_eq!(bytes_consumed, expected_consumed);
-                assert_eq!(line, expected_value);
+                assert_eq!(line.as_bstr(), expected_value.as_bstr());
             }
             Stream::Incomplete { .. } => panic!("expected parsing to be complete, not partial"),
         }
@@ -24,6 +25,32 @@ mod streaming {
         assert_complete(streaming(b"0000someotherstuff"), 4, PacketLine::Flush)
     }
 
+    #[test]
+    fn trailing_line_feeds_are_removed() -> crate::Result {
+        assert_complete(streaming(b"0006a\n"), 6, PacketLine::Data(b"a"))
+    }
+
+    #[test]
+    fn error_on_oversized_line() {
+        assert_err_display(
+            streaming(b"ffff"),
+            "The data received claims to be larger than than the maximum allowed size: got 65535, exceeds 65516",
+        );
+    }
+
+    #[test]
+    fn error_on_invalid_hex() {
+        assert_err_display(
+            streaming(b"fooo"),
+            "Failed to decode the first four hex bytes indicating the line length",
+        );
+    }
+
+    #[test]
+    fn error_on_empty_line() {
+        assert_err_display(streaming(b"0004"), "Received an invalid empty line");
+    }
+
     #[test]
     fn round_trips() -> crate::Result {
         for (line, bytes) in &[(PacketLine::Flush, 4), (PacketLine::Data(b"hello there"), 15)] {
@@ -33,4 +60,34 @@ mod streaming {
         }
         Ok(())
     }
+
+    mod incomplete {
+        use git_protocol::packet_line::decode::{self, streaming, Stream};
+
+        fn assert_incomplete(res: Result<Stream, decode::Error>, expected_missing: usize) -> crate::Result {
+            match res? {
+                Stream::Complete { .. } => {
+                    panic!("expected parsing to be partial, not complete");
+                }
+                Stream::Incomplete { bytes_needed } => {
+                    assert_eq!(bytes_needed, expected_missing);
+                }
+            }
+            Ok(())
+        }
+
+        #[test]
+        fn missing_hex_bytes() -> crate::Result {
+            assert_incomplete(streaming(b"0"), 3)?;
+            assert_incomplete(streaming(b"00"), 2)?;
+            Ok(())
+        }
+
+        #[test]
+        fn missing_data_bytes() -> crate::Result {
+            assert_incomplete(streaming(b"0005"), 1)?;
+            assert_incomplete(streaming(b"0006a"), 1)?;
+            Ok(())
+        }
+    }
 }

git-protocol/tests/packet_line/encode.rs

@@ -1,4 +1,5 @@
 mod data_to_write {
+    use crate::packet_line::assert_err_display;
     use bstr::ByteSlice;
     use git_protocol::packet_line::encode::{data_to_write, flush_to_write};
     use std::io;
@@ -9,13 +10,6 @@ mod data_to_write {
         v
     }
 
-    fn assert_err_display<T, E: std::error::Error>(res: Result<T, E>, expected: impl AsRef<str>) {
-        match res {
-            Ok(_) => assert!(false, "Expected error '{}', got value", expected.as_ref()),
-            Err(err) => assert_eq!(err.to_string(), expected.as_ref()),
-        }
-    }
-
     #[test]
     fn success_binary_and_non_binary() -> crate::Result {
         let mut out = Vec::new();

git-protocol/tests/packet_line/mod.rs

@@ -1,2 +1,9 @@
+fn assert_err_display<T: std::fmt::Debug, E: std::error::Error>(res: Result<T, E>, expected: impl AsRef<str>) {
+    match res {
+        Ok(v) => assert!(false, "Expected error '{}', got value {:?}", expected.as_ref(), v),
+        Err(err) => assert_eq!(err.to_string(), expected.as_ref()),
+    }
+}
+
 mod decode;
 mod encode;