On Fri, 21 May 2021 13:02:14 GMT, Jose Pereda <[email protected]> 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