> 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.
Jeremy Wood has updated the pull request incrementally with one additional commit since the last revision: Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> ------------- Changes: - all: https://git.openjdk.org/jdk/pull/23920/files - new: https://git.openjdk.org/jdk/pull/23920/files/e093793b..9573b82d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=23920&range=09 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23920&range=08-09 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/23920.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/23920/head:pull/23920 PR: https://git.openjdk.org/jdk/pull/23920
