vstroebel / jfifdump

Dump structure of a jpeg file
Apache License 2.0
1 stars 2 forks source link

Length of Scan segment can be off if there are restart markers #8

Open mauricefisher64 opened 8 months ago

mauricefisher64 commented 8 months ago

In reader.rs read_scan_data function should kick out not just for 0x00 but also for any restart segment (RSTn).

             if byte != 0x00 {
                self.current_marker = Some(byte);
                break;
            } else {
                for _ in 0..ff_count {
                    data.push(0xFF);
                }
                data.push(byte);
            }

I think it should be something like this:

fn in_entropy(marker: u8) -> bool { matches!(marker, 0xd0..=0xd7 | 0x00) }

              if !in_entropy(byte) {
                self.current_marker = Some(byte);
                break;
            } else {
                for _ in 0..ff_count {
                    data.push(0xFF);
                }
                data.push(byte);
            }
mauricefisher64 commented 8 months ago

I think there also needs to be a rollback of the position pointer - 2 when !in_entropy(byte) is true since you have read the 0xFF and byte (the next tag). Otherwise the size will be off by two bytes since this code just detected the next tag.

vstroebel commented 8 months ago

Even if not technically correct, the current implementation treats restart markers like normal segments because I need this information for my personal use case. Your solution would completely remove the existence of restart markers from the dump.

One possible alternative is to extend the Scan struct to include all restarts as individual data parts. But this could make reading/dumping broken JPEG file more complicated.