Repository: serde-rs/json (language: rust)

Issue:
StreamDeserializer enters a busy-loop for malformed input
See the test in

https://github.com/oli-obk/cargo_metadata/pull/106/commits/ea84dc353dc4367c507eec0f4fd1f73892cf4d28

There, the `for message in stream_deserilizer` loop loops infinitelly, because serde repeteadly tries to parse the same invalid portion of input. 

I *think* the correct behavior here is to "poison" the Iterator after the first error and return `None`. I can also imagine some error-recovey strategy (like skipping to the next utf8 codepoint), but poisoning seems more reliable to me. 


Discussion / hints:

Fix patch:
diff --git a/src/de.rs b/src/de.rs
index 1a7192672..890a51687 100644
--- a/src/de.rs
+++ b/src/de.rs
@@ -151,6 +151,7 @@ impl<'de, R: Read<'de>> Deserializer<R> {
         StreamDeserializer {
             de: self,
             offset: offset,
+            failed: false,
             output: PhantomData,
             lifetime: PhantomData,
         }
@@ -2046,6 +2047,7 @@ where
 pub struct StreamDeserializer<'de, R, T> {
     de: Deserializer<R>,
     offset: usize,
+    failed: bool,
     output: PhantomData<T>,
     lifetime: PhantomData<&'de ()>,
 }
@@ -2068,6 +2070,7 @@ where
         StreamDeserializer {
             de: Deserializer::new(read),
             offset: offset,
+            failed: false,
             output: PhantomData,
             lifetime: PhantomData,
         }
@@ -2132,6 +2135,10 @@ where
     type Item = Result<T>;
 
     fn next(&mut self) -> Option<Result<T>> {
+        if R::should_early_return_if_failed && self.failed {
+            return None;
+        }
+
         // skip whitespaces, if any
         // this helps with trailing whitespaces, since whitespaces between
         // values are handled for us.
@@ -2160,10 +2167,16 @@ where
                             self.peek_end_of_value().map(|_| value)
                         }
                     }
-                    Err(e) => Err(e),
+                    Err(e) => {
+                        self.de.read.set_failed(&mut self.failed);
+                        Err(e)
+                    }
                 })
             }
-            Err(e) => Some(Err(e)),
+            Err(e) => {
+                self.de.read.set_failed(&mut self.failed);
+                Some(Err(e))
+            }
         }
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
index 8114f3a3a..a276b3887 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -337,6 +337,7 @@
     clippy::missing_errors_doc,
     clippy::must_use_candidate,
 )]
+#![allow(non_upper_case_globals)]
 #![deny(missing_docs)]
 #![cfg_attr(not(feature = "std"), no_std)]
 
diff --git a/src/read.rs b/src/read.rs
index 89ed6e6c7..01d30682c 100644
--- a/src/read.rs
+++ b/src/read.rs
@@ -97,6 +97,18 @@ pub trait Read<'de>: private::Sealed {
     fn end_raw_buffering<V>(&mut self, visitor: V) -> Result<V::Value>
     where
         V: Visitor<'de>;
+
+    /// Whether StreamDeserializer::next needs to check the failed flag. True
+    /// for IoRead, false for StrRead and SliceRead which can track failure by
+    /// truncating their input slice to avoid the extra check on every next
+    /// call.
+    #[doc(hidden)]
+    const should_early_return_if_failed: bool;
+
+    /// Mark a persistent failure of StreamDeserializer, either by setting the
+    /// flag or by truncating the input data.
+    #[doc(hidden)]
+    fn set_failed(&mut self, failed: &mut bool);
 }
 
 pub struct Position {
@@ -379,6 +391,13 @@ where
             raw_value: Some(raw),
         })
     }
+
+    const should_early_return_if_failed: bool = true;
+
+    #[inline]
+    fn set_failed(&mut self, failed: &mut bool) {
+        *failed = true;
+    }
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -590,6 +609,13 @@ impl<'a> Read<'a> for SliceRead<'a> {
             raw_value: Some(raw),
         })
     }
+
+    const should_early_return_if_failed: bool = false;
+
+    #[inline]
+    fn set_failed(&mut self, _failed: &mut bool) {
+        self.slice = &self.slice[..self.index];
+    }
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -681,6 +707,13 @@ impl<'a> Read<'a> for StrRead<'a> {
             raw_value: Some(raw),
         })
     }
+
+    const should_early_return_if_failed: bool = false;
+
+    #[inline]
+    fn set_failed(&mut self, failed: &mut bool) {
+        self.delegate.set_failed(failed);
+    }
 }
 
 //////////////////////////////////////////////////////////////////////////////


Test patch:
diff --git a/tests/stream.rs b/tests/stream.rs
index 9fabe0de2..ca54e9a15 100644
--- a/tests/stream.rs
+++ b/tests/stream.rs
@@ -169,3 +169,14 @@ fn test_json_stream_invalid_number() {
         assert_eq!(second.to_string(), "trailing characters at line 1 column 2");
     });
 }
+
+#[test]
+fn test_error() {
+    let data = "true wrong false";
+
+    test_stream!(data, Value, |stream| {
+        assert_eq!(stream.next().unwrap().unwrap(), true);
+        assert!(stream.next().unwrap().is_err());
+        assert!(stream.next().is_none());
+    });
+}