On Wed, 5 Mar 2025 18:29:49 GMT, Jeremy Wood <d...@openjdk.org> wrote:
> JPEG segments can only be 65535-bytes long. (The marker length is expressed > as 2 bytes.) The problem in this ticket is that we were writing more than > 65535 bytes to a segment, which later caused parsing errors when we tried to > read the file back. > > This includes 2 changes: > > 1. We now clip the thumbnail width/height to fit within the available marker. > We were already constraining the width & height to a max of 255. (Because the > width & height are expressed as 1 byte.) So this new clipping is similar to > something we were already doing, and we issue the same WARNING_THUMB_CLIPPED > warning to the writer. (Does this require a CSR?) > > 2. This adds a bounds check to `write2bytes`. IMO the bigger problem in this > ticket was the `ImageIO.write(..)` caller thought they ended up with a valid > JPEG. (We were violating the "do no harm / lose no data" doctrine.) Now if > something like this ever comes up again: writing will fail with an > IIOException. Technically you can argue this change is unnecessary because > the new clipping avoids this error condition, but IMO including this safety > net is an overall good thing to keep. If anyone disagrees I'm OK with > removing it. > > Separately: > I included (and reversed) a commit in this branch that provides an alternate > solution. That commit anticipates the overflow problem and switches to a > JPEG-encoded thumbnail. From an end user perspective this "just works": they > get a (slightly lossly) thumbnail of the original dimensions. But I assume it > would be more controversial compared to the clipping approach, since the > clipping approach has a strong precedent in this codebase. This pull request has now been integrated. Changeset: 60f3d607 Author: Jeremy Wood <micklen...@gmail.com> Committer: Phil Race <p...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/60f3d607412dfe289f33dd922dfc1c9ff766810f Stats: 168 lines in 3 files changed: 167 ins; 0 del; 1 mod 8351110: ImageIO.write for JPEG can write corrupt JPEG for certain thumbnail dimensions Reviewed-by: aivanov, prr ------------- PR: https://git.openjdk.org/jdk/pull/23920
