Hello Jay
Good day to you.
I don't think it's possible to fix this issue despite having intermediate "
long " variable to hold intermediate bits per pixel.
Here is a simple math that considers the worst case scenario with max values:
. As per the PNG specification, the maximum permissible width, number of bands,
bit-depth are as follows-
max_width : (2 ^ 31) - 1 = 2147483647
max_input_bands = 4
max_bit_depth = 16 (2 Bytes)
. As per the logic proposed in the fix, the intermediate bits per row would fit
into a "long" variable but bytes per pixel would not fit into "int" variable.
// Fits into "long" data type
max_bits_per_row = max_width * max_input_bands * max_bit_depth
= 2147483647 x 4 x 16
= 137438953408
// Does not fit into "int" data type
max_bytes_per_row = max_bits_per_row + 7 / 8
= 17179869176
= (0x 3FFFFFFF8 - Goes beyond
4B boundary)
// Upon division by 2 for 16-bit images
max_elts_per_row = max_bytes_per_row / 2
= 8589934588
= (0x 1FFFFFFFC - Goes beyond 4B
boundary)
. If we intend to store bytes per row (and eltsPerRow which is scanline stride)
in a "long" variable, the same cannot be sent to Raster APIs because the APIs
accept "int" type for scanline stride in arguments list.
Going by the Raster APIs used in PNGImageReader, a proper fix would require
changes to its APIs as well to handle such large scanline stride values.
Thank you
Have a good day
Prahalad N.
----- Original Message -----
From: Jayathirth D V
Sent: Thursday, December 14, 2017 1:48 PM
To: 2d-dev
Subject: [OpenJDK 2D-Dev] RFR JDK-8191174: PngReader throws
IllegalArgumentException because ScanlineStride calculation logic is not proper
Hello All,
Please review the following fix in JDK :
Bug : https://bugs.openjdk.java.net/browse/JDK-8191174
Webrev : http://cr.openjdk.java.net/~jdv/8191174/webrev.00/
Issue : When we try to read PNG image with large width we throw undocumented
IllegalArgumentException with message "Pixel stride times width must be less
than or equal to the scanline stride".
Exception in thread "main" java.lang.IllegalArgumentException: Pixel stride
times width must be less than or equal to the scanline stride
at
java.desktop/java.awt.image.PixelInterleavedSampleModel.<init>(PixelInterleavedSampleModel.java:101)
at
java.desktop/java.awt.image.Raster.createInterleavedRaster(Raster.java:642)
at
java.desktop/com.sun.imageio.plugins.png.PNGImageReader.createRaster(PNGImageReader.java:974)
at
java.desktop/com.sun.imageio.plugins.png.PNGImageReader.decodePass(PNGImageReader.java:1099)
at
java.desktop/com.sun.imageio.plugins.png.PNGImageReader.decodeImage(PNGImageReader.java:1295)
at
java.desktop/com.sun.imageio.plugins.png.PNGImageReader.readImage(PNGImageReader.java:1420)
at
java.desktop/com.sun.imageio.plugins.png.PNGImageReader.read(PNGImageReader.java:1699)
at java.desktop/javax.imageio.ImageIO.read(ImageIO.java:1468)
at java.desktop/javax.imageio.ImageIO.read(ImageIO.java:1363)
at
PngReaderLargeWidthStrideTest.main(PngReaderLargeWidthStrideTest.java:50)
Root cause : We use large width present in IHDR and calculate elements per row
which is passed as scanlinestride for creating the required raster and its
corresponding sample model. When the call reaches creation of
PixelInterleavedSampleModel it checks the condition of (pixelStride * w) >
scanlineStride. Since in our case we pass this condition it throws
IllegalArgumentException. We have invalid scanlineStride value because when we
calculate elements per row/bytes per row value in PNGImageReader the integer
variable buffer is overflowed and we maintain invalid value for bytesPerRow.
Solution : We can maintain the intermediate bitsPerRow value in long datatype
and calculate bytesPerRow using the long value and then typecast it to int
bytesPerRow variable. By doing this we will maintain the required
scanlineStride/eltsPerRow value properly. After this solution we will throw
proper IIOException because of changes present in JDK-8190332 and not the
undocumented IllegalArgumentException.
Thanks,
Jay
