[OpenJDK 2D-Dev] [15] RFR JDK-8242557: Add length limit for strings in PNGImageWriter

[Jayathirth D v]

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.sadhukhan at 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.sadhukhan at oracle.com <mailto:prasanta.sadhukhan at 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
>> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20200430/2a9ac644/attachment.htm>