openpgp: Embed struct Container in the container packets.

  - This allows us to implement PartialEq and related traits more
    selectively.  See #93.

10 files changed, 169 insertions, 82 deletions

diff --git a/openpgp/src/message/mod.rs b/openpgp/src/message/mod.rs
index df38cf12..9592a703 100644
--- a/openpgp/src/message/mod.rs
+++ b/openpgp/src/message/mod.rs
@@ -997,9 +997,9 @@ mod tests {
         //  1: MDC
         // => good.
         let mut seip = SEIP1::new();
-        seip.children_mut().unwrap().push(
+        seip.container_mut().push(
             lit.clone().into());
-        seip.children_mut().unwrap().push(
+        seip.container_mut().push(
             MDC::from([0u8; 20]).into());
         packets[1] = seip.into();
 
@@ -1079,7 +1079,7 @@ mod tests {
         //  2: Literal
         // => bad.
         packets.remove(3);
-        packets[2].children_mut().unwrap().push(lit.clone().into());
+        packets[2].container_mut().unwrap().push(lit.clone().into());
 
         assert!(packets.iter().map(|p| p.tag()).collect::<Vec<Tag>>()
                 == [ Tag::SKESK, Tag::SKESK, Tag::SEIP ]);
@@ -1093,7 +1093,7 @@ mod tests {
         // 2: SEIP
         //  0: Literal
         // => good.
-        packets[2].children_mut().unwrap().packets.pop().unwrap();
+        packets[2].container_mut().unwrap().packets.pop().unwrap();
 
         #[allow(deprecated)]
         packets.insert(
diff --git a/openpgp/src/packet/aed.rs b/openpgp/src/packet/aed.rs
index a69dc575..b041533b 100644
--- a/openpgp/src/packet/aed.rs
+++ b/openpgp/src/packet/aed.rs
@@ -29,6 +29,9 @@ pub struct AED1 {
     chunk_size: usize,
     /// Initialization vector for the AEAD algorithm.
     iv: Box<[u8]>,
+
+    /// This is a container packet.
+    container: packet::Container,
 }
 
 impl AED1 {
@@ -55,6 +58,7 @@ impl AED1 {
             aead: aead,
             chunk_size: chunk_size,
             iv: iv,
+            container: Default::default(),
         })
     }
 
@@ -118,6 +122,8 @@ impl AED1 {
     }
 }
 
+impl_container_forwards!(AED1);
+
 impl From<AED1> for Packet {
     fn from(p: AED1) -> Self {
         super::AED::from(p).into()
diff --git a/openpgp/src/packet/compressed_data.rs b/openpgp/src/packet/compressed_data.rs
index b6914fe9..9cdb7e58 100644
--- a/openpgp/src/packet/compressed_data.rs
+++ b/openpgp/src/packet/compressed_data.rs
@@ -21,17 +21,18 @@ pub struct CompressedData {
     pub(crate) common: packet::Common,
     /// Algorithm used to compress the payload.
     algo: CompressionAlgorithm,
+
+    /// This is a container packet.
+    container: packet::Container,
 }
 
 impl fmt::Debug for CompressedData {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_struct("CompressedData")
             .field("algo", &self.algo)
-            .field("children",
-                   &self.common.children_ref()
-                       .map(|c| &c.packets).unwrap_or(&Vec::new()))
+            .field("children", &self.container.children_ref())
             .field("body (bytes)",
-                   &self.common.body().unwrap_or(&b"".to_vec()).len())
+                   &self.container.body().unwrap_or(&b"".to_vec()).len())
             .finish()
     }
 }
@@ -42,6 +43,7 @@ impl CompressedData {
         CompressedData {
             common: Default::default(),
             algo: algo,
+            container: Default::default(),
         }
     }
 
@@ -57,7 +59,7 @@ impl CompressedData {
 
     /// Adds a new packet to the container.
     pub fn push(mut self, packet: Packet) -> Self {
-        self.common.children_mut().unwrap().packets.push(packet);
+        self.container.push(packet);
         self
     }
 
@@ -66,11 +68,13 @@ impl CompressedData {
     /// container.  If `i` is one, it is inserted after the first
     /// packet, etc.
     pub fn insert(mut self, i: usize, packet: Packet) -> Self {
-        self.common.children_mut().unwrap().packets.insert(i, packet);
+        self.container.insert(i, packet);
         self
     }
 }
 
+impl_container_forwards!(CompressedData);
+
 impl From<CompressedData> for Packet {
     fn from(s: CompressedData) -> Self {
         Packet::CompressedData(s)
diff --git a/openpgp/src/packet/container.rs b/openpgp/src/packet/container.rs
new file mode 100644
index 00000000..79cbb99f
--- /dev/null
+++ b/openpgp/src/packet/container.rs
@@ -0,0 +1,45 @@
+macro_rules! impl_container_forwards {
+    ($typ:ident) => {
+        /// This packet implements the container interface.
+        ///
+        /// Container packets can contain other packets, unprocessed
+        /// data, or both.
+        impl $typ {
+            /// Returns a reference to the container.
+            pub(crate) fn container_ref(&self) -> &packet::Container {
+                &self.container
+            }
+
+            /// Returns a mutable reference to the container.
+            pub(crate) fn container_mut(&mut self) -> &mut packet::Container {
+                &mut self.container
+            }
+
+            /// Gets a reference to the this packet's body.
+            pub fn body(&self) -> Option<&[u8]> {
+                self.container.body()
+            }
+
+            /// Gets a mutable reference to the this packet's body.
+            pub fn body_mut(&mut self) -> Option<&mut Vec<u8>> {
+                self.container.body_mut()
+            }
+
+            /// Sets the this packet's body.
+            pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
+                self.container.set_body(data)
+            }
+
+            /// Returns an iterator over the packet's immediate children.
+            pub fn children<'a>(&'a self) -> impl Iterator<Item = &'a Packet> {
+                self.container.children()
+            }
+
+            /// Returns an iterator over all of the packet's descendants, in
+            /// depth-first order.
+            pub fn descendants(&self) -> super::Iter {
+                self.container.descendants()
+            }
+        }
+    };
+}
diff --git a/openpgp/src/packet/mod.rs b/openpgp/src/packet/mod.rs
index b0bd705f..ca29c2a8 100644
--- a/openpgp/src/packet/mod.rs
+++ b/openpgp/src/packet/mod.rs
@@ -13,6 +13,8 @@ use crate::Error;
 use crate::Result;
 use crate::Packet;
 
+#[macro_use]
+mod container;
 pub mod prelude;
 
 use crate::crypto::KeyPair;
@@ -105,38 +107,45 @@ impl<'a> DerefMut for Packet {
 /// Fields used by multiple packet types.
 #[derive(Debug, PartialEq, Eq, Hash, Clone)]
 pub struct Common {
-    /// XXX: Should only be embedded by the container types.
-    container: Container,
 }
 
 impl Default for Common {
     fn default() -> Common {
         Common {
-            container: Default::default(),
         }
     }
 }
 
-impl Common {
+impl Packet {
     pub(crate) // for packet_pile.rs
-    fn children_ref(&self) -> Option<&Container> {
-        self.container.as_ref()
+    fn container_ref(&self) -> Option<&Container> {
+        match self {
+            Packet::CompressedData(p) => Some(p.container_ref()),
+            Packet::SEIP(p) => Some(p.container_ref()),
+            Packet::AED(p) => Some(p.container_ref()),
+            _ => None,
+        }
     }
 
     pub(crate) // for packet_pile.rs
-    fn children_mut(&mut self) -> Option<&mut Container> {
-        Some(&mut self.container)
+    fn container_mut(&mut self) -> Option<&mut Container> {
+        match self {
+            Packet::CompressedData(p) => Some(p.container_mut()),
+            Packet::SEIP(p) => Some(p.container_mut()),
+            Packet::AED(p) => Some(p.container_mut()),
+            _ => None,
+        }
     }
 
     /// Returns an iterator over the packet's immediate children.
     pub fn children<'a>(&'a self) -> impl Iterator<Item = &'a Packet> {
-        self.container.children()
+        self.container_ref().map(|c| c.children()).unwrap_or_else(|| [].iter())
     }
 
     /// Returns an iterator over all of the packet's descendants, in
     /// depth-first order.
     pub fn descendants(&self) -> Iter {
-        self.container.descendants()
+        self.container_ref().map(|c| c.descendants()).unwrap_or_default()
     }
 
     /// Retrieves the packet's body.
@@ -145,22 +154,19 @@ impl Common {
     /// maximum recursion level is reached while parsing a sequence of
     /// packets, the container's body is stored as is.
     pub fn body(&self) -> Option<&[u8]> {
-        self.container.body()
+        self.container_ref().and_then(|c| c.body())
     }
 
+    #[deprecated]
     /// Sets the packet's body.
     ///
     /// Setting the body clears the old body, or any of the packet's
     /// descendants.
     pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
-        self.container.set_body(data)
-    }
-
-    pub(crate) // For parse.rs
-    fn body_mut(&mut self) -> Option<&mut Vec<u8>> {
-        self.container.body_mut()
+        self.container_mut().unwrap().set_body(data)
     }
 }
+
 
 /// Holds zero or more OpenPGP packets.
 ///
@@ -258,12 +264,14 @@ impl fmt::Debug for Container {
 }
 
 impl Container {
-    pub(crate) fn as_ref(&self) -> Option<&Self> {
-        if ! self.packets.is_empty() {
-            Some(self)
-        } else {
-            None
-        }
+    /// Returns a reference to this Packet's children.
+    pub fn children_ref(&self) -> &[Packet] {
+        &self.packets
+    }
+
+    /// Returns a mutable reference to this Packet's children.
+    pub fn children_mut(&mut self) -> &mut Vec<Packet> {
+        &mut self.packets
     }
 
     // Adds a new packet to the container.
@@ -343,7 +351,7 @@ impl Container {
         for (i, p) in self.packets.iter().enumerate() {
             eprintln!("{}{}: {:?}",
                       Self::indent(indent), i + 1, p);
-            if let Some(ref children) = self.packets[i].children_ref() {
+            if let Some(ref children) = self.packets[i].container_ref() {
                 children.pretty_print(indent + 1);
             }
         }
@@ -366,6 +374,17 @@ pub struct Iter<'a> {
     depth: usize,
 }
 
+impl<'a> Default for Iter<'a> {
+    fn default() -> Self {
+        Iter {
+            children: [].iter(),
+            child: None,
+            grandchildren: None,
+            depth: 0,
+        }
+    }
+}
+
 impl<'a> Iterator for Iter<'a> {
     type Item = &'a Packet;
 
@@ -469,7 +488,7 @@ fn packet_path_iter() {
             v.push(i);
             lpaths.push(v);
 
-            if let Some(ref container) = packet.children_ref() {
+            if let Some(ref container) = packet.container_ref() {
                 for mut path in paths(container.children()).into_iter() {
                     path.insert(0, i);
                     lpaths.push(path);
diff --git a/openpgp/src/packet/seip.rs b/openpgp/src/packet/seip.rs
index 12c24efa..8d132385 100644
--- a/openpgp/src/packet/seip.rs
+++ b/openpgp/src/packet/seip.rs
@@ -19,6 +19,9 @@ use crate::Packet;
 pub struct SEIP1 {
     /// CTB packet header fields.
     pub(crate) common: packet::Common,
+
+    /// This is a container packet.
+    container: packet::Container,
 }
 
 impl SEIP1 {
@@ -26,10 +29,13 @@ impl SEIP1 {
     pub fn new() -> Self {
         Self {
             common: Default::default(),
+            container: Default::default(),
         }
     }
 }
 
+impl_container_forwards!(SEIP1);
+
 impl From<SEIP1> for super::SEIP {
     fn from(p: SEIP1) -> Self {
         super::SEIP::V1(p)
@@ -64,9 +70,7 @@ mod tests {
 
     #[test]
     fn deref() {
-        let mut s = SEIP1 {
-            common: Default::default(),
-        };
+        let mut s = SEIP1::new();
         assert_eq!(s.body(), None);
         s.set_body(vec![0, 1, 2]);
         assert_eq!(s.body(), Some(&[0, 1, 2][..]));
diff --git a/openpgp/src/packet/unknown.rs b/openpgp/src/packet/unknown.rs
index 1d2d8c96..ed718e1b 100644
--- a/openpgp/src/packet/unknown.rs
+++ b/openpgp/src/packet/unknown.rs
@@ -67,6 +67,7 @@ impl Clone for Unknown {
             common: self.common.clone(),
             tag: self.tag,
             error: failure::err_msg(format!("{}", self.error)),
+            body: self.body.clone(),
         }
     }
 }
@@ -79,24 +80,10 @@ impl Unknown {
             common: Default::default(),
             tag: tag,
             error: error,
+            body: Vec::with_capacity(0),
         }
     }
 
-    /// Gets a reference to the unknown packet's body.
-    pub fn body(&self) -> &[u8] {
-        &self.body
-    }
-
-    /// Gets a mutable reference to the unknown packet's body.
-    pub fn body_mut(&mut self) -> &mut Vec<u8> {
-        &mut self.body
-    }
-
-    /// Sets the unknown packet's body.
-    pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
-        std::mem::replace(&mut self.body, data)
-    }
-
     /// Gets the unknown packet's tag.
     pub fn tag(&self) -> Tag {
         self.tag
@@ -126,8 +113,13 @@ impl Unknown {
     /// This is the raw packet content not include the CTB and length
     /// information, and not encoded using something like OpenPGP's
     /// partial body encoding.
-    pub fn body(&self) -> Option<&[u8]> {
-        self.common.body()
+    pub fn body(&self) -> &[u8] {
+        &self.body
+    }
+
+    /// Gets a mutable reference to the unknown packet's body.
+    pub fn body_mut(&mut self) -> &mut Vec<u8> {
+        &mut self.body
     }
 
     /// Sets the packet's contents.
@@ -136,7 +128,7 @@ impl Unknown {
     /// information, and not encoded using something like OpenPGP's
     /// partial body encoding.
     pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
-        self.common.set_body(data)
+        std::mem::replace(&mut self.body, data)
     }
 }
 
diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs
index 5b548de1..c3f95625 100644
--- a/openpgp/src/packet_pile.rs
+++ b/openpgp/src/packet_pile.rs
@@ -126,7 +126,7 @@ impl PacketPile {
                 if *i < c.packets.len() {
                     let p = &c.packets[*i];
                     packet = Some(p);
-                    cont = p.children_ref();
+                    cont = p.container_ref();
                     continue;
                 }
             }
@@ -156,7 +156,7 @@ impl PacketPile {
                 return Some(p)
             }
 
-            container = p.children_mut().unwrap();
+            container = p.container_mut().unwrap();
         }
 
         None
@@ -243,20 +243,10 @@ impl PacketPile {
                 return Err(Error::IndexOutOfRange.into());
             }
 
-            let p = &mut tmp.packets[i];
-            if p.children_ref().is_none() {
-                match p {
-                    Packet::CompressedData(_)
-                        | Packet::SEIP(_)
-                        | Packet::AED(_)
-                        => (), // Ok.
-                    _ => {
-                        return Err(Error::IndexOutOfRange.into());
-                    }
-                }
-            }
-
-            container = p.children_mut().unwrap();
+            container = match tmp.packets[i].container_mut() {
+                Some(c) => c,
+                None => return Err(Error::IndexOutOfRange.into()),
+            };
         }
 
         return Err(Error::IndexOutOfRange.into());
@@ -331,7 +321,7 @@ impl PacketPile {
                 let packets_len = tmp.packets.len();
                 let p = &mut tmp.packets[packets_len - 1];
 
-                container = p.children_mut().unwrap();
+                container = p.container_mut().unwrap();
             }
 
             if relative_position < 0 {
@@ -346,8 +336,7 @@ impl PacketPile {
                     // Create a new container.
                     let tmp = container;
                     let i = tmp.packets.len() - 1;
-                    assert!(tmp.packets[i].children_ref().is_none());
-                    container = tmp.packets[i].children_mut().unwrap();
+                    container = tmp.packets[i].container_mut().unwrap();
                 }
 
                 container.packets.push(packet);
@@ -638,7 +627,7 @@ mod test {
         }
 
         let mut seip = SEIP1::new();
-        seip.children_mut().unwrap().push(cd.into());
+        seip.container_mut().push(cd.into());
         packets.push(seip.into());
 
         eprintln!("{:#?}", packets);
diff --git a/openpgp/src/parse/packet_pile_parser.rs b/openpgp/src/parse/packet_pile_parser.rs
index 223b212b..8c6512a2 100644
--- a/openpgp/src/parse/packet_pile_parser.rs
+++ b/openpgp/src/parse/packet_pile_parser.rs
@@ -138,7 +138,7 @@ impl<'a> PacketPileParser<'a> {
                         "Internal inconsistency while building message.");
             }
 
-            container = p.children_mut().unwrap();
+            container = p.container_mut().unwrap();
         }
 
         container.packets.push(packet);
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index 3a3686a8..085b1108 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -3624,17 +3624,45 @@ impl <'a> PacketParser<'a> {
 
                 Ok(p.body())
             },
-            p => {
+            Packet::CompressedData(p) => {
+                if rest.len() > 0 {
+                    if p.body().map(|b| b.len()).unwrap_or(0) > 0 {
+                        p.body_mut().unwrap().append(&mut rest);
+                    } else {
+                        p.set_body(rest);
+                    }
+                }
+
+                Ok(p.body().unwrap_or(&b""[..]))
+            },
+            Packet::SEIP(p) => {
                 if rest.len() > 0 {
-                    if let Some(body) = p.body_mut() {
-                        body.append(&mut rest);
+                    if p.body().map(|b| b.len()).unwrap_or(0) > 0 {
+                        p.body_mut().unwrap().append(&mut rest);
                     } else {
                         p.set_body(rest);
                     }
                 }
 
-                if let Some(body) = p.body() {
-                    Ok(&body[..])
+                Ok(p.body().unwrap_or(&b""[..]))
+            },
+            Packet::AED(p) => {
+                if rest.len() > 0 {
+                    if p.body().map(|b| b.len()).unwrap_or(0) > 0 {
+                        p.body_mut().unwrap().append(&mut rest);
+                    } else {
+                        p.set_body(rest);
+                    }
+                }
+
+                Ok(p.body().unwrap_or(&b""[..]))
+            },
+            p => {
+                if rest.len() > 0 {
+                    Err(Error::MalformedPacket(
+                        format!("Unexpected body data for {:?}: {}",
+                                p, crate::fmt::hex::encode_pretty(rest)))
+                        .into())
                 } else {
                     Ok(&b""[..])
                 }