Ignore compressed data length on RGB compression mehod

[EvansJahja]

Thank you for helping out with tinybmp development! Please:

• Check that you've added passing tests and documentation
• Add a CHANGELOG.md entry in the Unreleased section under the appropriate heading (Added, Fixed, etc) if your changes affect the public API
• Run rustfmt on the project
• Run just build (Linux/macOS only) and make sure it passes. If you use Windows, check that CI passes once you've opened the PR.

PR description

"compressed image size" should be ignored for RGB data. I opened this PR because I have a program (Aseprite) that outputs a BMP file with bogus compressed image size that led me into thinking there's an issue with this library. The image generated by Aseprite can be opened normally in Windows Paint, Windows preview, Photoshop, Image Glass, but not this library.

Apparently, microsoft says the field may be zero for RGB and they're kind of the most authoritive entity on BMP files that I know of

biSizeImage
The size, in bytes, of the image. This may be set to zero for BI_RGB bitmaps.

If biCompression is BI_JPEG or BI_PNG, biSizeImage indicates the size of the JPEG or PNG image buffer, respectively.

ref

[rfuest]

This might be a regression introduced in version 0.6, where compression support was added. Previous versions of tinybmp calculated the expected image data length like this:

tinybmp/src/raw_bmp.rs

Lines 53 to 62 in ea2d38a

	let data_length = header
	.bytes_per_row()
	.checked_mul(height)
	.ok_or(ParseError::InvalidImageDimensions)?;
	// The `get` is split into two calls to prevent an possible integer overflow.
	let image_data = &bytes
	.get(header.image_data_start..)
	.and_then(|bytes| bytes.get(..data_length))
	.ok_or(ParseError::UnexpectedEndOfFile)?;

Your changes completely disable the checks for uncompressed images, which makes the error_truncated_image_data fail. I would suggest to use the pre 0.6 behavior for Rgb compressed images.

[EvansJahja]

Thank you for your feedback, I have adjusted as suggested

[rfuest]

Looks good, I just have a few minor suggestions. Please also add an entry to CHANGELOG.md.

[rfuest]

The code after the data length is determined is identical in both code paths and could be shared by using something like this (untested):

Suggested change

	if let crate::header::CompressionMethod::Rgb = header.compression_method {
	let height = header.image_size.height as usize;
	let data_length = match header.bytes_per_row().checked_mul(height) {
	Some(res) => res,
	None => return Err(ParseError::UnexpectedEndOfFile),
	};
	if image_data.len() < data_length {
	return Err(ParseError::UnexpectedEndOfFile);
	}
	let (image_data, _) = image_data.split_at(data_length);
	Ok(Self {
	header,
	color_type,
	color_table,
	image_data,
	})
	} else {
	let compressed_data_length = header.image_data_len as usize;
	if image_data.len() < compressed_data_length {
	return Err(ParseError::UnexpectedEndOfFile);
	}
	let (image_data, _) = image_data.split_at(compressed_data_length);
	Ok(Self {
	header,
	color_type,
	color_table,
	image_data,
	})
	}
	let data_length = if let crate::header::CompressionMethod::Rgb = header.compression_method {
	let height = header.image_size.height as usize;
	let Some(data_length) = header.bytes_per_row().checked_mul(height) else {
	return Err(ParseError::UnexpectedEndOfFile),
	};
	data_length
	} else {
	header.image_data_len as usize
	};
	if image_data.len() < data_length {
	return Err(ParseError::UnexpectedEndOfFile);
	}
	Ok(Self {
	header,
	color_type,
	color_table,
	image_data: &image_data[0..data_length],
	})

[EvansJahja]

Updated, and thank you so much for the suggestion, I'm still learning rust and didn't know about let...else construct, that's super clean!

Some adjustments from this:

1. move comments around and rewording
2. &image_data[0..data_length] sadly doesn't work in a const fn

[rfuest]

..., I'm still learning rust and didn't know about let...else construct, that's super clean!

let else was a great addition to Rust some time ago. A lot of code was written before this was available and doesn't make use of it yet.

2. &image_data[0..data_length] sadly doesn't work in a const fn

OK, thanks for reminding me. I was already wondering why the code used split_at and didn't remember that I used it to work around const limitations.

[rfuest]

The field ends in _len.

Suggested change

	// image_data_length is only meaningful when it's not RGB
	// `Header::image_data_len` is only meaningful when it's not RGB

[rfuest]

LGTM. Thanks for the PR!