der: have Reader::peek* methods take &mut self (#1418)

Right now `PemReader` uses interior mutability via `RefCell`, because
peeking might involve filling an internal peek buffer (i.e. decoding the
Base64 in a PEM document).

Rather than trying to hide the fact we're potentially modifying internal
buffers (but not the cursor/position) when peeking, this changes the
methods to explicitly accept `&mut self`, and with that change
eliminates all usages of `RefCell`.

This change appears to have no practical drawbacks, since peeking always
occurs during the decoding process where we need mutable access to the
reader anyway.

der/src/reader.rs

@@ -23,16 +23,16 @@ pub trait Reader<'r>: Sized {
     /// Get the length of the input.
     fn input_len(&self) -> Length;
 
-    /// Peek at the next byte of input without modifying the cursor.
-    fn peek_byte(&self) -> Option<u8>;
+    /// Peek at the next byte of input without modifying the position within the reader.
+    fn peek_byte(&mut self) -> Option<u8>;
 
     /// Peek forward in the input data, attempting to decode a [`Header`] from
     /// the data at the current position in the decoder.
     ///
-    /// Does not modify the decoder's state.
-    fn peek_header(&self) -> Result<Header, Error>;
+    /// Does not modify the position within the reader.
+    fn peek_header(&mut self) -> Result<Header, Error>;
 
-    /// Get the position within the buffer.
+    /// Get the position within the reader in bytes.
     fn position(&self) -> Length;
 
     /// Attempt to read data borrowed directly from the input as a slice,
@@ -103,8 +103,8 @@ pub trait Reader<'r>: Sized {
     /// Peek at the next byte in the decoder and attempt to decode it as a
     /// [`Tag`] value.
     ///
-    /// Does not modify the decoder's state.
-    fn peek_tag(&self) -> Result<Tag, Error> {
+    /// Does not modify the position within the reader.
+    fn peek_tag(&mut self) -> Result<Tag, Error> {
         match self.peek_byte() {
             Some(byte) => byte.try_into(),
             None => Err(Error::incomplete(self.input_len())),

der/src/reader/nested.rs

@@ -59,15 +59,15 @@ impl<'i, 'r, R: Reader<'r>> Reader<'r> for NestedReader<'i, R> {
         self.input_len
     }
 
-    fn peek_byte(&self) -> Option<u8> {
+    fn peek_byte(&mut self) -> Option<u8> {
         if self.is_finished() {
             None
         } else {
             self.inner.peek_byte()
         }
     }
 
-    fn peek_header(&self) -> Result<Header> {
+    fn peek_header(&mut self) -> Result<Header> {
         if self.is_finished() {
             Err(Error::incomplete(self.offset()))
         } else {

der/src/reader/pem.rs

@@ -2,7 +2,6 @@
 
 use super::Reader;
 use crate::{Decode, EncodingRules, Error, ErrorKind, Header, Length, Result};
-use core::cell::RefCell;
 
 #[allow(clippy::arithmetic_side_effects)]
 mod utils {
@@ -126,7 +125,7 @@ mod utils {
 #[derive(Clone)]
 pub struct PemReader<'i> {
     /// Inner PEM decoder wrapped in a BufReader.
-    reader: RefCell<utils::BufReader<'i>>,
+    reader: utils::BufReader<'i>,
 
     /// Encoding rules to apply when decoding the input.
     encoding_rules: EncodingRules,
@@ -148,7 +147,7 @@ impl<'i> PemReader<'i> {
         let input_len = Length::try_from(reader.remaining_len())?;
 
         Ok(Self {
-            reader: RefCell::new(reader),
+            reader,
             encoding_rules: EncodingRules::default(),
             input_len,
             position: Length::ZERO,
@@ -158,7 +157,7 @@ impl<'i> PemReader<'i> {
     /// Get the PEM label which will be used in the encapsulation boundaries
     /// for this document.
     pub fn type_label(&self) -> &'i str {
-        self.reader.borrow().type_label()
+        self.reader.type_label()
     }
 }
 
@@ -172,15 +171,15 @@ impl<'i> Reader<'i> for PemReader<'i> {
         self.input_len
     }
 
-    fn peek_byte(&self) -> Option<u8> {
+    fn peek_byte(&mut self) -> Option<u8> {
         if self.is_finished() {
             None
         } else {
-            self.reader.borrow().peek_byte()
+            self.reader.peek_byte()
         }
     }
 
-    fn peek_header(&self) -> Result<Header> {
+    fn peek_header(&mut self) -> Result<Header> {
         if self.is_finished() {
             Err(Error::incomplete(self.offset()))
         } else {
@@ -198,13 +197,12 @@ impl<'i> Reader<'i> for PemReader<'i> {
     }
 
     fn read_into<'o>(&mut self, buf: &'o mut [u8]) -> Result<&'o [u8]> {
-        let bytes = self.reader.borrow_mut().copy_to_slice(buf)?;
-
+        let bytes = self.reader.copy_to_slice(buf)?;
         self.position = (self.position + bytes.len())?;
 
         debug_assert_eq!(
             self.position,
-            (self.input_len - Length::try_from(self.reader.borrow().remaining_len())?)?
+            (self.input_len - Length::try_from(self.reader.remaining_len())?)?
         );
 
         Ok(bytes)

der/src/reader/slice.rs

@@ -77,13 +77,13 @@ impl<'a> Reader<'a> for SliceReader<'a> {
         self.bytes.len()
     }
 
-    fn peek_byte(&self) -> Option<u8> {
+    fn peek_byte(&mut self) -> Option<u8> {
         self.remaining()
             .ok()
             .and_then(|bytes| bytes.first().cloned())
     }
 
-    fn peek_header(&self) -> Result<Header, Error> {
+    fn peek_header(&mut self) -> Result<Header, Error> {
         Header::decode(&mut self.clone())
     }
 
@@ -212,15 +212,15 @@ mod tests {
 
     #[test]
     fn peek_tag() {
-        let reader = SliceReader::new(EXAMPLE_MSG).unwrap();
+        let mut reader = SliceReader::new(EXAMPLE_MSG).unwrap();
         assert_eq!(reader.position(), Length::ZERO);
         assert_eq!(reader.peek_tag().unwrap(), Tag::Integer);
         assert_eq!(reader.position(), Length::ZERO); // Position unchanged
     }
 
     #[test]
     fn peek_header() {
-        let reader = SliceReader::new(EXAMPLE_MSG).unwrap();
+        let mut reader = SliceReader::new(EXAMPLE_MSG).unwrap();
         assert_eq!(reader.position(), Length::ZERO);
 
         let header = reader.peek_header().unwrap();