On Thu, 10 Apr 2025 04:15:31 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 26 additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into BuiltInCheck
>  - added javadoc to testprofile
>  - single case type
>  - added test cases to read .icc, updated test code
>  - serialization test case changes
>  - 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
>  - ... and 16 more: https://git.openjdk.org/jdk/compare/b62ca1df...b55bebf2

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

> 167:     }
> 168: }
> 169: 

Suggestion:

}

A file should end in a single `\n` characters; there's no need to have two or 
more.

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

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

Reply via email to