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