Hi Jay,
>> we will throw exception in case of reader also
But in the fix, only PNGWriter is changed. I did not see in existing
PNGReader any exception being thrown for > 79 bytes. I see in reader, we
have
private void parse_iCCP_chunk(int chunkLength) throws IOException {
String keyword = readNullTerminatedString("ISO-8859-1", 80);
int compressedProfileLength = chunkLength - keyword.length() - 2;
if (compressedProfileLength <= 0) {
throw new IIOException("iCCP chunk length is not proper");
}
so chunkLength can be say, 100, keyWord.lenth = 79 so
compressedProfileLength = 100-79-2=19 so we will not throw IOException.
Am I missing something?
Regards
Prasanta
On 30-Apr-20 3:23 PM, Jayathirth D v wrote:
Hi Prasanta,
I didnt say reader will decode more than 79 bytes but user might
expect more than 79 bytes since we allowed more than 79 bytes write.
While reading non-standard PNG chunks(having greater than 79 bytes
null terminated string) we will throw exception in case of reader
also. Its a stream, we will not know the length and we rely completely
on null termination and we expect it to be spec complaint and not more
than 79 bytes.
Thanks,
Jay
On 30-Apr-2020, at 2:51 PM, Prasanta Sadhukhan
<prasanta.sadhuk...@oracle.com
<mailto:prasanta.sadhuk...@oracle.com>> wrote:
Hi Jay,
But why reader will read more than 79 bytes from the chunk..It should
also limit it's reading to 1st 79 bytes (if the chunks we get from
non-oracle non-standard PNG writer is more than 79bytes) as per spec, no?
Regards
Prasanta
On 30-Apr-20 2:01 PM, Jayathirth D v wrote:
Hi Prasanta,
Thanks for the review.
If we consume only 79 bytes and continue writing the image, it will
lead to unexpected behaviour when user tries to read the image
expecting longer than 79 string data. Our reader is also tightly
spec compliant for null terminated strings after JDK-8195841
<https://bugs.openjdk.java.net/browse/JDK-8195841> and JDK-8191023
<https://bugs.openjdk.java.net/browse/JDK-8191023>. Its better to be
spec compliant than leaving things open for interpretation.
Regards,
Jay
On 29-Apr-2020, at 1:22 PM, Prasanta Sadhukhan
<prasanta.sadhuk...@oracle.com
<mailto:prasanta.sadhuk...@oracle.com>> wrote:
Hi Jay,
Looks good to me . Although I have a question which is, instead of
throwing Exception, can we proceed with 1st 79 bytes of these
chunks and continue?
Regards
Prasanta
On 27-Apr-20 10:51 PM, Philip Race wrote:
I reviewed http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html
and I think you covered all the cases.
I also reviewed the reader and it seems we already check only up
to 80 chars there.
I note we assume the same max length of 80 for the language tag
for iTxt.
The spec. doesn't specify a limit but I think 80 is more than
generous here.
+1
-phil.
On 4/27/20, 9:59 AM, Jayathirth D v wrote:
Hello All,
Please review the following fix for JDK 15:
Bug : https://bugs.openjdk.java.net/browse/JDK-8242557
Webrev : http://cr.openjdk.java.net/~jdv/8242557/webrev.00/
<http://cr.openjdk.java.net/%7Ejdv/8242557/webrev.00/>
Issue : PNG specification restricts length of strings in some
chunks to
79(http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html )
excluding null termination. But our writer implementation has no
such check.
Solution : Add checks in different chunks where there must be
restrictions.
Thanks,
Jay