RFR: 8241370: Crash in JPEGImageLoader after fix for JDK-8212034

[Kevin Rushforth]

On Tue, 31 Mar 2020 05:33:35 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

> This is a regression of [JDK-8212034](https://bugs.openjdk.java.net/browse/JDK-8212034).
> When image is loaded in WebView usinga url, WebView attempts to load a image frames with partial image data. This was
> implemented under, JDK-8153148 -> WCImageDecoderImpl.addImageData() -> calls loadFrames() with partial image data.
> 
> Call to jpeg_read_header() may fail when the partial image data has incomplete header information.
> 
> In the given case the jpeg_read_header() call fails and code execution flow enters the 'if
> (setjmp(jerr->setjmp_buffer)) {}' block and results in call to disposeIIO(env, data);, which in turn calls
> imageio_dispose. This will free cinfo->err and set it to NULL, and the subsequent call to (*cinfo->err->format_message)
> crashes.  Verified All test run, Sanity tests with Ensemble app and Tested different web pages. Added a test, The test
> passes with fix and causes a native crash without the fix.

The fix looks good. I can also confirm that the test fails (crashes) without your fix and passes with your fix.

The new LoadCorruptJPEGTest.java test doesn't use Robot, so it should be moved to a package not underneath
`test.robot`. For consistency with similar tests in `javafx.graphics`, I recommend `test.com.sun.javafx.iio`.

I left a few other comments on the test.

tests/system/src/test/java/test/robot/javafx/iio/LoadCorruptJPEGTest.java line 64:

> 63:         Util.runAndWait(() -> {
> 64:             URL resource = this.getClass().getResource("corrupt.jpg");
> 65:             FileInputStream input = null;

Minor: It's generally preferred to use `LoadCorruptJPEGTest.class` rather than the `getClass()` method call.

tests/system/src/test/java/test/robot/javafx/iio/LoadCorruptJPEGTest.java line 69:

> 68:             } catch (FileNotFoundException e) {
> 69:                 e.printStackTrace();
> 70:             }

The `FileNotFoundException` should cause the test to fail. Rather than doing a try / catch, I recommend adding `throws
Exception` to the test method.

tests/system/src/test/java/test/robot/javafx/iio/LoadCorruptJPEGTest.java line 88:

> 87:             });
> 88:             stage.setAlwaysOnTop(true);
> 89:             stage.show();

This call is not needed.

tests/system/src/test/java/test/robot/javafx/iio/LoadCorruptJPEGTest.java line 116:

> 115:
> 116:     private static void waitForLatch(CountDownLatch latch, int seconds, String msg) {
> 117:         try {

This method is unused and can be removed.

-------------

PR: https://git.openjdk.java.net/jfx/pull/154