RFR: 8267314: Loading some animated GIFs fails with ArrayIndexOutOfBoundsException: Index 4096 out of bounds for length 4096

[Kevin Rushforth]

On Fri, 21 May 2021 13:02:14 GMT, Jose Pereda <jpereda at 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