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