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

[Sergey Bylokhov]

On Fri, 21 Mar 2025 17:39:37 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 21 additional commits since the last revision:
> 
>  - Merge branch 'master' into BuiltInCheck
>  - updated test to check for all builtIn profiles, serial-deserialization
>  - redudant stmt removed
>  - modifier order changed, added comment to BuiltInProfile
>  - review changes
>  - builtIn converted to transient, tests updated
>  - minor
>  - review changes
>  - doc update
>  - builtIn flag moved to constructor
>  - ... and 11 more: https://git.openjdk.org/jdk/compare/8a3e1b75...7da4c5c7

test/jdk/java/awt/color/ICC_Profile/BuiltInProfileCheck.java line 104:

> 102: 
> 103:     // this test is added to check the ICC_Profile's transient isBuiltIn flag
> 104:     private static void testSerialization(boolean isBuiltIn)

This method does not actually test serialization; instead, it clones/copies the ICC profile via the filesystem. This is why you did not get an exception at the end of this method, even for a built-in profile.

You should use something like this instead:


ByteArrayOutputStream baos = new ByteArrayOutputStream();
ObjectOutputStream oos = new ObjectOutputStream(baos);
oos.writeObject(iccProfile);


byte[] array = baos.toByteArray();
ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(array));
ICC_Profile iccProfile = (ICC_Profile) ois.readObject();

In the example above, the built-in profile should throw an IAE, but a custom profile should not. (I suggest covering all built-in profiles as well).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r2008431593