BUG ImageResizingConverter can produce zero-sized output dimensions

[akshan-main]

Describe the bug

ImageResizingConverter accepts positive scale factors that can truncate the output width or height to 0, causing Pillow to raise ValueError: height and width must be > 0. It also accepts non-finite values such as nan and inf.

Steps/Code to Reproduce

from PIL import Image
from pyrit.prompt_converter import ImageResizingConverter

ImageResizingConverter(scale_factor=0.5)._apply_transform(
    Image.new("RGB", (1, 1))

Fix

Reject non-finite scale factors and raise a clear ValueError before calling Pillow when either calculated dimension is below one pixel.

I would like to submit a focused fix with unit tests.

@romanlutz

[akshan-main]

friendly ping @rlundeen2 @jsong468

[saisharan0103]

scale_factor=0.5 on a 1x1 image is enough to make a computed resize dimension truncate to 0, so Pillow receives an invalid target size. A focused PR could start by validating the scale factor is finite and positive, then checking the computed width and height are both at least 1 before calling Pillow. The regression tests should cover tiny images with fractional scales plus nan and inf, asserting clear ValueErrors.

[akshan-main]

@romanlutz I thought it was good practice to open an issue before creating a PR, otherwise I would've created the PR directly myself. I feel like @ThryLox is taking credit for my finding. I'd have preferred if it were my PR that was merged.

[romanlutz]

@akshan-main that's fair. To be honest, I primarily saw the PR while reviewing a chunk of others and figured that this is pretty close to what we want. Apologies.

In an ideal world, people should stop submitting PRs for things they clearly can tell others want to fix (as you pointed out above). This is perhaps the third of fourth time I've seen this here and I can't quite understand why people do this. If someone wants to do work they can just... ask? We have a discord for that purpose...

In terms of this particular case, there isn't a lot I can do now that it's merged. I'll try to read the issue description more carefully next time. I still think it's best to submit issues since there are plenty where we can't actually accept the proposed solution. If you went ahead and implemented it it would be throwaway work. If you prefer that I guess that is ok, too.

[ThryLox]

Hi @akshan-main and @romanlutz,

I want to sincerely apologize to @akshan-main for the misunderstanding! All credit for reporting this bug and providing the initial analysis belongs to @akshan-main.

While trying out PyRIT, I ran into the exact same issue during my testing. When I searched GitHub, I found @akshan-main's issue (#2162) and jumped straight into coding a fix, without realizing that @akshan-main had expressed interest in opening their own PR. There was absolutely no intent to take credit for your finding or step on your toes.

Thank you @akshan-main for the detailed issue report, and thank you @romanlutz for the review. Moving forward, I'll be sure to comment on issue threads first before submitting PRs on PyRIT!

[romanlutz]

Thanks for chiming in @ThryLox !