On Mon, 5 Dec 2022 19:59:45 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix indentations and merge short lines
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java
>  line 601:
> 
>> 599:             }
>> 600:             args.add("--" + k + "=" + (v != null ? v : ""));
>> 601:             idx++;
> 
> I suspect there might be a deviation from original intent that had happened 
> earlier.  The original idea seems to be of capturing IOException and sending 
> some diagnostic output to stderr, then continuing.  I am guessing here, but 
> perhaps at some point in the past `Base64.getDecoder().decode()` replaced 
> whatever was there before - and instead of throwing `IOException` it now 
> throws `IllegalArgumentException` if things cannot be parsed.
> 
> So the try-catch block should remain, catching an `IllegalArgumentException` 
> instead.
> 
> Perhaps we ought to revert these changes and file a follow-up bug?
> 
> What do you think?

The code was already broken (since 2014 apparently when the new Base64 decoder 
was used).  Had the author turned on these handy warnings, they would have 
noticed that something was amiss.  Now they unwittingly changed the behavior.

Personally, I think the current code is an improvement.  The catch block was 
already unreachable since 2014, and the `IllegalArgumentException` has been 
bubbling up and terminating the program since then (it's called from 
`launchApplication`).

Whether a follow-up bug is needed depends on whether we want to restore the 
original behavior: log an exception, ignore parsing the rest of the parameters 
and continue without them...  I think we shouldn't.

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

PR: https://git.openjdk.org/jfx/pull/960

Reply via email to