On Thu, 13 Feb 2025 01:08:04 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
> Built-in Profiles are singleton objects and if the user happens to modify
> this shared profile object via setData() then the modified version of the
> profile is returned each time the same built-in profile is requested via
> getInstance().
>
> It is good to protect Built-in profiles from such direct modification by
> adding BuiltIn profile check in `setData()` such that **only copies** of
> Built-In profiles are allowed to be updated.
>
> With the proposed fix, if Built-In profile is updated using `.setData()` it
> throws _**IAE - "BuiltIn profile cannot be modified"**_
>
> Fix consists of:
> * `private boolean isBuiltIn = false` in ICC_Profile which is set to true
> when BuiltIn profiles are created and false for non-builtIn profile.
> * Converted BuiltInProfile from private interface to private static class
> (with all the profile instances as static final)
> * `isBuiltIn` flag is set to true when BuiltInProfile are loaded.
> * JavaDoc update for setData() (CSR required)
>
> There are no restrictions on creating copies of BuiltIn profile and then
> modifying it, but what is being restricted with this fix is - the direct
> modification of the shared BuiltIn profile instance.
>
> Applications which need a modified version of the ICC Profile should instead
> do the following:
>
>
> byte[] profileData = ICC_Profile.getData() // get the byte array represtation
> of BuiltIn- profile
> ICCProfile newProfile = ICC_Profile.getInstance(profileData) // create a new
> profile
> newProfile.setData() // to modify and customize the profile
>
>
> Following existing tests are modified to update a copy of Built-In profile.
>
> - java/awt/color/ICC_Profile/SetHeaderInfo.java
> - java/awt/color/ICC_ProfileSetNullDataTest.java
> - sun/java2d/cmm/ProfileOp/SetDataTest.java
src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1165:
> 1163: * the {@code tagSignature}
> 1164: * @throws IllegalArgumentException if the profile being updated has
> 1165: * {@code isBuiltIn} flag set to true
isBuiltin is a private variable. You can't reference it in public API javadoc.
What this ought to say is
@throw IllegalArgumentException if this is a profile for one of the built-in
pre-defined ColorSpaces, i.e those which can be obtained by calling
ICC_Profile.getInstance(int cspace)
There isn't a general way to get that list - but it is short - only 5 colour
spaces, and you could put an @see ColorSpace link
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1955247029
