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> 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 >> <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 >>> <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 >>> <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