On Fri, 21 Mar 2025 17:39:37 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"**_ >> >> 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
