Vulnerability in WaitForAsyncUtils

[Ortner]

Issue

There are several vulnerabilities in the WaitForAsyncUtils. Public methods require Exception handling, where it is not obvious.

The following code snippet serves as example:

try{
    Future<?> f=WaitForAsyncUtils.async(() -> {
        WaitForAsyncUtils.async(() -> {throw new SecurityException("Exception");});
    });
}catch(Exception e){e.printStackTrace();}

This code does not throw any exceptions.
If you do not call the get() method of the Future returned by async(), exceptions during execution will be masked in the callCallableAndSetFuture method. But when using the async() call I actually do not want to wait for the result.

I'm not completely done yet, as the Exception handling is not that easy to follow and different types of execution models with different kinds of exception handling are used (Runnables, callables with/without Executors...). But I think this is at least true for all the async..(...) methods. The waitForAsync...(...)Methods do check the exceptions later.

In my opinion this is dangerous, especially in the public API.

Current test case

The current test case is also invalid, the Exception is thrown by another method (see comment below).

@Test(timeout=1000)
    public void async_callable_with_exception() throws Exception {
        // given:
        Callable<Void> callable = () -> {
            throw new UnsupportedOperationException();
        };
        Future<Void> future = WaitForAsyncUtils.async(callable);

        // expect:
        thrown.expectCause(instanceOf(UnsupportedOperationException.class)); // <-- Validates after test
        WaitForAsyncUtils.waitFor(50, MILLISECONDS, future);  //<-- Exception is thrown here
    }

Possible relations

• GlassRobot uses this methods quite frequently without checking the return value (move mouse, release mouse, key press...)
• As exceptions may be masked, unpredicted behavior (e.g. concurrent modification exceptions may be masked), so this issue might be related to TestFX node lookup can cause javafx rendering issues #279

Proposal

I'm not completely done yet, but the simplest solution would be to remove the try catch block in callCallableAndSetFuture, if called from one of the async methods (extra method for async calls).