RFR: 8282577: ICC_Profile.setData(int, byte[]) invalidates the profile [v3]

Wed Mar 9 22:23:41 UTC 2022

On Tue, 8 Mar 2022 04:09:23 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

> The current bug looks similar to this one https://bugs.openjdk.java.net/browse/JDK-8272865 I tried to fix it the same way but I was not sure if it is safe to ignore the ReadWriteProfileTest.java or not, since it has five related bugids.

I missed that bug. There's overlap but this bug is mainly about an unusable profile being returned.
The "nicety" that getData() returns what setData() set is much less important.

However I have been pondering that test.
If by the 5 related bug IDs you mean those in the @bug tag
 * @bug 6476665 6523403 6733501 7042594 7043064

only the last of them says anything clearly related where the eval from 2013 says
--

The only exception is ReadWriteProfileTest. This test relies on an assumption, 
that we read a data for a tag in the exactly same form as it was written. 
However, this assumption is not correct in case of lcms, because this 
library may perform some sort of modifications on the tag data. 
For example, plain text tags can be promoted to multilinguage tags, 
color points can be normalized and etc. To get this test working, we ether 
have to upgrade the tag comparison routines in the test, or force 
"as is" tag injection in lcms. The second option seems to be a bit dangerous 
to me because it potentially open doors for an injection of malformed tags 
into a profile. 
---

Actually it is not clear to me that modifications to that effect were made to the test by that bug fix.
I only see it skipping Kodak tags : http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/a07c907a82b5

But it means even back then there was acknowledgement that getData() may not return what setData() set
even if the data were accepted. And plain (ascii) text tags being promoted to multi-lingual tags is an example of
what I saw. I've not dug into locating all versions of the ICC spec but an old copy makes no mention of the
multi-lingual support so I can imagine that LittleCMS is "bringing up to date" a tag that was encoded into
a profile before that support was present.

Source comments in LittleCMS on cmsReadRawTag seemed to suggest it would always cook the tag
so as to make sure it returned consistent results. But I checked with the LCMS maintainer and he
said that was out-dated and updated the source comments. So it is definite that we can't rely on
these being consistent.

So I have been wondering if the test is even valid and worth saving.

The spec doesn't say anything one way or another
https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/java/awt/color/ICC_Profile.html#getData()
so I am sure a case could be made either way as to what it intended. 

One way to "save" the test is that after doing the validation which calls cmsReadTag() and get an "OK"
we discard this profile and create ANOTHER copy which we return to the app. Since nothing will have
cooked that new profile data it will get back what it set. Unless something else cooks it or LittleCMS behaviour changes.

I'm up for doing that here and then the test will pass again without changes.

> The current bug looks similar to this one https://bugs.openjdk.java.net/browse/JDK-8272865 I tried to fix it the same way but I was not sure if it is safe to ignore the ReadWriteProfileTest.java or not, since it has five related bugids.