paolobarbolini / img-parts

Low level crate for reading and writing Jpeg, Png and RIFF image containers
https://crates.io/crates/img-parts
Apache License 2.0
25 stars 6 forks source link

Pushing to segments_mut creates unrecoverable segment #12

Open jacg opened 2 months ago

jacg commented 2 months ago

Here is some code illustrating the problem:

use std::env::args;
use img_parts::jpeg::{self, JpegSegment, Jpeg};

const OUR_MARKER: u8 = jpeg::markers::COM;

fn main() {

    // Get path of JPEG from CLI
    let mut args = args();
    let _executable = args.next();
    let path = args.next().unwrap();

    // Read JPEG from file and report number of segments
    let bytes = std::fs::read(path).unwrap();
    let mut jpeg = Jpeg::from_bytes(bytes.into()).unwrap();
    println!("Initial number of segments:                  {}", jpeg.segments().len());

    // Add segment to end, and report number of segments
    let segments = jpeg.segments_mut();
    segments.push(                        // BROKEN
    //segments.insert(segments.len() - 1, // WORKS
        JpegSegment::new_with_contents(
            OUR_MARKER,
            img_parts::Bytes::from("OUR SEGMENT CONTENT")
        )
    );
    println!("Number of segments after adding new segment: {}", jpeg.segments().len());

    // Roundtrip JPEG via bytes and report number of segments
    let new_bytes = {
        let mut bytes = vec![];
        jpeg.encoder().write_to(&mut bytes).unwrap();
        bytes
    };
    let new_jpeg = Jpeg::from_bytes(new_bytes.into()).unwrap();
    println!("Number of segments after roundtrip:          {}", new_jpeg.segments().len());
}

which demonstrates that pushing a new segment to .segments_mut() creates a segment which is accessible from the original JPEG instance, but disappears when the JPEG is serialized and deserialized again.

When push ing the new segment to jpeg.segments_mut(), the serialized representation of the JPEG contains the new segment after EOI and

https://github.com/paolobarbolini/img-parts/blob/main/src/jpeg/image.rs#L53-L55

ignores everything after EOI.

If you replace push(... with insert(<somewhere before last element>, then the new segment is placed before EOI and is recovered when the JPEG is deserialized.