feedback from colin:

- Move Decompressable and friends to generic_decompress.rs

- Add module docstring

- Add tests to feed garbage into gzip/zstd decompressors and make sure they don't panic on drop

Signed-off-by: John Eckersberg <[email protected]>

Author: jeckersb

ostree-ext/src/container/unencapsulate.rs

@@ -32,6 +32,9 @@
 // which is exactly what is exported by the [`crate::tar::export`] process.
 
 use crate::container::store::LayerProgress;
+use crate::generic_decompress::{
+    Decompressable, GzipDecompressor, TransparentDecompressor, ZstdDecompressor,
+};
 
 use super::*;
 use containers_image_proxy::{ImageProxy, OpenedImage};
@@ -191,57 +194,6 @@ pub async fn unencapsulate(repo: &ostree::Repo, imgref: &OstreeImageReference) -
     importer.unencapsulate().await
 }
 
-trait Decompressable: Read + Send + 'static {
-    fn get_inner_mut(&mut self) -> &mut (dyn Read);
-}
-
-// TransparentDecompressor
-
-struct TransparentDecompressor<R: Read + Send + 'static>(R);
-
-impl<R: Read + Send + 'static> Read for TransparentDecompressor<R> {
-    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
-        self.0.read(buf)
-    }
-}
-
-impl<R: Read + Send + 'static> Decompressable for TransparentDecompressor<R> {
-    fn get_inner_mut(&mut self) -> &mut (dyn Read) {
-        &mut self.0
-    }
-}
-
-// GzipDecompressor
-
-struct GzipDecompressor<R: std::io::BufRead>(flate2::bufread::GzDecoder<R>);
-
-impl<R: std::io::BufRead + Send + 'static> Read for GzipDecompressor<R> {
-    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
-        self.0.read(buf)
-    }
-}
-
-impl<R: std::io::BufRead + Send + 'static> Decompressable for GzipDecompressor<R> {
-    fn get_inner_mut(&mut self) -> &mut (dyn Read) {
-        self.0.get_mut()
-    }
-}
-
-// ZstdDecompressor
-struct ZstdDecompressor<'a, R: std::io::BufRead>(zstd::stream::read::Decoder<'a, R>);
-
-impl<'a: 'static, R: std::io::BufRead + Send + 'static> Read for ZstdDecompressor<'a, R> {
-    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
-        self.0.read(buf)
-    }
-}
-
-impl<'a: 'static, R: std::io::BufRead + Send + 'static> Decompressable for ZstdDecompressor<'a, R> {
-    fn get_inner_mut(&mut self) -> &mut (dyn Read) {
-        self.0.get_mut()
-    }
-}
-
 pub(crate) struct Decompressor {
     inner: Box<dyn Decompressable>,
     finished: bool,
@@ -429,4 +381,26 @@ mod tests {
         let d = Decompressor::new(&oci_image::MediaType::ImageLayerZstd, empty).unwrap();
         drop(d)
     }
+
+    #[test]
+    fn test_gzip_decompressor_with_garbage_input() {
+        let garbage = b"This is not valid gzip data";
+        let mut d = Decompressor::new(&oci_image::MediaType::ImageLayerGzip, &garbage[..]).unwrap();
+        let mut buf = [0u8; 32];
+        let e = d.read(&mut buf).unwrap_err();
+        assert!(matches!(e.kind(), std::io::ErrorKind::InvalidInput));
+        assert_eq!(e.to_string(), "invalid gzip header".to_string());
+        drop(d)
+    }
+
+    #[test]
+    fn test_zstd_decompressor_with_garbage_input() {
+        let garbage = b"This is not valid zstd data";
+        let mut d = Decompressor::new(&oci_image::MediaType::ImageLayerZstd, &garbage[..]).unwrap();
+        let mut buf = [0u8; 32];
+        let e = d.read(&mut buf).unwrap_err();
+        assert!(matches!(e.kind(), std::io::ErrorKind::Other));
+        assert_eq!(e.to_string(), "Unknown frame descriptor".to_string());
+        drop(d)
+    }
 }

ostree-ext/src/generic_decompress.rs

@@ -0,0 +1,66 @@
+//! This module contains the `Decompressable` trait and related
+//! concrete implementations thereof.  These provide a means for each
+//! specific decompressor to give mutable access to the inner reader.
+//!
+//! For example, the GzipDecompressor would give the underlying
+//! compressed stream.
+//!
+//! We need a common way to access this stream so that we can flush
+//! the data during cleanup.
+//!
+//! See: https://github.com/bootc-dev/bootc/issues/1407
+
+use std::io::Read;
+
+/// Extends the `Read` trait with another method to get mutable access to the inner reader
+pub(crate) trait Decompressable: Read + Send + 'static {
+    fn get_inner_mut(&mut self) -> &mut (dyn Read);
+}
+
+// TransparentDecompressor
+
+pub(crate) struct TransparentDecompressor<R: Read + Send + 'static>(pub R);
+
+impl<R: Read + Send + 'static> Read for TransparentDecompressor<R> {
+    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+        self.0.read(buf)
+    }
+}
+
+impl<R: Read + Send + 'static> Decompressable for TransparentDecompressor<R> {
+    fn get_inner_mut(&mut self) -> &mut (dyn Read) {
+        &mut self.0
+    }
+}
+
+// GzipDecompressor
+
+pub(crate) struct GzipDecompressor<R: std::io::BufRead>(pub flate2::bufread::GzDecoder<R>);
+
+impl<R: std::io::BufRead + Send + 'static> Read for GzipDecompressor<R> {
+    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+        self.0.read(buf)
+    }
+}
+
+impl<R: std::io::BufRead + Send + 'static> Decompressable for GzipDecompressor<R> {
+    fn get_inner_mut(&mut self) -> &mut (dyn Read) {
+        self.0.get_mut()
+    }
+}
+
+// ZstdDecompressor
+
+pub(crate) struct ZstdDecompressor<'a, R: std::io::BufRead>(pub zstd::stream::read::Decoder<'a, R>);
+
+impl<'a: 'static, R: std::io::BufRead + Send + 'static> Read for ZstdDecompressor<'a, R> {
+    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+        self.0.read(buf)
+    }
+}
+
+impl<'a: 'static, R: std::io::BufRead + Send + 'static> Decompressable for ZstdDecompressor<'a, R> {
+    fn get_inner_mut(&mut self) -> &mut (dyn Read) {
+        self.0.get_mut()
+    }
+}

ostree-ext/src/lib.rs

@@ -37,6 +37,7 @@ pub mod cli;
 pub mod container;
 pub mod container_utils;
 pub mod diff;
+pub(crate) mod generic_decompress;
 pub mod ima;
 pub mod keyfileext;
 pub(crate) mod logging;