From 6686adba044b5ece57a3285a14f93df01493c888 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Tue, 25 Apr 2023 08:37:10 -0700 Subject: [PATCH] fix panic with corrupt webp file There were a couple of layers of issue here: * In the ImageDataType::decode method, we didn't detect and do something reasonable when the decoded image had 0 frames, later leading to a panic in glyphcache when trying to index frame 0 of an empty vec. * We shouldn't have been using ImageDataType::decode for window background images * Make the fallback/placeholder black rather than fully transparent in the more modern decoder thread routine that we use for image decoding at the gui layer. refs: #3614 --- docs/changelog.md | 1 + termwiz/src/image.rs | 27 +++++++++++++++++++++++- wezterm-gui/src/glyphcache.rs | 23 +++++++++++++------- wezterm-gui/src/termwindow/background.rs | 2 +- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index da4dfd28e..d1bd32aaf 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -77,6 +77,7 @@ As features stabilize some brief notes about them will accumulate here. The about dialog has been replaced with a menu item that you can click to copy the version number. #3507 #3585 * Synthesized italics were double-skewed. Thanks to @rozbb! #3613 #3555 +* Panic when using corrupt/invalid webp images as window background #3614 #### Updated * Bundled harfbuzz to 7.1.0 diff --git a/termwiz/src/image.rs b/termwiz/src/image.rs index b37d2a423..4d7f8ebbf 100644 --- a/termwiz/src/image.rs +++ b/termwiz/src/image.rs @@ -284,6 +284,16 @@ impl ImageDataType { } } + /// Black pixels + pub fn placeholder() -> Self { + let mut data = vec![]; + let size = 8; + for _ in 0..size * size { + data.extend_from_slice(&[0, 0, 0, 0xff]); + } + ImageDataType::new_single_frame(size, size, data) + } + pub fn hash_bytes(bytes: &[u8]) -> [u8; 32] { use sha2::Digest; let mut hasher = sha2::Sha256::new(); @@ -380,7 +390,14 @@ impl ImageDataType { match format { ImageFormat::Gif => image::codecs::gif::GifDecoder::new(&*data) .and_then(|decoder| decoder.into_frames().collect_frames()) - .and_then(|frames| Ok(Self::decode_frames(frames))) + .and_then(|frames| { + if frames.is_empty() { + log::error!("decoded image has 0 frames, using placeholder"); + Ok(Self::placeholder()) + } else { + Ok(Self::decode_frames(frames)) + } + }) .unwrap_or_else(|err| { log::error!( "Unable to parse animated gif: {:#}, trying as single frame", @@ -395,6 +412,10 @@ impl ImageDataType { }; if decoder.is_apng() { match decoder.apng().into_frames().collect_frames() { + Ok(frames) if frames.is_empty() => { + log::error!("decoded image has 0 frames, using placeholder"); + Self::placeholder() + } Ok(frames) => Self::decode_frames(frames), _ => Self::EncodedFile(data), } @@ -408,6 +429,10 @@ impl ImageDataType { _ => return Self::EncodedFile(data), }; match decoder.into_frames().collect_frames() { + Ok(frames) if frames.is_empty() => { + log::error!("decoded image has 0 frames, using placeholder"); + Self::placeholder() + } Ok(frames) => Self::decode_frames(frames), _ => Self::EncodedFile(data), } diff --git a/wezterm-gui/src/glyphcache.rs b/wezterm-gui/src/glyphcache.rs index ce4b30006..8650ca65d 100644 --- a/wezterm-gui/src/glyphcache.rs +++ b/wezterm-gui/src/glyphcache.rs @@ -317,7 +317,8 @@ impl FrameDecoder { .next() .ok_or_else(|| { anyhow::anyhow!( - "Image format is not fully supported by \ + "Unable to decode image data. Either it is corrupt, or \ + the Image format is not fully supported by \ https://github.com/image-rs/image/blob/master/README.md#supported-image-formats") })?; let frame = frame.context("first frame result")?; @@ -386,15 +387,22 @@ struct FrameState { impl FrameState { fn new(rx: Receiver) -> Self { - static EMPTY: Lazy = Lazy::new(|| BlobManager::store(&[0u8; 4]).unwrap()); + const BLACK_SIZE: usize = 8; + static BLACK: Lazy = Lazy::new(|| { + let mut data = vec![]; + for _ in 0..BLACK_SIZE * BLACK_SIZE { + data.extend_from_slice(&[0, 0, 0, 0xff]); + } + BlobManager::store(&data).unwrap() + }); Self { source: FrameSource::Decoder(rx), frames: vec![], current_frame: DecodedFrame { - lease: EMPTY.clone(), - width: 1, - height: 1, + lease: BLACK.clone(), + width: BLACK_SIZE, + height: BLACK_SIZE, duration: Duration::from_millis(0), }, } @@ -412,7 +420,7 @@ impl FrameState { Err(TryRecvError::Disconnected) => { self.source = FrameSource::FrameIndex(0); if self.frames.is_empty() { - log::warn!("decoder thread terminated"); + log::warn!("image decoder thread terminated"); self.current_frame.duration = Duration::from_secs(86400); self.frames.push(self.current_frame.clone()); false @@ -463,8 +471,7 @@ pub struct DecodedImage { impl DecodedImage { fn placeholder() -> Self { - // A single black pixel - let image = ImageData::with_data(ImageDataType::new_single_frame(1, 1, vec![0, 0, 0, 0])); + let image = ImageData::with_data(ImageDataType::placeholder()); Self { frame_start: RefCell::new(Instant::now()), current_frame: RefCell::new(0), diff --git a/wezterm-gui/src/termwindow/background.rs b/wezterm-gui/src/termwindow/background.rs index 23798045e..21d35cd1b 100644 --- a/wezterm-gui/src/termwindow/background.rs +++ b/wezterm-gui/src/termwindow/background.rs @@ -206,7 +206,7 @@ impl CachedImage { let data = std::fs::read(path) .with_context(|| format!("Failed to load window_background_image {}", path))?; log::trace!("loaded {}", path); - let mut data = ImageDataType::EncodedFile(data).decode(); + let mut data = ImageDataType::EncodedFile(data); data.adjust_speed(speed); let image = Arc::new(ImageData::with_data(data));