Implement --fix to fix CRC errors in PNG block headers

Closes #5
This commit is contained in:
Joshua Holmer 2016-05-02 09:25:48 -04:00
parent 08eb44ec1a
commit a4a5953786
8 changed files with 75 additions and 40 deletions

View file

@ -2,6 +2,7 @@
- Fix issue where output directory would not be created if it did not exist
- Use miniz for compression strategies where it outperforms zlib
- [SEMVER_MINOR] Partially implement -p / --preserve, as far as stable Rust will allow for now
- [SEMVER_MINOR] Implement --fix to ignore CRC errors and recalculate correct CRC in output
**Version 0.5.0**
- [SEMVER_MINOR] Palette entries can now reduced, on by default ([#11](https://github.com/shssoichiro/oxipng/issues/11))

View file

@ -90,7 +90,7 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> {
writeln!(&mut stderr(), "Processing: {}", filepath.to_str().unwrap()).ok();
}
let in_file = Path::new(filepath);
let mut png = match png::PngData::new(&in_file) {
let mut png = match png::PngData::new(&in_file, opts.fix_errors) {
Ok(x) => x,
Err(x) => return Err(x),
};

View file

@ -4,8 +4,7 @@ use crc::crc32;
use std::collections::{HashMap, HashSet};
use std::fmt;
use std::fs::File;
use std::io::Cursor;
use std::io::prelude::*;
use std::io::{Cursor, Read, Write};
use std::iter::Iterator;
use std::path::Path;
@ -292,7 +291,7 @@ pub struct IhdrData {
impl PngData {
/// Create a new `PngData` struct by opening a file
pub fn new(filepath: &Path) -> Result<PngData, String> {
pub fn new(filepath: &Path, fix_errors: bool) -> Result<PngData, String> {
let mut file = match File::open(filepath) {
Ok(f) => f,
Err(_) => return Err("Failed to open file for reading".to_owned()),
@ -314,7 +313,7 @@ impl PngData {
let mut aux_headers: HashMap<String, Vec<u8>> = HashMap::new();
let mut idat_headers: Vec<u8> = Vec::new();
loop {
let header = parse_next_header(byte_data.as_ref(), &mut byte_offset);
let header = parse_next_header(byte_data.as_ref(), &mut byte_offset, fix_errors);
let header = match header {
Ok(x) => x,
Err(x) => return Err(x),
@ -1574,7 +1573,8 @@ fn file_header_is_valid(bytes: &[u8]) -> bool {
}
fn parse_next_header(byte_data: &[u8],
byte_offset: &mut usize)
byte_offset: &mut usize,
fix_errors: bool)
-> Result<Option<(String, Vec<u8>)>, String> {
let mut rdr = Cursor::new(byte_data.iter()
.skip(*byte_offset)
@ -1615,8 +1615,9 @@ fn parse_next_header(byte_data: &[u8],
};
*byte_offset += 4;
header_bytes.extend(data.clone());
if crc32::checksum_ieee(header_bytes.as_ref()) != crc {
return Err(format!("Corrupt data chunk found--CRC Mismatch in {}", header));
if !fix_errors && crc32::checksum_ieee(header_bytes.as_ref()) != crc {
return Err(format!("Corrupt data chunk found--CRC Mismatch in {}\nThis may be recoverable by using --fix",
header));
}
Ok(Some((header, data)))

BIN
tests/files/fix_errors.png Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 43 KiB

View file

@ -56,7 +56,7 @@ fn test_it_converts(input: &Path,
bit_depth_in: png::BitDepth,
color_type_out: png::ColorType,
bit_depth_out: png::BitDepth) {
let png = png::PngData::new(input).unwrap();
let png = png::PngData::new(input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == color_type_in);
assert!(png.ihdr_data.bit_depth == bit_depth_in);
@ -67,7 +67,7 @@ fn test_it_converts(input: &Path,
};
assert!(output.exists());
let png = match png::PngData::new(output) {
let png = match png::PngData::new(output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();

View file

@ -56,7 +56,7 @@ fn test_it_converts(input: &Path,
bit_depth_in: png::BitDepth,
color_type_out: png::ColorType,
bit_depth_out: png::BitDepth) {
let png = png::PngData::new(input).unwrap();
let png = png::PngData::new(input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == color_type_in);
assert!(png.ihdr_data.bit_depth == bit_depth_in);
@ -67,7 +67,7 @@ fn test_it_converts(input: &Path,
};
assert!(output.exists());
let png = match png::PngData::new(output) {
let png = match png::PngData::new(output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -111,7 +111,7 @@ fn strip_headers_list() {
opts.strip = png::Headers::Some(vec!["iCCP".to_owned(), "tEXt".to_owned()]);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.aux_headers.contains_key("tEXt"));
assert!(png.aux_headers.contains_key("iTXt"));
@ -123,7 +123,7 @@ fn strip_headers_list() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -152,7 +152,7 @@ fn strip_headers_safe() {
opts.strip = png::Headers::Safe;
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.aux_headers.contains_key("tEXt"));
assert!(png.aux_headers.contains_key("iTXt"));
@ -164,7 +164,7 @@ fn strip_headers_safe() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -193,7 +193,7 @@ fn strip_headers_all() {
opts.strip = png::Headers::All;
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.aux_headers.contains_key("tEXt"));
assert!(png.aux_headers.contains_key("iTXt"));
@ -205,7 +205,7 @@ fn strip_headers_all() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -234,7 +234,7 @@ fn strip_headers_none() {
opts.strip = png::Headers::None;
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.aux_headers.contains_key("tEXt"));
assert!(png.aux_headers.contains_key("iTXt"));
@ -246,7 +246,7 @@ fn strip_headers_none() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -275,7 +275,7 @@ fn interlacing_0_to_1() {
opts.interlace = Some(1);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.interlaced == 0);
@ -285,7 +285,7 @@ fn interlacing_0_to_1() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -312,7 +312,7 @@ fn interlacing_1_to_0() {
opts.interlace = Some(0);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.interlaced == 1);
@ -322,7 +322,7 @@ fn interlacing_1_to_0() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -349,7 +349,7 @@ fn interlacing_0_to_1_small_files() {
opts.interlace = Some(1);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.interlaced == 0);
assert!(png.ihdr_data.color_type == png::ColorType::Indexed);
@ -361,7 +361,7 @@ fn interlacing_0_to_1_small_files() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -390,7 +390,7 @@ fn interlacing_1_to_0_small_files() {
opts.interlace = Some(0);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.interlaced == 1);
assert!(png.ihdr_data.color_type == png::ColorType::Indexed);
@ -402,7 +402,7 @@ fn interlacing_1_to_0_small_files() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -434,7 +434,7 @@ fn interlaced_0_to_1_other_filter_mode() {
opts.filter = filter;
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.interlaced == 0);
@ -444,7 +444,7 @@ fn interlaced_0_to_1_other_filter_mode() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -481,3 +481,36 @@ fn preserve_attrs() {
// TODO: Actually check permissions
}
#[test]
fn fix_errors() {
let input = PathBuf::from("tests/files/fix_errors.png");
let mut opts = get_opts(&input);
opts.fix_errors = true;
let output = opts.out_file.clone();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == png::ColorType::RGBA);
assert!(png.ihdr_data.bit_depth == png::BitDepth::Eight);
match oxipng::optimize(&input, &opts) {
Ok(_) => (),
Err(x) => panic!(x.to_owned()),
};
assert!(output.exists());
let png = match png::PngData::new(&output, false) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
panic!(x.to_owned())
}
};
assert!(png.ihdr_data.color_type == png::ColorType::Grayscale);
assert!(png.ihdr_data.bit_depth == png::BitDepth::Eight);
// Cannot check if pixels are equal because image crate cannot read corrupt (input) PNGs
remove_file(output).ok();
}

View file

@ -56,7 +56,7 @@ fn test_it_converts(input: &Path,
bit_depth_in: png::BitDepth,
color_type_out: png::ColorType,
bit_depth_out: png::BitDepth) {
let png = png::PngData::new(input).unwrap();
let png = png::PngData::new(input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == color_type_in);
assert!(png.ihdr_data.bit_depth == bit_depth_in);
@ -68,7 +68,7 @@ fn test_it_converts(input: &Path,
};
assert!(output.exists());
let png = match png::PngData::new(output) {
let png = match png::PngData::new(output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();

View file

@ -56,7 +56,7 @@ fn test_it_converts(input: &Path,
bit_depth_in: png::BitDepth,
color_type_out: png::ColorType,
bit_depth_out: png::BitDepth) {
let png = png::PngData::new(input).unwrap();
let png = png::PngData::new(input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == color_type_in);
assert!(png.ihdr_data.bit_depth == bit_depth_in);
@ -68,7 +68,7 @@ fn test_it_converts(input: &Path,
};
assert!(output.exists());
let png = match png::PngData::new(output) {
let png = match png::PngData::new(output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(output).ok();
@ -1281,7 +1281,7 @@ fn palette_should_be_reduced_with_dupes() {
let opts = get_opts(&input);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == png::ColorType::Indexed);
assert!(png.ihdr_data.bit_depth == png::BitDepth::Eight);
@ -1293,7 +1293,7 @@ fn palette_should_be_reduced_with_dupes() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(&output).ok();
@ -1321,7 +1321,7 @@ fn palette_should_be_reduced_with_unused() {
let opts = get_opts(&input);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == png::ColorType::Indexed);
assert!(png.ihdr_data.bit_depth == png::BitDepth::Eight);
@ -1333,7 +1333,7 @@ fn palette_should_be_reduced_with_unused() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(&output).ok();
@ -1361,7 +1361,7 @@ fn palette_should_be_reduced_with_both() {
let opts = get_opts(&input);
let output = opts.out_file.clone();
let png = png::PngData::new(&input).unwrap();
let png = png::PngData::new(&input, opts.fix_errors).unwrap();
assert!(png.ihdr_data.color_type == png::ColorType::Indexed);
assert!(png.ihdr_data.bit_depth == png::BitDepth::Eight);
@ -1373,7 +1373,7 @@ fn palette_should_be_reduced_with_both() {
};
assert!(output.exists());
let png = match png::PngData::new(&output) {
let png = match png::PngData::new(&output, opts.fix_errors) {
Ok(x) => x,
Err(x) => {
remove_file(&output).ok();