On Fri, 21 May 2021 13:02:14 GMT, Jose Pereda <jper...@openjdk.org> wrote:
> This PR limits the `tableIndex` value, used by the LZWDecoder algorithm in > `GIFImageLoader2`, to avoid a potential AIOOB exception that happens on some > animated GIFs, to the maximum size of the tables used (4096). > > In some occasions loading an animated GIF like the one used in the included > test, doesn't throw such exception, because we `allow partially loaded > animated images` in `ImageStorage`, but only a few frames are loaded. > > In theory, greater values of such index would operate over completely full > tables, so there is no need to add new values in this case, and therefore, > there is no risk in limiting the value to 4096. > > This PR will prevent the exception and all the frames should load. The > included test passes now (and fails loading only 10 frames out of 44 without > the proposed fix). The fix seems safe enough, although should have a second reviewer. The test will need changes. modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java line 91: > 89: @Test > 90: public void testLoadAllFramesAnimatedGIF() throws > ImageStorageException { > 91: String path = > "https://upload.wikimedia.org/wikipedia/commons/2/2c/Rotating_earth_%28large%29.gif"; Except for specific network tests we don't want to load anything from the internet during a run. For one thing this will fail behind a firewall (without setting a proxy, which we don't want to require). For another, the content could change out from under us. The best solution would be to generate a synthetic image with properties that will cause it to fail (we can't use a preexisting image due to copyright issues). ------------- Changes requested by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/513