On Thu, 12 Jun 2025 18:07:53 GMT, Sergey Bylokhov <s...@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
