RFR: 8358057: Update validation of ICC_Profile header data

[Harshitha Onkar]

On Thu, 29 May 2025 10:22:39 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

> The [next PR](https://github.com/openjdk/jdk/pull/23044) introduces several new methods to the ICC_Profile class:
> -  getProfileClass(byte[])
> -  getColorSpaceType(byte[])
> -  getPCSType(byte[])
> -  checkRenderingIntent(byte[])
> 
> These new methods extract data directly from the provided byte array rather than relying on the profile instance. The first three methods essentially duplicate the existing ones (getProfileClass(), getColorSpaceType(), getPCSType()).
> 
> It is possible to update implementation:
> - The existing methods getColorSpaceType() and getPCSType() could delegate to the new getColorSpaceType(byte[]) and getPCSType(byte[]) methods.
> - The checkRenderingIntent(byte[]) method could be updated to report the actual invalid intent value when an error occurs
> 
> Tests:
>  - Old ValidateICCHeaderData test is update to verify the new output of the checkRenderingIntent
>  - New RenderingIntentStressTest test is added to check the next part of the icc_spec:
> 
>           * ICC spec: only the least-significant 16 bits encode the rendering
>           * intent. The most significant 16 bits must be zero and can be ignored.
>           * See https://www.color.org/ICC1v42_2006-05.pdf, section 7.2.15.
> 
> 
> 
> @honkar-jdk please take a look.
> 
> Note: There is currently an inconsistency in the usage of `getData(icSigHead)` vs `getData(cmmProfile(), icSigHead)` throughout the codebase. I plan to address this separately.

Initial testing looks good.
I have added minor inline comments.

src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1191:

> 1189:          * ICC spec: only the least-significant 16 bits encode the rendering
> 1190:          * intent. The most significant 16 bits must be zero and can be ignored.
> 1191:          * See https://www.color.org/ICC1v42_2006-05.pdf, section 7.2.15.

ICC Spec link in the comment refers to older one, probably update to the new version ?  https://www.color.org/specification/ICC.1-2022-05.pdf , https://www.color.org/icc_specs2.xalter.

test/jdk/java/awt/color/ICC_Profile/RenderingIntentStressTest.java line 95:

> 93:                 || intent == icMediaRelativeColorimetric
> 94:                 || intent == icSaturation || intent == icAbsoluteColorimetric
> 95:                 || intent == icICCAbsoluteColorimetric;

I hadn't noticed it earlier but `icAbsoluteColorimetric` and `icICCAbsoluteColorimetric` (since 1.5) point to the same intent. Can it be unified or has it be maintained as two separate constants for backward compatibility ?

test/jdk/java/awt/color/ICC_Profile/ValidateICCHeaderData/ValidateICCHeaderData.java line 199:

> 197:             if (!message.contains(": " + invalidRenderIntent)) {
> 198:                 throw new RuntimeException("Test Failed ! Unexpected text");
> 199:             }

The usual recommendation is not to check for specific exception messages but in this case it should be fine since this fix changes the exception msg and as regression test update for the fix.

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

PR Review: https://git.openjdk.org/jdk/pull/25519#pullrequestreview-2882873809
PR Review Comment: https://git.openjdk.org/jdk/pull/25519#discussion_r2116863194
PR Review Comment: https://git.openjdk.org/jdk/pull/25519#discussion_r2116878277
PR Review Comment: https://git.openjdk.org/jdk/pull/25519#discussion_r2116872440