[OpenJDK 2D-Dev] [10] RFR JDK-8191431: Reading multiple PNG images with unique IDAT chunk positions will cause IIOException

[Prahalad Kumar Narayanan]

Hello Jay

The change looks good.

Thanks
Have a good day

Prahalad

-----Original Message-----
From: Jayathirth D V 
Sent: Tuesday, November 21, 2017 9:01 AM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [10] RFR JDK-8191431: Reading multiple PNG images with unique IDAT chunk positions will cause IIOException

Hi Prahalad,

Thanks for the review.
I merged the two functions into one instead of moving BufferedImage code into the two functions as it reduces the LOC by good amount.

Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8191431/webrev.02/ 

Thanks,
Jay

-----Original Message-----
From: Prahalad Kumar Narayanan 
Sent: Tuesday, November 21, 2017 8:13 AM
To: Jayathirth D V; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [10] RFR JDK-8191431: Reading multiple PNG images with unique IDAT chunk positions will cause IIOException

Hello Jay

Good day to you. I looked into the modified test code.

The test file has two methods ( writeAndReadImageWithPalette & writeAndReadImageWithoutPalette ) with exactly the same implementation. 
    . Can we merge them into one method ? 
    . Another option would be to move the logic that prepares required buffered image within these methods.

  86     private static void writeAndReadImageWithoutPalette(BufferedImage image)
  87             throws IOException {
  88         File output = File.createTempFile("output", ".png");
  89         ImageInputStream stream = null;
  90         try {
  91             ImageIO.write(image, "png", output);
  92 
  93             stream = ImageIO.createImageInputStream(output);
  94             ImageReadParam param = PNG_READER.getDefaultReadParam();
  95             PNG_READER.setInput(stream, true, true);
  96             PNG_READER.read(0, param);
  97         } finally {
  98             if (stream != null) {
  99                 stream.close();
 100             }
 101             Files.delete(output.toPath());
 102         }
 103     }

 105     private static void writeAndReadImageWithPalette(BufferedImage image)
 106             throws IOException {
 107         File output = File.createTempFile("output", ".png");
 108         ImageInputStream stream = null;
 109         try {
 110             ImageIO.write(image, "png", output);
 111 
 112             stream = ImageIO.createImageInputStream(output);
 113             ImageReadParam param = PNG_READER.getDefaultReadParam();
 114             PNG_READER.setInput(stream, true, true);
 115             PNG_READER.read(0, param);
 116         } finally {
 117             if (stream != null) {
 118                 stream.close();
 119             }
 120             Files.delete(output.toPath());
 121         }
 122     }
 123 }

Thank you
Have a good day

Prahalad

-----Original Message-----
From: Jayathirth D V 
Sent: Monday, November 20, 2017 6:18 PM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [10] RFR JDK-8191431: Reading multiple PNG images with unique IDAT chunk positions will cause IIOException

Hi Prahalad,

Thanks for the review.
I have updated the test case.

Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8191431/webrev.01/ 

Thanks,
Jay

-----Original Message-----
From: Prahalad Kumar Narayanan 
Sent: Monday, November 20, 2017 5:25 PM
To: Jayathirth D V; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [10] RFR JDK-8191431: Reading multiple PNG images with unique IDAT chunk positions will cause IIOException

Hello Jay

The code change at PNGImageReader is correct.
But the test wouldn't delete temporary files when exception occurs.

Thank you
Have a good day

Prahalad

----- Original Message -----
From: Jayathirth D V 
Sent: Monday, November 20, 2017 4:34 PM
To: 2d-dev
Subject: [OpenJDK 2D-Dev] [10] RFR JDK-8191431: Reading multiple PNG images with unique IDAT chunk positions will cause IIOException

Hello All,

Please review the following fix in JDK10 :

Bug : https://bugs.openjdk.java.net/browse/JDK-8191431 
Webrev : http://cr.openjdk.java.net/~jdv/8191431/webrev.00/ 

Issue : When we try to read multiple PNG images with different IDAT chunk positions using the same PNGImageReader instance we get "IIOException: Error reading PNG image data".

Root cause : Issue is happening because of changes present in JDK-8164971.
                Under JDK-8164971 we have made changes such that imageStartPosition for IDAT chunk will be updated only once for a given PNGImageReader instance while reading metadata.
                case IDAT_TYPE:
                    // If chunk type is 'IDAT', we've reached the image data.
                    if (imageStartPosition == -1L) {
                        /*
                         * PNGs may contain multiple IDAT chunks containing
                         * a portion of image data. We store the position of
                         * the first IDAT chunk and continue with iteration
                         * of other chunks that follow image data.
                         */
                        imageStartPosition = stream.getStreamPosition() - 8;
                   }

                When we use same PNGImageReader instance to read another PNG image and if IDAT chunk position and length differs we will get IIOException.

Solution : We already have resetStreamSettings() method which we call when user tries to decode an image under ImageReader.setInput(). In resetStreamSettings() function if we initialize imageStartPosition to '-1L' it will resolve this issue.

Thanks,
Jay