openpgp: Make hash_update_text more idomatic.

  - Make `hash_update_text` a method on `HashingMode<Digest>`,
    `HashingMode<Digest>::update`.

3 files changed, 88 insertions, 75 deletions

diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs
index a34a880b..85c372f7 100644
--- a/openpgp/src/parse.rs
+++ b/openpgp/src/parse.rs
@@ -253,7 +253,6 @@ mod hashed_reader;
 pub(crate) use self::hashed_reader::{
     HashingMode,
     HashedReader,
-    hash_update_text,
 };
 
 mod packet_parser_builder;
diff --git a/openpgp/src/parse/hashed_reader.rs b/openpgp/src/parse/hashed_reader.rs
index 1a02f079..3ac13b2b 100644
--- a/openpgp/src/parse/hashed_reader.rs
+++ b/openpgp/src/parse/hashed_reader.rs
@@ -6,8 +6,9 @@ use std::fmt;
 use buffered_reader::BufferedReader;
 use buffered_reader::buffered_reader_generic_read_impl;
 
-use crate::Result;
+use crate::crypto::hash::Digest;
 use crate::parse::{Cookie, HashesFor, Hashing};
+use crate::Result;
 use crate::types::HashAlgorithm;
 use crate::types::SignatureType;
 
@@ -83,6 +84,53 @@ impl<T> HashingMode<T> {
             HashingMode::Binary(t)
         }
     }
+
+    pub(crate) fn into_inner(self) -> T {
+        use self::HashingMode::*;
+        match self {
+            Binary(t) => t,
+            Text(t) => t,
+        }
+    }
+}
+
+impl<D> HashingMode<D>
+    where D: Digest
+{
+    /// Updates the given hash context.  When in text mode, normalize
+    /// the line endings to "\r\n" on the fly.
+    pub(crate) fn update(&mut self, data: &[u8]) {
+        match self {
+            HashingMode::Text(h) => {
+                let mut line = data;
+                while ! line.is_empty() {
+                    let mut next = 0;
+                    for (i, c) in line.iter().cloned().enumerate() {
+                        match c {
+                            b'\r' | b'\n' => {
+                                h.update(&line[..i]);
+                                h.update(b"\r\n");
+                                next = i + 1;
+                                if c == b'\r' && line.get(next) == Some(&b'\n') {
+                                    next += 1;
+                                }
+                                break;
+                            },
+                            _ => (),
+                        }
+                    }
+
+                    if next > 0 {
+                        line = &line[next..];
+                    } else {
+                        h.update(line);
+                        break;
+                    }
+                }
+            }
+            HashingMode::Binary(h) => h.update(data),
+        }
+    }
 }
 
 pub(crate) struct HashedReader<R: BufferedReader<Cookie>> {
@@ -126,37 +174,6 @@ impl<R: BufferedReader<Cookie>> HashedReader<R> {
     }
 }
 
-/// Updates the given hash context normalizing line endings to "\r\n"
-/// on the fly.
-pub(crate) fn hash_update_text(h: &mut dyn crate::crypto::hash::Digest,
-                               text: &[u8]) {
-    let mut line = text;
-    while ! line.is_empty() {
-        let mut next = 0;
-        for (i, c) in line.iter().cloned().enumerate() {
-            match c {
-                b'\r' | b'\n' => {
-                    h.update(&line[..i]);
-                    h.update(b"\r\n");
-                    next = i + 1;
-                    if c == b'\r' && line.get(next) == Some(&b'\n') {
-                        next += 1;
-                    }
-                    break;
-                },
-                _ => (),
-            }
-        }
-
-        if next > 0 {
-            line = &line[next..];
-        } else {
-            h.update(line);
-            break;
-        }
-    }
-}
-
 impl Cookie {
     fn hash_update(&mut self, data: &[u8]) {
         let level = self.level.unwrap_or(0);
@@ -181,16 +198,13 @@ impl Cookie {
             // We fix that here by hashing the stashed data into the
             // former topmost signature-group's hash.
             assert!(ngroups > 1);
-            for mode in self.sig_groups[ngroups-2].hashes.iter_mut()
+            for h in self.sig_groups[ngroups-2].hashes.iter_mut()
             {
                 t!("({:?}): group {} {:?} hashing {} stashed bytes.",
-                   hashes_for, ngroups-2, mode.map(|ctx| ctx.algo()),
+                   hashes_for, ngroups-2, h.map(|ctx| ctx.algo()),
                    data.len());
 
-                match mode {
-                    HashingMode::Binary(h) => h.update(&stashed_data),
-                    HashingMode::Text(h) => hash_update_text(h, &stashed_data),
-                }
+                h.update(&stashed_data);
             }
         }
 
@@ -213,13 +227,10 @@ impl Cookie {
                 return;
             }
 
-            for mode in sig_group.hashes.iter_mut() {
+            for h in sig_group.hashes.iter_mut() {
                 t!("{:?}: group {} {:?} hashing {} bytes.",
-                   hashes_for, i, mode.map(|ctx| ctx.algo()), data.len());
-                match mode {
-                    HashingMode::Binary(h) => h.update(data),
-                    HashingMode::Text(h) => hash_update_text(h, data),
-                }
+                   hashes_for, i, h.map(|ctx| ctx.algo()), data.len());
+                h.update(data);
             }
         }
     }
@@ -261,17 +272,13 @@ impl Cookie {
 
         // Hash stashed data first.
         if let Some(stashed_data) = self.hash_stash.take() {
-            for mode in self.sig_groups[0].hashes.iter_mut() {
+            for h in self.sig_groups[0].hashes.iter_mut() {
                 t!("{:?}: {:?} hashing {} stashed bytes.",
-                   hashes_for, mode.map(|ctx| ctx.algo()),
+                   hashes_for, h.map(|ctx| ctx.algo()),
                    stashed_data.len());
-                match mode {
-                    HashingMode::Binary(_) =>
-                        unreachable!("CSF transformation uses \
-                                      text signatures"),
-                    HashingMode::Text(h) =>
-                        hash_update_text(h, &stashed_data[..]),
-                }
+                assert!(matches!(h, HashingMode::Text(_)),
+                        "CSF transformation uses text signatures");
+                h.update(&stashed_data[..]);
             }
         }
 
@@ -291,14 +298,12 @@ impl Cookie {
         };
 
         // Hash everything but the last newline now.
-        for mode in self.sig_groups[0].hashes.iter_mut() {
+        for h in self.sig_groups[0].hashes.iter_mut() {
             t!("{:?}: {:?} hashing {} bytes.",
-               hashes_for, mode.map(|ctx| ctx.algo()), l);
-            match mode {
-                HashingMode::Binary(_) =>
-                    unreachable!("CSF transformation uses text signatures"),
-                HashingMode::Text(h) => hash_update_text(h, &data[..l]),
-            }
+               hashes_for, h.map(|ctx| ctx.algo()), l);
+            assert!(matches!(h, HashingMode::Text(_)),
+                    "CSF transformation uses text signatures");
+            h.update(&data[..l]);
         }
 
         // The newline we stash away.  If more text is written
@@ -522,8 +527,9 @@ mod test {
             "one\rtwo\rthree",
             "one\ntwo\r\nthree",
         ] {
-            let mut ctx = HashAlgorithm::SHA256.context()?;
-            super::hash_update_text(&mut ctx, text.as_bytes());
+            let mut ctx = HashingMode::Text(HashAlgorithm::SHA256.context()?);
+            ctx.update(text.as_bytes());
+            let mut ctx = ctx.into_inner();
             let mut digest = vec![0; ctx.digest_size()];
             let _ = ctx.digest(&mut digest);
             assert_eq!(
diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs
index 6c7d0958..cdadcb30 100644
--- a/openpgp/src/serialize/stream.rs
+++ b/openpgp/src/serialize/stream.rs
@@ -135,6 +135,7 @@ use crate::{
 };
 use crate::packet::header::CTB;
 use crate::packet::header::BodyLength;
+use crate::parse::HashingMode;
 use super::{
     Marshal,
 };
@@ -636,7 +637,7 @@ pub struct Signer<'a> {
     mode: SignatureMode,
     template: signature::SignatureBuilder,
     creation_time: Option<SystemTime>,
-    hash: Box<dyn crypto::hash::Digest>,
+    hash: HashingMode<Box<dyn crypto::hash::Digest>>,
     cookie: Cookie,
     position: u64,
 
@@ -813,7 +814,8 @@ impl<'a> Signer<'a> {
             mode: SignatureMode::Inline,
             template: template.into(),
             creation_time: None,
-            hash: HashAlgorithm::default().context().unwrap(),
+            hash: HashingMode::Binary(
+                HashAlgorithm::default().context().unwrap()),
             cookie: Cookie {
                 level,
                 private: Private::Signer,
@@ -1147,7 +1149,7 @@ impl<'a> Signer<'a> {
     /// # Ok(()) }
     /// ```
     pub fn hash_algo(mut self, algo: HashAlgorithm) -> Result<Self> {
-        self.hash = algo.context()?;
+        self.hash = HashingMode::Binary(algo.context()?);
         Ok(self)
     }
 
@@ -1265,7 +1267,14 @@ impl<'a> Signer<'a> {
         }
 
         if let Some(hash) = acceptable_hashes.first() {
-            self.hash = hash.context().unwrap();
+            let ctx = hash.context().unwrap();
+            self.hash = if self.template.typ() == SignatureType::Text
+                || self.mode == SignatureMode::Cleartext
+            {
+                HashingMode::Text(ctx)
+            } else {
+                HashingMode::Binary(ctx)
+            };
         } else {
             return Err(Error::NoAcceptableHash.into());
         }
@@ -1278,7 +1287,7 @@ impl<'a> Signer<'a> {
                     let key = keypair.public();
                     let mut ops = OnePassSig3::new(self.template.typ());
                     ops.set_pk_algo(key.pk_algo());
-                    ops.set_hash_algo(self.hash.algo());
+                    ops.set_hash_algo(self.hash.as_ref().algo());
                     ops.set_issuer(key.keyid());
                     ops.set_last(i == self.signers.len() - 1);
                     Packet::OnePassSig(ops.into())
@@ -1293,7 +1302,8 @@ impl<'a> Signer<'a> {
                 // Write the header.
                 let mut sink = self.inner.take().unwrap();
                 writeln!(sink, "-----BEGIN PGP SIGNED MESSAGE-----")?;
-                writeln!(sink, "Hash: {}", self.hash.algo().text_name()?)?;
+                writeln!(sink, "Hash: {}",
+                         self.hash.as_ref().algo().text_name()?)?;
                 writeln!(sink)?;
 
                 // We now install two filters.  See the comment on
@@ -1351,7 +1361,8 @@ impl<'a> Signer<'a> {
                 }
 
                 // Compute the signature.
-                let sig = sig.sign_hash(signer.as_mut(), hash)?;
+                let sig = sig.sign_hash(signer.as_mut(),
+                                        hash.into_inner())?;
 
                 // And emit the packet.
                 Packet::Signature(sig).serialize(sink)?;
@@ -1409,8 +1420,7 @@ impl<'a> Write for Signer<'a> {
                 // a newline, but we know that more text follows (buf
                 // is not empty), so it cannot be the last.
                 assert!(! buf.is_empty());
-                crate::parse::hash_update_text(&mut self.hash,
-                                               &self.hash_stash[..]);
+                self.hash.update(&self.hash_stash[..]);
                 crate::vec_truncate(&mut self.hash_stash, 0);
 
                 // Compute the length of data that should be hashed.
@@ -1428,13 +1438,11 @@ impl<'a> Write for Signer<'a> {
                 // b"\r\n" in one write.
 
                 // Hash everything but the last newline now.
-                crate::parse::hash_update_text(&mut self.hash, &data[..l]);
+                self.hash.update(&data[..l]);
                 // The newline we stash away.  If more text is written
                 // later, we will hash it then.  Otherwise, it is
                 // implicitly omitted when the signer is finalized.
                 self.hash_stash.extend_from_slice(&data[l..]);
-            } else if self.template.typ() == SignatureType::Text {
-                crate::parse::hash_update_text(&mut self.hash, data);
             } else {
                 self.hash.update(data);
             }