RFR: 8351110: ImageIO.write for JPEG can write corrupt JPEG for certain thumbnail dimensions [v9]

[Jeremy Wood]

On Wed, 9 Apr 2025 20:56:40 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Jeremy Wood has updated the pull request incrementally with six additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/JDK-8351110' into JDK-8351110
>>    
>>    # Conflicts:
>>    #	test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java
>>  - 8351110: code cleanup
>>    
>>    This is in response to:
>>    https://github.com/openjdk/jdk/pull/23920#discussion_r2035785315
>>  - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java
>>    
>>    Co-authored-by: Alexey Ivanov <alexey.ivanov at oracle.com>
>>  - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java
>>    
>>    Co-authored-by: Alexey Ivanov <alexey.ivanov at oracle.com>
>>  - Update test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java
>>    
>>    Co-authored-by: Alexey Ivanov <alexey.ivanov at oracle.com>
>>  - 8351110: code cleanup
>>    
>>    This is in response to:
>>    https://github.com/openjdk/jdk/pull/23920#discussion_r2029297962
>
> test/jdk/javax/imageio/plugins/jpeg/WriteJPEGThumbnailTest.java line 85:
> 
>> 83:             ByteArrayOutputStream byteOut = new ByteArrayOutputStream();
>> 84:             BufferedImage thumbnail = writeImage(byteOut);
>> 85:             byte[] jpegData = byteOut.toByteArray();
> 
> Now I see the complication. When you use try-with-resources, the stream is declared within the scope of the try-block. I didn't notice it before.
> 
> I may suggest something like this to overcome the complication:
> 
> 
>             BufferedImage thumbnail;
>             ByteArrayOutputStream byteOut = new ByteArrayOutputStream();
>             try (byteOut) {
>                 thumbnail = writeImage(byteOut);
>             }
> 
>             ImageReader reader = getJPEGImageReader();
>             ImageInputStream stream = ImageIO.createImageInputStream(
>                     new ByteArrayInputStream(byteOut.toByteArray()));
>             reader.setInput(stream);
> 
> 
> Such syntax for try-with-resources is acceptable since Java 9, if my memory serves me right; yet Java 8 requires you to declare and initialise the auto-closable stream inside the `try`-parentheses.
> 
> Anyway the current version looks good to me too, and it's shorter.

OK; if it looks good then I'll resolve this conversation.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23920#discussion_r2036573003