RFR: JDK-8346465 : Add a check in setData() to restrict the update of Built-In ICC_Profiles [v11]

[Alexey Ivanov]

On Sat, 1 Mar 2025 01:11:45 GMT, Harshitha Onkar <honkar at 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"**_
>> 
>> 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 representation 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
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   doc update

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 113:

> 111:     /**
> 112:      * Set to true for {@code BuiltInProfile}, false otherwise.
> 113:      * This check is used in {@link #setData(int, byte[])} to prevent modifying

Suggestion:

     * Set to {@code true} for {@code BuiltInProfile}, {@code false} otherwise.
     * This check is used in {@link #setData(int, byte[])} to prevent modifying


[This suggestion](https://github.com/openjdk/jdk/pull/23606#discussion_r1969536537) isn't fully applied, yet it's marked as resolved.

The [javadoc style guide](https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#styleguide) recommends, <q cite="https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#styleguide">Use `<code>` style for keywords and names.</q> Therefore both `true` and `false` should be marked up with `{@code}`.

src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 132:

> 130: 
> 131:         ICC_Profile LRGB = new ICC_ProfileRGB(new ProfileDeferralInfo(
> 132:                 "LINEAR_RGB.pf", ColorSpace.TYPE_RGB, 3, CLASS_DISPLAY));

There should be no additional space added, all these lines in `BuiltInProfile` should remain unmodified when compared to the `master` branch.

src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1168:

> 1166:      *         the {@code tagSignature}
> 1167:      * @throws IllegalArgumentException if this is a built-in profile for one
> 1168:      *         of the pre-defined ColorSpaces, i.e. those which can be obtained

Suggestion:

     *         of the pre-defined color spaces, i.e. those which can be obtained

Here, you refer to _“color spaces”_ as the concept rather than the [`ColorSpace`](https://docs.oracle.com/en/java/javase/21/docs/api/java.desktop/java/awt/color/ColorSpace.html) class, therefore regular capitalisation and spelling applies.

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

PR Review: https://git.openjdk.org/jdk/pull/23606#pullrequestreview-2657212692
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1979306472
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1979320565
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1979315193