RFR: JDK-8346465 : Add a check in setData() to restrict the update of Built-In ICC_Profiles [v11]
Harshitha Onkar
honkar at openjdk.org
Tue Mar 18 17:10:14 UTC 2025
On Fri, 14 Mar 2025 01:24:04 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:
>>> > Since there is no way to check if a profile is built-in, making built-in profiles read-only might be a better approach, similar to [java properties](https://bugs.openjdk.org/browse/JDK-8066709).
>>>
>>> But it also has its own disadvantages...
>>
>> @mrserb You mentioned earlier that there are disadvantages of using java properties approach, so using builtIn flag would be better then, correct?
>>
>>> I remember we discussed this issue during the review of [this commit](https://github.com/openjdk/jdk/pull/3037/files#diff-3dd53a33889801159f43dbb990ba033066bdabaed71bbc7254f58331d3898d69R39), and the conclusion was that it was too late to change it and break apps.
>>
>> I wasn't able to find the previous discussion thread so I'm not sure what are the disadvantages. Can you please explain?
>
> If we start from the beginning. The problem we are trying to solve is preventing changes to our standard profiles so that we can be sure one part of the application does not unexpectedly affect another. This could be achieved in two ways:
>
> - Throw an exception if a standard profile is modified
> - Ignore the requested change and use the default profile properties as is
>
> For new applications, we could require always cloning the color profile before modification (via the updated specification); otherwise, the application must always check whether the profile is built-in or not.
>
> However, for older applications, these two solutions lead to different outcomes:
>
> - Throwing the new exception will most likely break the application
> - Ignoring the data change allows the application to continue working, but the color conversion result may not be fully accurate
>
> The next question is: how big of an issue is this inaccuracy? For example, if we have an sRGB profile that rejects some modifications, we would still try to produce valid sRGB data during conversion.
>
> So the trade-off is: broken applications vs. inaccurate (but still ICC-spec-compliant) conversions.
>
> PS: By the way, why do we want to enforce this rule only for built-in profiles? Why can't a library create similar profiles and mark them as "read-only"?
@mrserb
> The app may obtain a profile from some place and use it for some images or pixels. Then, if the app wants to tweak the rendering intent for some reason, what should it do?
>
> Clone the profile and then change the intent?
The app would follow the above approach.
There is only one step extra with this fix - creating a copy of built-in profile and then modifying it.
byte[] builtInData = ICC_Profile.getInstance(ColorSpace.CS_sRGB).getData(); // get the byte array representation of BuiltIn- profile
ICCProfile newProfile = ICC_Profile.getInstance(builtInData) // create a new profile
newProfile.setData(...)
> However, for older applications, these two solutions lead to different outcomes:
> Throwing the new exception will most likely break the application
The chances of breaking an existing application is minimal since API usage for setData() was checked and not many were found and moreover they were not called on built-in profiles.
>Ignoring the data change allows the application to continue working, but the color conversion result may not be fully accurate
This does not seem viable because the user in unaware as to whether setData() worked or not.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r2001540142
More information about the client-libs-dev
mailing list