buffered-reader: Fix returning partial reads ending in errors.

  - Make sure that we return the data we already have in our buffer,
    even though we encountered an IO error while filling it.

  - Notably, the packet parser assumes that data once read can be
    requested through the buffered reader protocol again and again.
    Unfortunately, that was not the case, leading to a panic.

  - As the generic reader is used to implement the buffered reader
    protocol on top of io::Read, this problem affects among other
    things the compression container.  Demonstrate this using test.

  - Fixes #1005.

2 files changed, 59 insertions, 6 deletions

diff --git a/buffered-reader/src/generic.rs b/buffered-reader/src/generic.rs
index 1c283c40..721375f1 100644
--- a/buffered-reader/src/generic.rs
+++ b/buffered-reader/src/generic.rs
@@ -119,12 +119,6 @@ impl<T: io::Read + Send + Sync, C: fmt::Debug + Sync + Send> Generic<T, C> {
            self.cursor,
            self.buffer.as_ref().map(|buffer| buffer.len()));
 
-        // See if there is an error from the last invocation.
-        if let Some(e) = self.error.take() {
-            t!("Returning stashed error: {}", e);
-            return Err(e);
-        }
-
         if let Some(ref buffer) = self.buffer {
             // We have a buffer.  Make sure `cursor` is sane.
             assert!(self.cursor <= buffer.len());
@@ -160,6 +154,12 @@ impl<T: io::Read + Send + Sync, C: fmt::Debug + Sync + Send> Generic<T, C> {
                     break;
                 }
 
+                // See if there is an error from the last invocation.
+                if let Some(e) = &self.error {
+                    t!("We have a stashed error, don't poll again: {}", e);
+                    break;
+                }
+
                 match self.reader.read(&mut buffer_new
                                        [amount_buffered + amount_read..]) {
                     Ok(read) => {
@@ -404,4 +404,29 @@ mod test {
             }
         }
     }
+
+    /// Tests that we can request some data using data_hard even if a
+    /// previous request for more data failed.
+    #[test]
+    fn data_hard_after_failure() -> io::Result<()> {
+        /// Returns one byte once, then errors.
+        #[derive(Default)]
+        struct BuggySource(bool);
+        impl io::Read for BuggySource {
+            fn read(&mut self, _: &mut [u8]) -> io::Result<usize> {
+                if self.0 {
+                    Err(io::Error::new(io::ErrorKind::Other, "oops"))
+                } else {
+                    self.0 = true;
+                    Ok(1)
+                }
+            }
+        }
+
+        let mut br = Generic::new(BuggySource::default(), None);
+        assert!(br.data(2).is_ok()); // Ok...
+        assert_eq!(br.data(2).unwrap().len(), 1); // ... but short.
+        assert!(br.data_hard(1).is_ok()); // Should be fine then.
+        Ok(())
+    }
 }
diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs
index d8b4fd07..d414cbe4 100644
--- a/openpgp/src/parse.rs
+++ b/openpgp/src/parse.rs
@@ -6463,4 +6463,32 @@ mod test {
 
         Ok(())
     }
+
+    /// Tests for a panic in the packet parser.
+    fn parse_message(message: &str) {
+        eprintln!("parsing {:?}", message);
+        let mut ppr = match PacketParser::from_bytes(message) {
+            Ok(ppr) => ppr,
+            Err(_) => return,
+        };
+        while let PacketParserResult::Some(pp) = ppr {
+            dbg!(&pp.packet);
+            if let Ok((_, tmp)) = pp.recurse() {
+                ppr = tmp;
+            } else {
+                break;
+            }
+        }
+    }
+
+    /// Tests issue 1005.
+    #[test]
+    fn panic_on_short_zip() {
+        parse_message("-----BEGIN PGP SIGNATURE-----
+
+owGjAA0=
+zXvj
+-----END PGP SIGNATURE-----
+");
+    }
 }