[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException [v2]
From:       Phil Race <prr () openjdk ! java ! net>
Date:       2021-10-29 16:50:12
Message-ID: 0OV-4vyqgUJWui7IfJ5VuqLjrD7WYmHRiv7-xYSW-DQ=.ade14fff-9c56-47c5-9954-bbb6eef539ad () github ! com
[Download RAW message or body]

On Fri, 29 Oct 2021 09:36:37 GMT, Masanori Yano <myano@openjdk.org> wrote:

> > Could you please review the 8262297 bug fixes?
> > 
> > In this case, ImageIO.write() should throw java.io.IOException rather than \
> > java.lang.IndexOutOfBoundsException. IndexOutOfBoundsException is caught and \
> > wrapped in IIOException in ImageIO.write() with this fix. In addition, \
> > IndexOutOfBoundsException is not expected to throw by RandomAccessFile#write() \
> > according to its API specification. So it should be fixed.
> 
> Masanori Yano has updated the pull request incrementally with one additional commit \
> since the last revision: 
> 8262297: ImageIO.write() method will throw IndexOutOfBoundsException

I am not sure that this is the right thing at all here. You need to dig deeper.
- If Image I/O reads the png and constructs a BufferedImage, then that BufferedImage \
                ought to be valid.
- If a valid image is written I would NOT expect an exception so catching it isn't \
the answer, fixing it is.

- If the image being read is "truncated" so that the BufferedImage isn't valid, why \
                was there no exception from read ?
- If we do have an invalid BufferedImage then, in that case, we may indeed expect \
that write might throw an exception which needs to be caught but IIOBE is just one \
thing that could go wrong, isn't it ?

So for me this starts with the read, and a proper explanation of what happens there, \
and then an explanation as to what is wrong with the BufferedImage. I have no \
interest in slapping a sticking plaster on the observed symptom. It needs to be root \
cause explained and fixed.

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

PR: https://git.openjdk.java.net/jdk/pull/6151


[prev in list] [next in list] [prev in thread] [next in thread]