openpgp: Make {is,possible}_{message,keyring,tpk} return a Result

  - PacketParserEOF::is_message, PacketParserEOF::is_keyring,
    PacketParserEOF::is_tpk, PacketParserResult::possible_message,
    PacketParserResult::possible_keyring, and
    PacketParserResult::possible_tpk returned a boolean.

  - Change them to return a Result<()> instead, which is more Rusty,
    and, in particular, allows the caller to determine why the message
    didn't parse.

9 files changed, 98 insertions, 58 deletions

diff --git a/ffi/lang/python/sequoia/openpgp.py b/ffi/lang/python/sequoia/openpgp.py
index ddd76dbe..1c19d80a 100644
--- a/ffi/lang/python/sequoia/openpgp.py
+++ b/ffi/lang/python/sequoia/openpgp.py
@@ -280,7 +280,7 @@ class PacketParserEOF(SQObject):
     _del = lib.pgp_packet_parser_eof_free
 
     def is_message(self):
-        return bool(lib.pgp_packet_parser_eof_is_message(self.ref()))
+        return invoke(lib.pgp_packet_parser_eof_is_message, self.ref())
 
 class PacketParser(SQObject):
     _del = lib.pgp_packet_parser_free
diff --git a/openpgp-ffi/include/sequoia/openpgp.h b/openpgp-ffi/include/sequoia/openpgp.h
index b6fac2a6..1e9b7ef6 100644
--- a/openpgp-ffi/include/sequoia/openpgp.h
+++ b/openpgp-ffi/include/sequoia/openpgp.h
@@ -1195,8 +1195,12 @@ void pgp_packet_parser_free (pgp_packet_parser_t pp);
 
 /*/
 /// Returns whether the message is a well-formed OpenPGP message.
+///
+/// If the message is not well-formed, `*errp` explains why this is
+/// not the case.
 /*/
-bool pgp_packet_parser_eof_is_message(pgp_packet_parser_eof_t eof);
+bool pgp_packet_parser_eof_is_message(pgp_error_t *errp,
+                                      pgp_packet_parser_eof_t eof);
 
 /*/
 /// Frees the packet parser EOF object.
diff --git a/openpgp-ffi/src/parse/mod.rs b/openpgp-ffi/src/parse/mod.rs
index 02fc4b27..df13bcbd 100644
--- a/openpgp-ffi/src/parse/mod.rs
+++ b/openpgp-ffi/src/parse/mod.rs
@@ -92,11 +92,13 @@ pub extern "C" fn pgp_packet_parser_free(pp: Option<&mut PacketParser>) {
 /// Frees the packet parser EOF object.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
 pub extern "C" fn pgp_packet_parser_eof_is_message(
+    errp: Option<&mut *mut ::error::Error>,
     eof: *const PacketParserEOF) -> bool
 {
+    ffi_make_fry_from_errp!(errp);
     let eof = ffi_param_ref!(eof);
-
-    eof.is_message()
+    ffi_try_or!(eof.is_message(), false);
+    true
 }
 
 /// Frees the packet parser EOF object.
diff --git a/openpgp/examples/notarize.rs b/openpgp/examples/notarize.rs
index b46fbe1a..44ebb0f1 100644
--- a/openpgp/examples/notarize.rs
+++ b/openpgp/examples/notarize.rs
@@ -76,8 +76,8 @@ fn main() {
         .expect("Failed to build parser");
 
     while let PacketParserResult::Some(mut pp) = ppr {
-        if ! pp.possible_message() {
-            panic!("Malformed OpenPGP message");
+        if let Err(err) = pp.possible_message() {
+            panic!("Malformed OpenPGP message: {}", err);
         }
 
         match pp.packet {
@@ -108,8 +108,8 @@ fn main() {
         ppr = pp.recurse().expect("Failed to recurse").1;
     }
     if let PacketParserResult::EOF(eof) = ppr {
-        if ! eof.is_message() {
-            panic!("Malformed OpenPGP message")
+        if let Err(err) = eof.is_message() {
+            panic!("Malformed OpenPGP message: {}", err)
         }
     } else {
         unreachable!()
diff --git a/openpgp/src/autocrypt.rs b/openpgp/src/autocrypt.rs
index d99b5852..53545bd5 100644
--- a/openpgp/src/autocrypt.rs
+++ b/openpgp/src/autocrypt.rs
@@ -752,9 +752,9 @@ impl<'a> AutocryptSetupMessageParser<'a> {
                 // If we've gotten this far, then the outer message
                 // has the right sequence of packets, but we haven't
                 // carefully checked the nesting.  We do that now.
-                if ! pp.is_message() {
-                    return Err(Error::MalformedMessage(
-                        "Invalid OpenPGP Message".into()).into());
+                if let Err(err) = pp.is_message() {
+                    return Err(err.context(
+                        "Invalid OpenPGP Message").into());
                 }
             }
             PacketParserResult::Some(pp) =>
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index 7d8d2213..a706e0c5 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -2596,22 +2596,40 @@ impl PacketParserEOF {
     /// Whether the message is an OpenPGP Message.
     ///
     /// As opposed to a TPK or just a bunch of packets.
-    pub fn is_message(&self) -> bool {
-        self.state.message_validator.is_message()
+    pub fn is_message(&self) -> Result<()> {
+        use message::MessageValidity;
+
+        match self.state.message_validator.check() {
+            MessageValidity::Message => Ok(()),
+            MessageValidity::MessagePrefix => unreachable!(),
+            MessageValidity::Error(err) => Err(err),
+        }
     }
 
     /// Whether the message is an OpenPGP keyring.
     ///
     /// As opposed to a Message or just a bunch of packets.
-    pub fn is_keyring(&self) -> bool {
-        self.state.keyring_validator.is_keyring()
+    pub fn is_keyring(&self) -> Result<()> {
+        use tpk::KeyringValidity;
+
+        match self.state.keyring_validator.check() {
+            KeyringValidity::Keyring => Ok(()),
+            KeyringValidity::KeyringPrefix => unreachable!(),
+            KeyringValidity::Error(err) => Err(err),
+        }
     }
 
     /// Whether the message is an OpenPGP TPK.
     ///
     /// As opposed to a Message or just a bunch of packets.
-    pub fn is_tpk(&self) -> bool {
-        self.state.tpk_validator.is_tpk()
+    pub fn is_tpk(&self) -> Result<()> {
+        use tpk::TPKValidity;
+
+        match self.state.tpk_validator.check() {
+            TPKValidity::TPK => Ok(()),
+            TPKValidity::TPKPrefix => unreachable!(),
+            TPKValidity::Error(err) => Err(err),
+        }
     }
 
     /// Returns the path of the last packet.
@@ -2866,8 +2884,14 @@ impl <'a> PacketParser<'a> {
     /// to say whether the message is definitely an OpenPGP Message.
     /// Before that, it is only possible to say that the message is a
     /// valid prefix or definitely not an OpenPGP message.
-    pub fn possible_message(&self) -> bool {
-        self.state.message_validator.check().is_message_prefix()
+    pub fn possible_message(&self) -> Result<()> {
+        use message::MessageValidity;
+
+        match self.state.message_validator.check() {
+            MessageValidity::Message => unreachable!(),
+            MessageValidity::MessagePrefix => Ok(()),
+            MessageValidity::Error(err) => Err(err),
+        }
     }
 
     /// Returns whether the message appears to be an OpenPGP keyring.
@@ -2876,8 +2900,14 @@ impl <'a> PacketParser<'a> {
     /// to say whether the message is definitely an OpenPGP keyring.
     /// Before that, it is only possible to say that the message is a
     /// valid prefix or definitely not an OpenPGP keyring.
-    pub fn possible_keyring(&self) -> bool {
-        self.state.keyring_validator.check().is_keyring_prefix()
+    pub fn possible_keyring(&self) -> Result<()> {
+        use tpk::KeyringValidity;
+
+        match self.state.keyring_validator.check() {
+            KeyringValidity::Keyring => unreachable!(),
+            KeyringValidity::KeyringPrefix => Ok(()),
+            KeyringValidity::Error(err) => Err(err),
+        }
     }
 
     /// Returns whether the message appears to be an OpenPGP TPK.
@@ -2886,8 +2916,14 @@ impl <'a> PacketParser<'a> {
     /// to say whether the message is definitely an OpenPGP TPK.
     /// Before that, it is only possible to say that the message is a
     /// valid prefix or definitely not an OpenPGP TPK.
-    pub fn possible_tpk(&self) -> bool {
-        self.state.tpk_validator.check().is_tpk_prefix()
+    pub fn possible_tpk(&self) -> Result<()> {
+        use tpk::TPKValidity;
+
+        match self.state.tpk_validator.check() {
+            TPKValidity::TPK => unreachable!(),
+            TPKValidity::TPKPrefix => Ok(()),
+            TPKValidity::Error(err) => Err(err),
+        }
     }
 
     /// Returns Ok if the data appears to be a legal packet.
@@ -4108,7 +4144,7 @@ mod test {
             // Make sure we actually decrypted...
             let mut saw_literal = false;
             while let PacketParserResult::Some(mut pp) = ppr {
-                assert!(pp.possible_message());
+                assert!(pp.possible_message().is_ok());
 
                 match pp.packet {
                     Packet::SEIP(_) | Packet::AED(_) => {
@@ -4127,7 +4163,7 @@ mod test {
             }
             assert!(saw_literal);
             if let PacketParserResult::EOF(eof) = ppr {
-                assert!(eof.is_message());
+                assert!(eof.is_message().is_ok());
             } else {
                 unreachable!();
             }
@@ -4149,12 +4185,12 @@ mod test {
                 .expect(&format!("Error reading {:?}", path));
 
             while let PacketParserResult::Some(mut pp) = ppr {
-                assert!(pp.possible_keyring());
+                assert!(pp.possible_keyring().is_ok());
                 ppr = pp.recurse().unwrap().1;
             }
             if let PacketParserResult::EOF(eof) = ppr {
-                assert!(eof.is_keyring());
-                assert!(! eof.is_tpk());
+                assert!(eof.is_keyring().is_ok());
+                assert!(eof.is_tpk().is_err());
             } else {
                 unreachable!();
             }
@@ -4174,13 +4210,13 @@ mod test {
                 .expect(&format!("Error reading {:?}", path));
 
             while let PacketParserResult::Some(mut pp) = ppr {
-                assert!(pp.possible_keyring());
-                assert!(pp.possible_tpk());
+                assert!(pp.possible_keyring().is_ok());
+                assert!(pp.possible_tpk().is_ok());
                 ppr = pp.recurse().unwrap().1;
             }
             if let PacketParserResult::EOF(eof) = ppr {
-                assert!(eof.is_keyring());
-                assert!(eof.is_tpk());
+                assert!(eof.is_keyring().is_ok());
+                assert!(eof.is_tpk().is_ok());
             } else {
                 unreachable!();
             }
@@ -4199,7 +4235,7 @@ mod test {
 
             let mut saw_literal = false;
             while let PacketParserResult::Some(mut pp) = ppr {
-                assert!(pp.possible_message());
+                assert!(pp.possible_message().is_ok());
 
                 match pp.packet {
                     Packet::Literal(_) => {
@@ -4214,7 +4250,7 @@ mod test {
             assert!(! saw_literal);
             if let PacketParserResult::EOF(eof) = ppr {
                 eprintln!("eof: {:?}", eof);
-                assert!(eof.is_message());
+                assert!(eof.is_message().is_ok());
             } else {
                 unreachable!();
             }
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index d9791479..957a75bf 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -482,9 +482,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> {
         let mut issuers = Vec::new();
 
         while let PacketParserResult::Some(pp) = ppr {
-            if ! pp.possible_message() {
-                return Err(Error::MalformedMessage(
-                    "Malformed OpenPGP message".into()).into());
+            if let Err(err) = pp.possible_message() {
+                return Err(err.context("Malformed OpenPGP message").into());
             }
 
             match pp.packet {
@@ -586,9 +585,9 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> {
                 // Process the rest of the packets.
                 let mut ppr = PacketParserResult::Some(pp);
                 while let PacketParserResult::Some(mut pp) = ppr {
-                    if ! pp.possible_message() {
-                        return Err(Error::MalformedMessage(
-                            "Malformed OpenPGP message".into()).into());
+                    if let Err(err) = pp.possible_message() {
+                        return Err(err.context(
+                            "Malformed OpenPGP message").into());
                     }
 
                     let (p, ppr_tmp) = pp.recurse()?;
@@ -1232,10 +1231,9 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
 
         while let PacketParserResult::Some(mut pp) = ppr {
             v.helper.inspect(&pp)?;
-            if ! pp.possible_message() {
-                t!("Malformed message");
-                return Err(Error::MalformedMessage(
-                    "Malformed OpenPGP message".into()).into());
+            if let Err(err) = pp.possible_message() {
+                t!("Malformed message: {}", err);
+                return Err(err.context("Malformed OpenPGP message").into());
             }
 
             match pp.packet {
@@ -1388,9 +1386,9 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                         self.helper.inspect(&pp)?;
                     }
 
-                    if ! pp.possible_message() {
-                        return Err(Error::MalformedMessage(
-                            "Malformed OpenPGP message".into()).into());
+                    if let Err(err) = pp.possible_message() {
+                        return Err(err.context(
+                            "Malformed OpenPGP message").into());
                     }
 
                     match pp.packet {
diff --git a/tool/src/commands/inspect.rs b/tool/src/commands/inspect.rs
index 0ba21dd9..84e66520 100644
--- a/tool/src/commands/inspect.rs
+++ b/tool/src/commands/inspect.rs
@@ -31,7 +31,9 @@ pub fn inspect(m: &clap::ArgMatches, output: &mut io::Write)
     while let PacketParserResult::Some(mut pp) = ppr {
         match pp.packet {
             Packet::PublicKey(_) | Packet::SecretKey(_) => {
-                if ! pp.possible_tpk() && pp.possible_keyring() {
+                if pp.possible_tpk().is_err()
+                    && pp.possible_keyring().is_ok()
+                {
                     if ! type_called {
                         writeln!(output, "OpenPGP Keyring.")?;
                         writeln!(output)?;
@@ -53,7 +55,7 @@ pub fn inspect(m: &clap::ArgMatches, output: &mut io::Write)
             _ => (),
         }
 
-        let possible_keyring = pp.possible_keyring();
+        let possible_keyring = pp.possible_keyring().is_ok();
         let (packet, ppr_) = pp.recurse()?;
         ppr = ppr_;
 
@@ -70,7 +72,7 @@ pub fn inspect(m: &clap::ArgMatches, output: &mut io::Write)
     }
 
     if let PacketParserResult::EOF(eof) = ppr {
-        if eof.is_message() {
+        if eof.is_message().is_ok() {
             writeln!(output, "{}OpenPGP Message.",
                      match (encrypted, ! sigs.is_empty()) {
                          (false, false) => "",
@@ -92,7 +94,7 @@ pub fn inspect(m: &clap::ArgMatches, output: &mut io::Write)
                          if literal_prefix.len() == 40 { "..." } else { "" })?;
             }
 
-        } else if eof.is_tpk() || eof.is_keyring() {
+        } else if eof.is_tpk().is_ok() || eof.is_keyring().is_ok() {
             let pp = openpgp::PacketPile::from(packets);
             let tpk = openpgp::TPK::from_packet_pile(pp)?;
             inspect_tpk(output, &tpk, print_keygrips, print_certifications)?;
diff --git a/tool/src/commands/sign.rs b/tool/src/commands/sign.rs
index bcead074..f527f3a7 100644
--- a/tool/src/commands/sign.rs
+++ b/tool/src/commands/sign.rs
@@ -8,7 +8,7 @@ extern crate sequoia_openpgp as openpgp;
 use openpgp::armor;
 use openpgp::constants::DataFormat;
 use openpgp::crypto;
-use openpgp::{Packet, Error, Result};
+use openpgp::{Packet, Result};
 use openpgp::packet::Signature;
 use openpgp::parse::{
     Parse,
@@ -179,9 +179,8 @@ fn sign_message(input: &mut io::Read, output_path: Option<&str>,
         };
 
     while let PacketParserResult::Some(mut pp) = ppr {
-        if ! pp.possible_message() {
-            return Err(Error::MalformedMessage(
-                "Malformed OpenPGP message".into()).into());
+        if let Err(err) = pp.possible_message() {
+            return Err(err.context("Malformed OpenPGP message").into());
         }
 
         match pp.packet {
@@ -301,9 +300,8 @@ fn sign_message(input: &mut io::Read, output_path: Option<&str>,
     }
 
     if let PacketParserResult::EOF(eof) = ppr {
-        if ! eof.is_message() {
-            return Err(Error::MalformedMessage(
-                "Malformed OpenPGP message".into()).into());
+        if let Err(err) = eof.is_message() {
+            return Err(err.context("Malformed OpenPGP message").into());
         }
     } else {
         unreachable!()