RFR: 8358623: Avoid unnecessary data copying in ICC_Profile
Harshitha Onkar
honkar at openjdk.org
Fri Jun 13 19:21:42 UTC 2025
On Thu, 12 Jun 2025 18:07:53 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:
>> src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 802:
>>
>>> 800: public static ICC_Profile getInstance(byte[] data) {
>>> 801: ProfileDataVerifier.verify(data);
>>> 802: verifyHeader(data);
>>
>> [verifyHeader(byte[] data)](https://github.com/openjdk/jdk/blob/3b32f6a8ec37338764d3e6713247ff96e49bf5b3/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java#L1176C2-L1184C6), expects header byte array and not the entire profile array, is it technically correct to send the entire profile byte array?
>>
>> Please note verifyHeader() is called from two places from `getInstance(byte[] data)` and `setData(int tagSignature, byte[] tagData)`,
>>
>> With the current fix, logically all the checks within verifyHeader() work (getProfileClass(data), getColorSpaceType(data) ...) because the data is extracted at specified index from the superset - entire profile byte array instead of the header byte array.
>>
>> If it is decided to send only header byte array to `verifyHeader() ` then changing the method param name to **header** instead of **data** may be clearer.
>>
>>
>> private static void verifyHeader(byte[] header) {
>> if (header == null || header.length < HEADER_SIZE) {
>> throw new IllegalArgumentException("Invalid header data");
>> }
>> getProfileClass(header);
>> getColorSpaceType(header);
>> getPCSType(header);
>> checkRenderingIntent(header);
>> }
>
>> [verifyHeader(byte[] data)](https://github.com/openjdk/jdk/blob/3b32f6a8ec37338764d3e6713247ff96e49bf5b3/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java#L1176C2-L1184C6), expects header byte array and not the entire profile array, is it technically correct to send the entire profile byte array?
>
> Validating the header directly without cloning is fine and actually better. The old code effectively skipped the "header.length < HEADER_SIZE" check because it always created a "new byte[HEADER_SIZE]".
I see your point. I do think renaming the param name for the following checks to something more generic such as `data` or `profileData` instead of header will cause less confusion then.
private static void checkRenderingIntent(**byte[] header**)
private static int getColorSpaceType(**byte[] theHeader**)
private static int getProfileClass(**byte[] theHeader**)
private static int getPCSType(**byte[] theHeader**)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25650#discussion_r2145917397
More information about the client-libs-dev
mailing list