ipc: do not implement openpgp::serialize traits

- Serialize and Marshal have certain guarentees
  such as not exporting non-exportable Signatures.

- In addition Serialize should only be implemented
  for standardized data structures.

- We are also pondering adding `to_armored` functionality for serialized data.
  This does not really work with the ipc data structures.

- Ipc provides serialization - but not the kind of serialization
  the openpgp crate is talking about.
  Plus we are considering sealing the traits.

3 files changed, 26 insertions, 176 deletions

diff --git a/ipc/src/gnupg.rs b/ipc/src/gnupg.rs
index 75bd182d..1492345c 100644
--- a/ipc/src/gnupg.rs
+++ b/ipc/src/gnupg.rs
@@ -20,7 +20,6 @@ use openpgp::fmt::hex;
 use openpgp::crypto;
 use openpgp::packet::prelude::*;
 use openpgp::parse::Parse;
-use openpgp::serialize::Serialize;
 
 use crate::Result;
 use crate::assuan;
diff --git a/ipc/src/sexp.rs b/ipc/src/sexp.rs
index 14d3e4c8..8386a827 100644
--- a/ipc/src/sexp.rs
+++ b/ipc/src/sexp.rs
@@ -21,7 +21,6 @@ use openpgp::Error;
 use openpgp::Result;
 
 mod parse;
-mod serialize;
 
 /// An *S-Expression*.
 ///
@@ -225,6 +224,21 @@ impl Sexp {
         }
     }
 
+
+    pub fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
+        match self {
+            Sexp::String(ref s) => s.serialize(o),
+            Sexp::List(ref l) => {
+                write!(o, "(")?;
+                for sexp in l {
+                    sexp.serialize(o)?;
+                }
+                write!(o, ")")?;
+                Ok(())
+            },
+        }
+    }
+
     /// Given an alist, selects by key and returns the value.
     fn get(&self, key: &[u8]) -> Result<Option<Vec<Sexp>>> {
         match self {
@@ -367,6 +381,17 @@ impl String_ {
     pub fn display_hint(&self) -> Option<&[u8]> {
         self.1.as_ref().map(|b| b.as_ref())
     }
+
+    pub fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
+        if let Some(display) = self.display_hint() {
+            write!(o, "[{}:", display.len())?;
+            o.write_all(display)?;
+            write!(o, "]")?;
+        }
+        write!(o, "{}:", self.len())?;
+        o.write_all(self)?;
+        Ok(())
+    }
 }
 
 impl From<&str> for String_ {
@@ -405,7 +430,6 @@ impl Arbitrary for String_ {
 mod tests {
     use super::*;
     use openpgp::parse::Parse;
-    use openpgp::serialize::Serialize;
 
     quickcheck::quickcheck! {
         fn roundtrip(s: Sexp) -> bool {
diff --git a/ipc/src/sexp/serialize.rs b/ipc/src/sexp/serialize.rs
deleted file mode 100644
index a360c01e..00000000
--- a/ipc/src/sexp/serialize.rs
+++ /dev/null
@@ -1,173 +0,0 @@
-use std::io;
-
-use sequoia_openpgp as openpgp;
-use openpgp::{
-    Result,
-    Error,
-    serialize::{
-        Marshal,
-        MarshalInto,
-    },
-};
-
-use crate::sexp::{Sexp, String_};
-
-impl openpgp::serialize::Serialize for Sexp {}
-
-impl Marshal for Sexp {
-    fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
-        match self {
-            Sexp::String(ref s) => s.serialize(o),
-            Sexp::List(ref l) => {
-                write!(o, "(")?;
-                for sexp in l {
-                    sexp.serialize(o)?;
-                }
-                write!(o, ")")?;
-                Ok(())
-            },
-        }
-    }
-}
-
-impl openpgp::serialize::SerializeInto for Sexp {}
-
-impl MarshalInto for Sexp {
-    fn serialized_len(&self) -> usize {
-        match self {
-            Sexp::String(ref s) => s.serialized_len(),
-            Sexp::List(ref l) =>
-                2 + l.iter().map(|s| s.serialized_len()).sum::<usize>(),
-        }
-    }
-
-    fn serialize_into(&self, buf: &mut [u8]) -> Result<usize> {
-        generic_serialize_into(self, self.serialized_len(), buf)
-    }
-}
-
-impl Marshal for String_ {
-    fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
-        if let Some(display) = self.display_hint() {
-            write!(o, "[{}:", display.len())?;
-            o.write_all(display)?;
-            write!(o, "]")?;
-        }
-        write!(o, "{}:", self.len())?;
-        o.write_all(self)?;
-        Ok(())
-    }
-}
-
-/// Computes the length of the size tag for a given string length.
-fn size_tag_len(len: usize) -> usize {
-    // Compute log10(self.len()).
-    let mut l = len;
-    let mut digits = 0;
-    while l > 0 {
-        l /= 10;
-        digits += 1;
-    }
-
-    std::cmp::max(1, digits) // 0 takes up 1 char, too.
-}
-
-impl MarshalInto for String_ {
-    fn serialized_len(&self) -> usize {
-        self.display_hint()
-            .map(|d| size_tag_len(d.len()) + 3 + d.len()).unwrap_or(0)
-            + size_tag_len(self.len()) + 1 + self.len()
-    }
-
-    fn serialize_into(&self, buf: &mut [u8]) -> Result<usize> {
-        generic_serialize_into(self, self.serialized_len(), buf)
-    }
-}
-
-/// Provides a generic implementation for SerializeInto::serialize_into.
-///
-/// For now, we express SerializeInto using Serialize.  In the future,
-/// we may provide implementations not relying on Serialize for a
-/// no_std configuration of this crate.
-fn generic_serialize_into(o: &dyn Marshal, serialized_len: usize,
-                          buf: &mut [u8])
-                          -> Result<usize> {
-    let buf_len = buf.len();
-    let mut cursor = ::std::io::Cursor::new(buf);
-    match o.serialize(&mut cursor) {
-        Ok(_) => (),
-        Err(e) => {
-            let short_write =
-                if let Some(ioe) = e.downcast_ref::<io::Error>() {
-                    ioe.kind() == io::ErrorKind::WriteZero
-                } else {
-                    false
-                };
-            return if short_write {
-                assert!(buf_len < serialized_len,
-                        "o.serialized_len() underestimated the required space");
-                Err(Error::InvalidArgument(
-                    format!("Invalid buffer size, expected {}, got {}",
-                            serialized_len, buf_len)).into())
-            } else {
-                Err(e)
-            }
-        }
-    };
-    Ok(cursor.position() as usize)
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn sexp() {
-        assert_eq!(
-            &Sexp::List(vec![]).to_vec().unwrap(),
-            b"()");
-        assert_eq!(
-            &Sexp::List(vec![Sexp::String(b"hi"[..].into()),
-                            Sexp::String(b"ho"[..].into()),
-            ]).to_vec().unwrap(),
-            b"(2:hi2:ho)");
-        assert_eq!(
-            &Sexp::List(vec![
-                Sexp::String(b"hi"[..].into()),
-                Sexp::String(String_::with_display_hint(b"ho".to_vec(),
-                                                        b"fancy".to_vec())),
-            ]).to_vec().unwrap(),
-            b"(2:hi[5:fancy]2:ho)");
-        assert_eq!(
-            &Sexp::List(vec![
-                Sexp::String(b"hi"[..].into()),
-                Sexp::List(vec![
-                    Sexp::String(b"ha"[..].into()),
-                    Sexp::String(b"ho"[..].into()),
-                ]),
-            ]).to_vec().unwrap(),
-            b"(2:hi(2:ha2:ho))");
-        assert_eq!(
-            &Sexp::List(vec![
-                Sexp::String(b"sig-val"[..].into()),
-                Sexp::List(vec![
-                    Sexp::String(b"rsa"[..].into()),
-                    Sexp::List(vec![
-                        Sexp::String(b"s"[..].into()),
-                        Sexp::String(b"abc"[..].into()),
-                    ]),
-                ]),
-            ]).to_vec().unwrap(),
-            b"(7:sig-val(3:rsa(1:s3:abc)))");
-    }
-
-    #[test]
-    fn string() {
-        assert_eq!(&String_::new(b"hi".to_vec()).to_vec().unwrap(),
-                   b"2:hi");
-        assert_eq!(&String_::with_display_hint(b"hi".to_vec(),
-                                               b"fancy".to_vec())
-                   .to_vec().unwrap(),
-                   b"[5:fancy]2:hi");
-    }
-}