RFR: JDK-8347377 : Add validation checks for ICC_Profile header fields [v3]
Sergey Bylokhov
serb at openjdk.org
Mon Jan 13 19:22:47 UTC 2025
On Fri, 10 Jan 2025 19:34:36 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:
>> ICC_Profile.setData(..) does validation of the specified tag contents and throws an exception if it is not valid. But if the tag represents the header, at least some of the validation is lazy, occurring only when the data is used, leading to unexpected exceptions at a later time. The check should be done up-front when the data is set, as in other cases.
>>
>> `verifyHeader(byte[] data)`is called when header data is being updated and the following fields are validated according to the ICC Spec Document. [[1] Pg#19](https://www.color.org/specification/ICC.1-2022-05.pdf).
>>
>> - Profile/Device class
>> - Color Space
>> - Rendering Intent
>> - PCS
>> - Header Size check (ICC Header Size = 128 bytes)
>>
>> These validation checks are added to ICC_Profile.getInstance(..) & ICC_Profile.setData(..) methods.
>>
>> Reference: [1] https://www.color.org/specification/ICC.1-2022-05.pdf
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
>
> indentation
src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 795:
> 793: }
> 794:
> 795: if (p != null) {
If it possible to get null here we should thrown an exception, but I think we thrown that exception already in the native.
src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 995:
> 993: case icSigAbstractClass -> CLASS_ABSTRACT;
> 994: case icSigNamedColorClass -> CLASS_NAMEDCOLOR;
> 995: default -> throw new IllegalArgumentException("Unknown device class");
This will expand the line out of 80 chars per line
src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1176:
> 1174: return true;
> 1175: }
> 1176: default -> throw new IllegalArgumentException("Unknown Rendering Intent");
how it is handled by the lcms library? don't we need to ignore unknown intents(and other parameters) and lets lcms decide what to do?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23044#discussion_r1913684990
PR Review Comment: https://git.openjdk.org/jdk/pull/23044#discussion_r1913682136
PR Review Comment: https://git.openjdk.org/jdk/pull/23044#discussion_r1913688450
More information about the client-libs-dev
mailing list