bin_ser: reject trailing bytes by default (#587)

Changes `LeSer`/`BeSer::des`. Also adds a new `des_prefix` function to
keep a way to allow trailing bytes.

pageserver/src/repository.rs

@@ -213,6 +213,7 @@ mod tests {
     use crate::walredo::{WalRedoError, WalRedoManager};
     use crate::PageServerConf;
     use postgres_ffi::pg_constants;
+    use postgres_ffi::xlog_utils::SIZEOF_CHECKPOINT;
     use std::fs;
     use std::str::FromStr;
     use std::time::Duration;
@@ -252,6 +253,7 @@ mod tests {
     }
 
     static ZERO_PAGE: Bytes = Bytes::from_static(&[0u8; 8192]);
+    static ZERO_CHECKPOINT: Bytes = Bytes::from_static(&[0u8; SIZEOF_CHECKPOINT]);
 
     fn get_test_repo(test_name: &str) -> Result<Box<dyn Repository>> {
         let repo_dir = PageServerConf::test_repo_dir(test_name);
@@ -483,7 +485,7 @@ mod tests {
 
         // Import initial dummy checkpoint record, otherwise the get_timeline() call
         // after branching fails below
-        tline.put_page_image(RelishTag::Checkpoint, 0, Lsn(0x10), ZERO_PAGE.clone())?;
+        tline.put_page_image(RelishTag::Checkpoint, 0, Lsn(0x10), ZERO_CHECKPOINT.clone())?;
 
         // Create a relation on the timeline
         tline.put_page_image(TESTREL_A, 0, Lsn(0x20), TEST_IMG("foo blk 0 at 2"))?;
@@ -539,7 +541,7 @@ mod tests {
 
         // Import initial dummy checkpoint record, otherwise the get_timeline() call
         // after branching fails below
-        tline.put_page_image(RelishTag::Checkpoint, 0, Lsn(0x10), ZERO_PAGE.clone())?;
+        tline.put_page_image(RelishTag::Checkpoint, 0, Lsn(0x10), ZERO_CHECKPOINT.clone())?;
 
         // Create a relation on the timeline
         tline.put_page_image(TESTREL_A, 0, Lsn(0x20), TEST_IMG("foo blk 0 at 2"))?;

postgres_ffi/src/controlfile_utils.rs

@@ -58,7 +58,7 @@ impl ControlFileData {
         let expectedcrc = crc32c::crc32c(&buf[0..OFFSETOF_CRC]);
 
         // Use serde to deserialize the input as a ControlFileData struct.
-        let controlfile = ControlFileData::des(buf)?;
+        let controlfile = ControlFileData::des_prefix(buf)?;
 
         // Check the CRC
         if expectedcrc != controlfile.crc {

zenith_utils/src/bin_ser.rs

@@ -69,14 +69,13 @@ impl From<bincode::Error> for SerializeError {
 /// Properties:
 /// - Big endian
 /// - Fixed integer encoding (i.e. 1u32 is 00000001 not 01)
-/// - Allow trailing bytes: this means we don't throw an error
-///   if the deserializer is passed a buffer with more data
-///   past the end.
+///
+/// Does not allow trailing bytes in deserialization. If this is desired, you
+/// may set [`Options::allow_trailing_bytes`] to explicitly accomodate this.
 pub fn be_coder() -> impl Options {
     bincode::DefaultOptions::new()
         .with_big_endian()
         .with_fixint_encoding()
-        .allow_trailing_bytes()
 }
 
 /// A shortcut that configures little-ending binary serialization
@@ -84,14 +83,13 @@ pub fn be_coder() -> impl Options {
 /// Properties:
 /// - Little endian
 /// - Fixed integer encoding (i.e. 1u32 is 00000001 not 01)
-/// - Allow trailing bytes: this means we don't throw an error
-///   if the deserializer is passed a buffer with more data
-///   past the end.
+///
+/// Does not allow trailing bytes in deserialization. If this is desired, you
+/// may set [`Options::allow_trailing_bytes`] to explicitly accomodate this.
 pub fn le_coder() -> impl Options {
     bincode::DefaultOptions::new()
         .with_little_endian()
         .with_fixint_encoding()
-        .allow_trailing_bytes()
 }
 
 /// Binary serialize/deserialize helper functions (Big Endian)
@@ -118,13 +116,28 @@ pub trait BeSer: Serialize + DeserializeOwned {
         be_coder().serialize(&self).map_err(|e| e.into())
     }
 
-    /// Deserialize from a byte slice
+    /// Deserialize from the full contents of a byte slice
+    ///
+    /// See also: [`BeSer::des_prefix`]
     fn des(buf: &[u8]) -> Result<Self, DeserializeError> {
         be_coder()
             .deserialize(buf)
             .or(Err(DeserializeError::BadInput))
     }
 
+    /// Deserialize from a prefix of the byte slice
+    ///
+    /// Uses as much of the byte slice as is necessary to deserialize the
+    /// type, but does not guarantee that the entire slice is used.
+    ///
+    /// See also: [`BeSer::des`]
+    fn des_prefix(buf: &[u8]) -> Result<Self, DeserializeError> {
+        be_coder()
+            .allow_trailing_bytes()
+            .deserialize(buf)
+            .or(Err(DeserializeError::BadInput))
+    }
+
     /// Deserialize from a reader
     fn des_from<R: Read>(r: &mut R) -> Result<Self, DeserializeError> {
         be_coder().deserialize_from(r).map_err(|e| e.into())
@@ -163,13 +176,28 @@ pub trait LeSer: Serialize + DeserializeOwned {
         le_coder().serialize(&self).map_err(|e| e.into())
     }
 
-    /// Deserialize from a byte slice
+    /// Deserialize from the full contents of a byte slice
+    ///
+    /// See also: [`LeSer::des_prefix`]
     fn des(buf: &[u8]) -> Result<Self, DeserializeError> {
         le_coder()
             .deserialize(buf)
             .or(Err(DeserializeError::BadInput))
     }
 
+    /// Deserialize from a prefix of the byte slice
+    ///
+    /// Uses as much of the byte slice as is necessary to deserialize the
+    /// type, but does not guarantee that the entire slice is used.
+    ///
+    /// See also: [`LeSer::des`]
+    fn des_prefix(buf: &[u8]) -> Result<Self, DeserializeError> {
+        le_coder()
+            .allow_trailing_bytes()
+            .deserialize(buf)
+            .or(Err(DeserializeError::BadInput))
+    }
+
     /// Deserialize from a reader
     fn des_from<R: Read>(r: &mut R) -> Result<Self, DeserializeError> {
         le_coder().deserialize_from(r).map_err(|e| e.into())
@@ -247,8 +275,10 @@ mod tests {
         assert_eq!(decoded, SHORT2);
 
         // with trailing data
-        let decoded = ShortStruct::des(SHORT2_ENC_BE_TRAILING).unwrap();
+        let decoded = ShortStruct::des_prefix(SHORT2_ENC_BE_TRAILING).unwrap();
         assert_eq!(decoded, SHORT2);
+        let err = ShortStruct::des(SHORT2_ENC_BE_TRAILING).unwrap_err();
+        assert!(matches!(err, DeserializeError::BadInput));
 
         // serialize into a `Write` sink.
         let mut buf = Cursor::new(vec![0xFF; 8]);
@@ -279,8 +309,10 @@ mod tests {
         assert_eq!(decoded, SHORT2);
 
         // with trailing data
-        let decoded = ShortStruct::des(SHORT2_ENC_LE_TRAILING).unwrap();
+        let decoded = ShortStruct::des_prefix(SHORT2_ENC_LE_TRAILING).unwrap();
         assert_eq!(decoded, SHORT2);
+        let err = ShortStruct::des(SHORT2_ENC_LE_TRAILING).unwrap_err();
+        assert!(matches!(err, DeserializeError::BadInput));
 
         // serialize into a `Write` sink.
         let mut buf = Cursor::new(vec![0xFF; 8]);