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

List:       openjdk-2d-dev
Subject:    RFR: 8299333: Unify exceptions used by all variants of ICC_Profile.getInstance(null)
From:       Sergey Bylokhov <serb () openjdk ! org>
Date:       2022-12-24 3:41:18
Message-ID: vcJUwGyej8Uk8AQC25VHJib-Tn0MMDATnf8H3JEZygQ=.8212bab6-583d-4e8b-9dce-15a94ef52c39 () github ! com
[Download RAW message or body]

I tried to clean up the documentation in the `java.awt.color` package and specify how \
the `null` parameters are handled here and there. But it looks like the `ICC_profile` \
class is too specific so I would like to discuss it separately.

The ICC_Profile has these methods:
 * `getInstance(int)`      - always throws specified `IllegalArgumentException` on \
                the wrong constant
 * `getInstance(byte[])` - always throws specified `IllegalArgumentException` on \
                `null`
 * `getInstance(String fileName)` - always throws unspecified `NullPointerException` \
                on `null`
 * `getInstance(InputStream)`      - thrown unspecified `NullPointerException` before \
[JDK-8227816](https://github.com/openjdk/jdk/commit/af20c6b9c4a3a21c1e2c7f56721e1077e7d8f388) \
and now throws specified but misleading `IOException: Stream closed` exception on \
`null`

So we have 3 methods that can get null as a parameter and each throw a different \
exception.

Note that all of the specs for the methods above have this part:

> @throws IllegalArgumentException If the stream does not contain valid ICC Profile \
> data

So I have an idea to change `getInstance(String)` and `getInstance(InputStream)` to \
throw `IllegalArgumentException`  even if I personally prefer NPE to be thrown.

From another point of view the `ICC_Profile` profile has other methods:
 * `write(String fileName)` - always throws unspecified `NullPointerException` on \
                `null`
 * `write(OutputStream) `  - always throws unspecified `NullPointerException` on \
`null`

I am not sure we can throw `IllegalArgumentException` from the `write` methods, so \
the specification for `getInstance(String)` and `getInstance(InputStream)` could be \
updated to throw NPE on null same as related `write`.

Thoughts?

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

Commit messages:
 - 8299333: Unify used exceptions by all variants of ICC_Profile.getInstance(null)

Changes: https://git.openjdk.org/jdk/pull/11784/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11784&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8299333
  Stats: 22 lines in 2 files changed: 19 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/11784.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11784/head:pull/11784

PR: https://git.openjdk.org/jdk/pull/11784


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