From 046cbc7837cfae8505cb77cdc0352eeaac023a85 Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Thu, 19 Dec 2019 14:15:21 +0100 Subject: openpgp: Move fields from Common to Container. --- openpgp/src/message/mod.rs | 2 - openpgp/src/packet/compressed_data.rs | 15 +-- openpgp/src/packet/mod.rs | 201 +++++++++++++++++--------------- openpgp/src/packet/unknown.rs | 10 +- openpgp/src/packet_pile.rs | 15 +-- openpgp/src/parse/packet_pile_parser.rs | 12 +- 6 files changed, 124 insertions(+), 131 deletions(-) (limited to 'openpgp/src') diff --git a/openpgp/src/message/mod.rs b/openpgp/src/message/mod.rs index 51c548e0..df38cf12 100644 --- a/openpgp/src/message/mod.rs +++ b/openpgp/src/message/mod.rs @@ -454,7 +454,6 @@ mod tests { use crate::crypto::mpis::{Ciphertext, MPI}; use crate::packet::prelude::*; use crate::KeyID; - use crate::Container; #[test] fn tokens() { @@ -998,7 +997,6 @@ mod tests { // 1: MDC // => good. let mut seip = SEIP1::new(); - seip.set_children(Some(Container::new())); seip.children_mut().unwrap().push( lit.clone().into()); seip.children_mut().unwrap().push( diff --git a/openpgp/src/packet/compressed_data.rs b/openpgp/src/packet/compressed_data.rs index db166ed3..b6914fe9 100644 --- a/openpgp/src/packet/compressed_data.rs +++ b/openpgp/src/packet/compressed_data.rs @@ -3,7 +3,6 @@ use std::ops::{Deref, DerefMut}; use crate::packet::{self, Common}; use crate::Packet; -use crate::Container; use crate::types::CompressionAlgorithm; /// Holds a compressed data packet. @@ -29,10 +28,10 @@ impl fmt::Debug for CompressedData { f.debug_struct("CompressedData") .field("algo", &self.algo) .field("children", - &self.common.children.as_ref() + &self.common.children_ref() .map(|c| &c.packets).unwrap_or(&Vec::new())) .field("body (bytes)", - &self.common.body.as_ref().unwrap_or(&b"".to_vec()).len()) + &self.common.body().unwrap_or(&b"".to_vec()).len()) .finish() } } @@ -58,10 +57,7 @@ impl CompressedData { /// Adds a new packet to the container. pub fn push(mut self, packet: Packet) -> Self { - if self.common.children.is_none() { - self.common.children = Some(Container::new()); - } - self.common.children.as_mut().unwrap().push(packet); + self.common.children_mut().unwrap().packets.push(packet); self } @@ -70,10 +66,7 @@ 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 { - if self.common.children.is_none() { - self.common.children = Some(Container::new()); - } - self.common.children.as_mut().unwrap().insert(i, packet); + self.common.children_mut().unwrap().packets.insert(i, packet); self } } diff --git a/openpgp/src/packet/mod.rs b/openpgp/src/packet/mod.rs index 4cee893d..b0bd705f 100644 --- a/openpgp/src/packet/mod.rs +++ b/openpgp/src/packet/mod.rs @@ -103,8 +103,71 @@ impl<'a> DerefMut for Packet { } /// Fields used by multiple packet types. -#[derive(PartialEq, Eq, Hash, Clone)] +#[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 { + pub(crate) // for packet_pile.rs + fn children_ref(&self) -> Option<&Container> { + self.container.as_ref() + } + + pub(crate) // for packet_pile.rs + fn children_mut(&mut self) -> Option<&mut Container> { + Some(&mut self.container) + } + + /// Returns an iterator over the packet's immediate children. + pub fn children<'a>(&'a self) -> impl Iterator { + self.container.children() + } + + /// Returns an iterator over all of the packet's descendants, in + /// depth-first order. + pub fn descendants(&self) -> Iter { + self.container.descendants() + } + + /// Retrieves the packet's body. + /// + /// Packets can store a sequence of bytes as body, e.g. if the + /// 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() + } + + /// 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) -> Vec { + self.container.set_body(data) + } + + pub(crate) // For parse.rs + fn body_mut(&mut self) -> Option<&mut Vec> { + self.container.body_mut() + } +} + +/// Holds zero or more OpenPGP packets. +/// +/// This is used by OpenPGP container packets, like the compressed +/// data packet, to store the containing packets. +#[derive(PartialEq, Eq, Hash, Clone)] +pub(crate) struct Container { /// Used by container packets (such as the encryption and /// compression packets) to reference their immediate children. /// This results in a tree structure. @@ -118,7 +181,7 @@ pub struct Common { /// [`PacketPile`]: ../struct.PacketPile.html /// [`PacketPile::from_file`]: ../struct.PacketPile.html#method.from_file /// [`PacketParser`]: ../parse/struct.PacketParser.html - children: Option, + pub(crate) packets: Vec, /// Holds a packet's body. /// @@ -168,99 +231,22 @@ pub struct Common { body: Option>, } -impl fmt::Debug for Common { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Common") - .field("children", &self.children) - .field("body (bytes)", - &self.body.as_ref().map(|body| body.len())) - .finish() - } -} - -impl Default for Common { - fn default() -> Common { - Common { - children: None, +impl Default for Container { + fn default() -> Self { + Self { + packets: Vec::with_capacity(0), body: None, } } } -impl Common { - pub(crate) // for packet_pile.rs - fn children_ref(&self) -> Option<&Container> { - self.children.as_ref() - } - - pub(crate) // for packet_pile.rs - fn children_mut(&mut self) -> Option<&mut Container> { - self.children.as_mut() - } - - pub(crate) // for packet_pile.rs - fn set_children(&mut self, v: Option) -> Option { - std::mem::replace(&mut self.children, v) - } - - fn children_iter<'a>(&'a self) -> slice::Iter<'a, Packet> { - if let Some(ref container) = self.children { - container.packets.iter() - } else { - let empty_packet_slice : &[Packet] = &[]; - empty_packet_slice.iter() - } - } - - /// Returns an iterator over the packet's immediate children. - pub fn children<'a>(&'a self) -> impl Iterator { - self.children_iter() - } - - /// Returns an iterator over all of the packet's descendants, in - /// depth-first order. - pub fn descendants(&self) -> Iter { - return Iter { - children: self.children_iter(), - child: None, - grandchildren: None, - depth: 0, +impl From> for Container { + fn from(packets: Vec) -> Self { + Self { + packets, + body: None, } } - - /// Retrieves the packet's body. - /// - /// Packets can store a sequence of bytes as body, e.g. if the - /// 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.body.as_ref().map(|b| b.as_slice()) - } - - /// 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) -> Vec { - self.children = None; - ::std::mem::replace(&mut self.body, - if data.len() == 0 { None } else { Some(data) }) - .unwrap_or(Vec::new()) - } - - pub(crate) // For parse.rs - fn body_mut(&mut self) -> Option<&mut Vec> { - self.body.as_mut() - } -} - -/// Holds zero or more OpenPGP packets. -/// -/// This is used by OpenPGP container packets, like the compressed -/// data packet, to store the containing packets. -#[derive(PartialEq, Eq, Hash, Clone)] -pub(crate) struct Container { - pub(crate) packets: Vec, } impl fmt::Debug for Container { @@ -272,8 +258,12 @@ impl fmt::Debug for Container { } impl Container { - pub(crate) fn new() -> Container { - Container { packets: Vec::with_capacity(8) } + pub(crate) fn as_ref(&self) -> Option<&Self> { + if ! self.packets.is_empty() { + Some(self) + } else { + None + } } // Adds a new packet to the container. @@ -311,6 +301,31 @@ impl Container { self.packets.into_iter() } + /// Retrieves the packet's body. + /// + /// Packets can store a sequence of bytes as body, e.g. if the + /// 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.body.as_ref().map(|b| b.as_slice()) + } + + /// 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) -> Vec { + self.packets.clear(); + ::std::mem::replace(&mut self.body, + if data.len() == 0 { None } else { Some(data) }) + .unwrap_or(Vec::new()) + } + + pub(crate) // For parse.rs + fn body_mut(&mut self) -> Option<&mut Vec> { + self.body.as_mut() + } + // Converts an indentation level to whitespace. fn indent(depth: usize) -> &'static str { use std::cmp; @@ -328,7 +343,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 { + if let Some(ref children) = self.packets[i].children_ref() { children.pretty_print(indent + 1); } } @@ -454,8 +469,8 @@ fn packet_path_iter() { v.push(i); lpaths.push(v); - if let Some(ref children) = packet.children { - for mut path in paths(children.packets.iter()).into_iter() { + if let Some(ref container) = packet.children_ref() { + for mut path in paths(container.children()).into_iter() { path.insert(0, i); lpaths.push(path); } diff --git a/openpgp/src/packet/unknown.rs b/openpgp/src/packet/unknown.rs index 07cd5e0e..d9bfbfd0 100644 --- a/openpgp/src/packet/unknown.rs +++ b/openpgp/src/packet/unknown.rs @@ -41,10 +41,6 @@ impl PartialOrd for Unknown impl Ord for Unknown { fn cmp(&self, other: &Unknown) -> Ordering { - // An unknown packet cannot have children. - assert!(self.common.children.is_none()); - assert!(other.common.children.is_none()); - match self.tag.cmp(&other.tag) { Ordering::Equal => self.common.body().cmp(&other.common.body()), o => o, @@ -110,7 +106,7 @@ impl Unknown { /// information, and not encoded using something like OpenPGP's /// partial body encoding. pub fn body(&self) -> Option<&[u8]> { - self.common.body.as_ref().map(|b| b.as_slice()) + self.common.body() } /// Sets the packet's contents. @@ -118,8 +114,8 @@ 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 set_body(&mut self, data: Vec) -> Option> { - ::std::mem::replace(&mut self.common.body, Some(data)) + pub fn set_body(&mut self, data: Vec) -> Vec { + self.common.set_body(data) } } diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs index a290f841..5b548de1 100644 --- a/openpgp/src/packet_pile.rs +++ b/openpgp/src/packet_pile.rs @@ -74,7 +74,7 @@ impl std::str::FromStr for PacketPile { impl From> for PacketPile { fn from(p: Vec) -> Self { - PacketPile { top_level: Container { packets: p } } + PacketPile { top_level: Container::from(p) } } } @@ -246,11 +246,10 @@ impl PacketPile { let p = &mut tmp.packets[i]; if p.children_ref().is_none() { match p { - Packet::CompressedData(_) | Packet::SEIP(_) => { - // We have a container with no children. - // That's okay. We can create the container. - p.set_children(Some(Container::new())); - }, + Packet::CompressedData(_) + | Packet::SEIP(_) + | Packet::AED(_) + => (), // Ok. _ => { return Err(Error::IndexOutOfRange.into()); } @@ -304,7 +303,7 @@ impl PacketPile { } // Create a top-level container. - let mut top_level = Container::new(); + let mut top_level = Container::default(); let mut last_position = 0; @@ -348,7 +347,6 @@ impl PacketPile { let tmp = container; let i = tmp.packets.len() - 1; assert!(tmp.packets[i].children_ref().is_none()); - tmp.packets[i].set_children(Some(Container::new())); container = tmp.packets[i].children_mut().unwrap(); } @@ -640,7 +638,6 @@ mod test { } let mut seip = SEIP1::new(); - seip.set_children(Some(Container::new())); seip.children_mut().unwrap().push(cd.into()); packets.push(seip.into()); diff --git a/openpgp/src/parse/packet_pile_parser.rs b/openpgp/src/parse/packet_pile_parser.rs index 58a93718..223b212b 100644 --- a/openpgp/src/parse/packet_pile_parser.rs +++ b/openpgp/src/parse/packet_pile_parser.rs @@ -4,7 +4,6 @@ use std::path::Path; use crate::{ Result, Packet, - Container, PacketPile, }; use crate::parse::{ @@ -109,7 +108,7 @@ impl<'a> PacketPileParser<'a> { -> Result> { Ok(PacketPileParser { - pile: PacketPile { top_level: Container::new() }, + pile: PacketPile { top_level: Default::default() }, ppr: ppr, returned_first: false, }) @@ -135,13 +134,8 @@ impl<'a> PacketPileParser<'a> { let packets_len = tmp.packets.len(); let p = &mut tmp.packets[packets_len - 1]; if p.children().next().is_none() { - if i == position - 1 { - // This is the leaf. Create a new container - // here. - p.set_children(Some(Container::new())); - } else { - panic!("Internal inconsistency while building message."); - } + assert!(i == position - 1, + "Internal inconsistency while building message."); } container = p.children_mut().unwrap(); -- cgit v1.2.3