Fix reading pascal string (#42)

src/psd_channel.rs

@@ -149,14 +149,18 @@ pub trait IntoRgba {
 
         let mut idx = 0;
         let offset = channel_kind.rgba_offset().unwrap();
+        let len = cursor.get_ref().len() as u64;
 
-        while cursor.position() != cursor.get_ref().len() as u64 {
+        while cursor.position() < len {
             let header = cursor.read_i8() as i16;
 
             if header == -128 {
                 continue;
             } else if header >= 0 {
                 let bytes_to_read = 1 + header;
+                if cursor.position() + bytes_to_read as u64 > len {
+                    break;
+                }
                 for byte in cursor.read(bytes_to_read as u32) {
                     let rgba_idx = self.rgba_idx(idx);
                     rgba[rgba_idx * 4 + offset] = *byte;
@@ -165,6 +169,9 @@ pub trait IntoRgba {
                 }
             } else {
                 let repeat = 1 - header;
+                if cursor.position() + 1 > len {
+                    break;
+                }
                 let byte = cursor.read_1()[0];
                 for _ in 0..repeat as usize {
                     let rgba_idx = self.rgba_idx(idx);
@@ -321,3 +328,47 @@ impl PsdChannelKind {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::sections::layer_and_mask_information_section::layer::{
+        BlendMode, LayerChannels, LayerProperties,
+    };
+    use crate::PsdLayer;
+
+    use super::*;
+
+    /// Verify that when inserting an RLE channel's bytes into an RGBA byte vec we do not attempt to
+    /// read beyond the channel's length.
+    #[test]
+    fn does_not_read_beyond_rle_channels_bytes() {
+        let layer_properties = LayerProperties {
+            name: "".into(),
+            layer_top: 0,
+            layer_left: 0,
+            layer_bottom: 0,
+            layer_right: 0,
+            visible: true,
+            opacity: 0,
+            clipping_mask: false,
+            psd_width: 1,
+            psd_height: 1,
+            blend_mode: BlendMode::Normal,
+            group_id: None,
+        };
+
+        let layer = PsdLayer {
+            channels: LayerChannels::from([(
+                PsdChannelKind::Red,
+                ChannelBytes::RleCompressed(vec![0, 0, 0]),
+            )]),
+            layer_properties,
+        };
+
+        let mut rgba = vec![0; (layer.width() * layer.height() * 4) as usize];
+
+        layer.insert_channel_bytes(&mut rgba, PsdChannelKind::Red, layer.red());
+
+        assert_eq!(rgba, vec![0; 4]);
+    }
+}

src/sections/image_resources_section.rs

@@ -541,7 +541,7 @@ impl DescriptorStructure {
 
         for n in 0..count {
             let key = DescriptorStructure::read_key_length(cursor);
-            let key = String::from_utf8(key.to_vec()).unwrap();
+            let key = String::from_utf8_lossy(key).into_owned();
 
             m.insert(key, DescriptorStructure::read_descriptor_field(cursor)?);
         }

src/sections/layer_and_mask_information_section/layer.rs

@@ -17,7 +17,7 @@ use crate::sections::image_data_section::ChannelBytes;
 #[derive(Debug, Clone)]
 pub struct LayerProperties {
     /// The name of this layer
-    pub(super) name: String,
+    pub(crate) name: String,
     /// The position of the top of the layer
     pub(crate) layer_top: i32,
     /// The position of the left of the layer
@@ -188,7 +188,7 @@ pub struct PsdLayer {
     /// channel, or you might make use of the layer masks.
     ///
     /// Storing the channels separately allows for this flexability.
-    pub(super) channels: LayerChannels,
+    pub(in crate) channels: LayerChannels,
     /// Common layer properties
     pub(in crate) layer_properties: LayerProperties,
 }

src/sections/layer_and_mask_information_section/mod.rs

@@ -85,7 +85,14 @@ impl LayerAndMaskInformationSection {
         // the exact number of bytes in the layer and information mask section of the PSD file,
         // so there's no way for us to accidentally read too many bytes. If we did the program
         // would panic.
-        cursor.read_4();
+        let len = cursor.read_u32();
+
+        if len == 0 {
+            return Ok(LayerAndMaskInformationSection {
+                layers: Layers::new(),
+                groups: Groups::with_capacity(0),
+            });
+        }
 
         // Read the next four bytes to get the length of the layer info section.
         let _layer_info_section_len = cursor.read_u32();

src/sections/mod.rs

@@ -300,10 +300,13 @@ impl<'a> PsdCursor<'a> {
     pub fn read_pascal_string(&mut self) -> String {
         let len = self.read_u8();
         let data = self.read(len as u32);
-        let result = String::from_utf8(data.to_vec()).unwrap();
+        let result = String::from_utf8_lossy(data).into_owned();
+
+        if len % 2 == 0 {
+            // If the total length is odd, read an extra null byte
+            self.read_u8();
+        }
 
-        // read null byte
-        self.read_u8();
         result
     }
 }

tests/image_resources_section.rs

@@ -61,3 +61,25 @@ fn image_check_16x16p_bound_field() {
         panic!("expected descriptor");
     }
 }
+
+/// The image contains a non-UTF-8 Pascal string of even length in its image resource block.
+///
+/// cargo test --test image_resources_section image_non_utf8_pascal_string -- --exact
+#[test]
+fn image_non_utf8_pascal_string() {
+    let psd = include_bytes!("./fixtures/non-utf8-pascal-string.psd");
+    let psd = Psd::from_bytes(psd).unwrap();
+
+    assert!(psd.layers().is_empty());
+}
+
+/// The image contains a Pascal string of odd length in its image resource block.
+///
+/// cargo test --test image_resources_section image_odd_length_pascal_string -- --exact
+#[test]
+fn image_odd_length_pascal_string() {
+    let psd = include_bytes!("./fixtures/odd-length-pascal-string.psd");
+    let psd = Psd::from_bytes(psd).unwrap();
+
+    assert!(psd.layers().is_empty());
+}