RFR: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException
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. ------------- Commit messages: - 8262297: ImageIO.write() method will throw IndexOutOfBoundsException Changes: https://git.openjdk.java.net/jdk/pull/6151/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6151&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262297 Stats: 96 lines in 4 files changed: 71 ins; 0 del; 25 mod Patch: https://git.openjdk.java.net/jdk/pull/6151.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6151/head:pull/6151 PR: https://git.openjdk.java.net/jdk/pull/6151
On Thu, 28 Oct 2021 09:29:13 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.
src/java.base/share/classes/java/io/RandomAccessFile.java line 558:
556: * @throws IndexOutOfBoundsException If {@code off} is negative, 557: * {@code len} is negative, or {@code len} is greater than 558: * {@code b.length - off}
The IOOBE is specified in the super interface, it's just not shown in the javadoc because it's a runtime exception. So I think what you want here is: @throws IndexOutOfBoundsException {@inheritDoc} ------------- PR: https://git.openjdk.java.net/jdk/pull/6151
On Thu, 28 Oct 2021 14:30:20 GMT, Alan Bateman <alanb@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.
src/java.base/share/classes/java/io/RandomAccessFile.java line 558:
556: * @throws IndexOutOfBoundsException If {@code off} is negative, 557: * {@code len} is negative, or {@code len} is greater than 558: * {@code b.length - off}
The IOOBE is specified in the super interface, it's just not shown in the javadoc because it's a runtime exception. So I think what you want here is:
@throws IndexOutOfBoundsException {@inheritDoc}
Unfortunately the parent class does not specify this exception via javadoc tags like "@throws IndexOutOfBoundsException", but instead just describe it in the method description should we fix it separately from the imageio fix? ------------- PR: https://git.openjdk.java.net/jdk/pull/6151
On Fri, 29 Oct 2021 07:01:04 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:
Unfortunately the parent class does not specify this exception via javadoc tags like "@throws IndexOutOfBoundsException", but instead just describe it in the method description
should we fix it separately from the imageio fix?
Yes, it might be better to separate them because we'll like need a CSR if we are missing @throws in a few places. ------------- PR: https://git.openjdk.java.net/jdk/pull/6151
On Fri, 29 Oct 2021 07:13:32 GMT, Alan Bateman <alanb@openjdk.org> wrote:
Unfortunately the parent class does not specify this exception via javadoc tags like "@throws IndexOutOfBoundsException", but instead just describe it in the method description
should we fix it separately from the imageio fix?
Unfortunately the parent class does not specify this exception via javadoc tags like "@throws IndexOutOfBoundsException", but instead just describe it in the method description
should we fix it separately from the imageio fix?
Yes, it might be better to separate them because we'll like need a CSR if we are missing @throws in a few places.
I understood to separate javadoc fix. I will remove the javadoc fix from this PR and issue another Bug ID. ------------- PR: https://git.openjdk.java.net/jdk/pull/6151
On Thu, 28 Oct 2021 09:29:13 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.
This needs looking at closely .. I don't know what's best but the imaging APIs are very exception heavy because of the arrays, indices, you name it that they consume. Most of the time its probably best for an IIO user to just get an IIOException wrapping the original cause. But there could be exceptions. ------------- PR: https://git.openjdk.java.net/jdk/pull/6151
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 ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/6151/files - new: https://git.openjdk.java.net/jdk/pull/6151/files/d6a652fa..08b54fff Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6151&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6151&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6151.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6151/head:pull/6151 PR: https://git.openjdk.java.net/jdk/pull/6151
participants (4)
-
Alan Bateman
-
Masanori Yano
-
Phil Race
-
Sergey Bylokhov