1From 1ec761327e59f712374068e63a44f705b9162e0f Mon Sep 17 00:00:00 2001
2From: Lukasz Anforowicz <[email protected]>
3Date: Fri, 4 Oct 2024 22:51:09 +0000
4Subject: [PATCH 106/113] Avoid infinite loop when retrying after earlier fatal
5 error.
6
7When `StreamingDecoder` reports an error, it leaves `state` set to
8`None`.  Before this commit, calling `next_frame` in this state would
9have led to an infinite loop:
10
11* `ReadDecoder::decode_next` would loop forever (`!self.at_eof` is true
12  after an error) and would fail to make progress, because
13* When `StreamingDecoder::update` sees `state` saw set to `None` then
14  before this commit it wouldn't enter the `next_state` loop and would
15  immediately return no progress
16  (`Ok((/* consumer bytes = */ 0, Decoded::Nothing))`).
17
18After this commit, `StreamingDecoder::update` checks if the `state` is
19`None` and treats this as an error.
20---
21 src/common.rs         |  7 +++++++
22 src/decoder/stream.rs | 32 +++++++++++++++++++++++++-------
23 2 files changed, 32 insertions(+), 7 deletions(-)
24
25diff --git a/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/common.rs b/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/common.rs
26index 4475153..a3b6c27 100644
27--- a/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/common.rs
28+++ b/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/common.rs
29@@ -791,6 +791,10 @@ pub(crate) enum ParameterErrorKind {
30     /// library will perform the checks necessary to ensure that data was accurate or error with a
31     /// format error otherwise.
32     PolledAfterEndOfImage,
33+    /// Attempt to continue decoding after a fatal, non-resumable error was reported (e.g. after
34+    /// [`DecodingError::Format`]).  The only case when it is possible to resume after an error
35+    /// is an `UnexpectedEof` scenario - see [`DecodingError::IoError`].
36+    PolledAfterFatalError,
37 }
38
39 impl From<ParameterErrorKind> for ParameterError {
40@@ -807,6 +811,9 @@ impl fmt::Display for ParameterError {
41                 write!(fmt, "wrong data size, expected {} got {}", expected, actual)
42             }
43             PolledAfterEndOfImage => write!(fmt, "End of image has been reached"),
44+            PolledAfterFatalError => {
45+                write!(fmt, "A fatal decoding error has been encounted earlier")
46+            }
47         }
48     }
49 }
50diff --git a/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/decoder/stream.rs b/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/decoder/stream.rs
51index f52523a..e8412f3 100644
52--- a/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/decoder/stream.rs
53+++ b/third_party/rust/chromium_crates_io/vendor/png-0.17.14/src/decoder/stream.rs
54@@ -10,7 +10,7 @@ use super::zlib::ZlibStream;
55 use crate::chunk::{self, ChunkType, IDAT, IEND, IHDR};
56 use crate::common::{
57     AnimationControl, BitDepth, BlendOp, ColorType, DisposeOp, FrameControl, Info, ParameterError,
58-    PixelDimensions, ScaledFloat, SourceChromaticities, Unit,
59+    ParameterErrorKind, PixelDimensions, ScaledFloat, SourceChromaticities, Unit,
60 };
61 use crate::text_metadata::{ITXtChunk, TEXtChunk, TextDecodingError, ZTXtChunk};
62 use crate::traits::ReadBytesExt;
63@@ -627,15 +627,24 @@ impl StreamingDecoder {
64         mut buf: &[u8],
65         image_data: &mut Vec<u8>,
66     ) -> Result<(usize, Decoded), DecodingError> {
67+        if self.state.is_none() {
68+            return Err(DecodingError::Parameter(
69+                ParameterErrorKind::PolledAfterFatalError.into(),
70+            ));
71+        }
72+
73         let len = buf.len();
74-        while !buf.is_empty() && self.state.is_some() {
75+        while !buf.is_empty() {
76             match self.next_state(buf, image_data) {
77                 Ok((bytes, Decoded::Nothing)) => buf = &buf[bytes..],
78                 Ok((bytes, result)) => {
79                     buf = &buf[bytes..];
80                     return Ok((len - buf.len(), result));
81                 }
82-                Err(err) => return Err(err),
83+                Err(err) => {
84+                    debug_assert!(self.state.is_none());
85+                    return Err(err);
86+                }
87             }
88         }
89         Ok((len - buf.len(), Decoded::Nothing))
90@@ -1917,8 +1926,18 @@ mod tests {
91         // 0-length fdAT should result in an error.
92         let err = reader.next_frame(&mut buf).unwrap_err();
93         assert!(matches!(&err, DecodingError::Format(_)));
94-        let msg = format!("{err}");
95-        assert_eq!("fdAT chunk shorter than 4 bytes", msg);
96+        assert_eq!("fdAT chunk shorter than 4 bytes", format!("{err}"));
97+
98+        // Calling `next_frame` again should return an error.  Same error as above would be nice,
99+        // but it is probably unnecessary and infeasible (`DecodingError` can't derive `Clone`
100+        // because `std::io::Error` doesn't implement `Clone`)..  But it definitely shouldn't enter
101+        // an infinite loop.
102+        let err2 = reader.next_frame(&mut buf).unwrap_err();
103+        assert!(matches!(&err2, DecodingError::Parameter(_)));
104+        assert_eq!(
105+            "A fatal decoding error has been encounted earlier",
106+            format!("{err2}")
107+        );
108     }
109
110     #[test]
111@@ -1935,8 +1954,7 @@ mod tests {
112         // 3-bytes-long fdAT should result in an error.
113         let err = reader.next_frame(&mut buf).unwrap_err();
114         assert!(matches!(&err, DecodingError::Format(_)));
115-        let msg = format!("{err}");
116-        assert_eq!("fdAT chunk shorter than 4 bytes", msg);
117+        assert_eq!("fdAT chunk shorter than 4 bytes", format!("{err}"));
118     }
119
120     #[test]
121--
1222.47.0.rc0.187.ge670bccf7e-goog
123
124