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