Fix for a wrong buffer size with an animated WEBP

[arskov]

In case of an animated WEBP image, which contains frames of a smaller size than an image's canvas, the buffer of a whole image is bigger than the buffer of the first frame. This results in a panic while trying to copy a frame buffer to an image buffer. This is applicable to a case when a user wants to create an image from a file or an in memory buffer like this:

let path = "/some/path/to/animated_transparent.webp"
let mut buffer = Vec::new();
let _ = std::fs::File::open(path)?.read_to_end(&mut buffer);
let img = image::load_from_memory(&buffer)?;
let size = img.dimensions();
let buffer = img.to_rgb8().into_raw();

This fix checks if the first frame is of a smaller size, and in this case draws a subimage to a canvas.
I have this issue in my personal case, but I believe it also fixes #1856.

I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to choose either at their option.

[fintelia]

The fuzz testing from the CI is showing some issues:

failures:

---- codecs::webp::encoder::tests::fuzz_webp_no_panic stdout ----
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at '[quickcheck] TEST FAILED (runtime error). Arguments: ([], 0, 0, 0)
Error: "called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION"', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/quickcheck-1.0.3/src/tester.rs:165:28


failures:
    codecs::webp::encoder::tests::fuzz_webp_no_panic

[arskov]

The fuzz testing from the CI is showing some issues:

failures:

---- codecs::webp::encoder::tests::fuzz_webp_no_panic stdout ----
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at 'called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/webp-0.2.4/src/encoder.rs:78:40
thread 'codecs::webp::encoder::tests::fuzz_webp_no_panic' panicked at '[quickcheck] TEST FAILED (runtime error). Arguments: ([], 0, 0, 0)
Error: "called `Result::unwrap()` on an `Err` value: VP8_ENC_ERROR_BAD_DIMENSION"', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/quickcheck-1.0.3/src/tester.rs:165:28


failures:
    codecs::webp::encoder::tests::fuzz_webp_no_panic

I tried to run that tests on a master without my changes, and it produces the same errors with the same tests. I'll re-check, what would be a suggestion in this case?

[fintelia]

Could you grab a backtrace of the failures? (By running with RUST_BACKTRACE=1)

If it isn't in the code being changed here, I'd say we just merge this PR and then figure out what's going wrong in a followup.

[fintelia]

I'm slightly nervous that first_frame.width or first_frame.height could be larger than the size of the canvas. Do you know what would happen in that case?

[arskov]

Good point. If the frame's width or height are larger than the canvas size, or even if they are smaller, but they have an x or y offset + width/height, that gets larger than the canvas width/height, than the code will panic on extended.rs#L268, because of how Index is implemented for ImageBuffer.

We can make an additional check if the calculated index is within a canvas size extended.rs#266 and if some x or y is out of bounds, then break the corresponding loop. Some sort of crop of the frame if it is outside the canvas.

[fintelia]

If it isn't too much trouble, could you implement that cropping logic now? Otherwise it can wait for a followup PR

[arskov]

I created #1960, you can assign it to me. I'll implement it in a separate PR if you don't mind.

[arskov]

This is backtrace for tests from master (without the change)
https://gist.github.com/arskov/ef235460456cb29bc65cd25621be03bf

This is backtrace from the branch with the proposal
https://gist.github.com/arskov/df18c28a8c61031e11010e8a28bac3ab

Both show that the panic happens on the line below, so it looks like a separate issue to investigate
https://github.com/image-rs/image/blob/master/src/codecs/webp/encoder.rs#L106