> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 24 additional commits since the last revision: - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> - Merge remote-tracking branch 'origin/JDK-8351110' into JDK-8351110 - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> - Update src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JFIFMarkerSegment.java Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com> - Merge branch 'master' into JDK-8351110 - Merge branch 'master' of https://github.com/mickleness/jdk - Merge branch 'openjdk:master' into master - 8351110: adding a few clarifying comments - ... and 14 more: https://git.openjdk.org/jdk/compare/ce0122f9...ecd7883c ------------- Changes: - all: https://git.openjdk.org/jdk/pull/23920/files - new: https://git.openjdk.org/jdk/pull/23920/files/21dce8e5..ecd7883c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=23920&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23920&range=04-05 Stats: 149020 lines in 3405 files changed: 66274 ins; 63624 del; 19122 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
