[RFC][icedtea-web]: Fix PR1166 - Embedded JNLP File is not supported in applet tag

[Adam Domurad]

On 11/09/2012 03:19 PM, Saad Mohammad wrote:
> Hi Adam,
>
> Thanks for taking a look over the fix and unit test patches.
> Updated patches are attached with few comments below.
>
> Changelog has not changed from previous post but I will still include it below:
>
> ==============================================================================
> BUG FIX:
>
> 2012-10-03  Saad Mohammad  <smohammad at redhat.com>
>
> 	Fix PR1166: Embedded JNLP File is not supported in applet tag.
> 	* configure.ac: Checks for sun.misc.BASE64Encoder and
> 	sun.misc.BASE64Decoder
> 	* netx/net/sourceforge/jnlp/JNLPFile.java (JNLPFile):
> 	New constructor which accepts inputstream of jnlp file and a
> 	specified codebase.
> 	* netx/net/sourceforge/jnlp/Parser.java (Parser): If parsing of
> 	codebase fails, it will overwrite the codebase with the one passed
> 	in through parameters.
> 	* netx/net/sourceforge/jnlp/PluginBridge.java:
> 	(PluginBridge) Supports embedded jnlp file.
> 	(decodeBase64String) Decodes Base64 strings to byte array.
>
> ==============================================================================
> UNIT TESTS:
>
> 2012-10-03  Saad Mohammad  <smohammad at redhat.com>
>
> 	* tests/netx/unit/net/sourceforge/jnlp/JNLPFileTest.java:
> 	Tests the JNLPFile constructor that accepts an InputStream and an alternative
> codebase.
> 	* tests/netx/unit/net/sourceforge/jnlp/ParserTest.java:
> 	Tests if the constructor handles the alternative codebase parameter correctly.
> 	* tests/netx/unit/net/sourceforge/jnlp/PluginBridgeTest.java:
> 	Tests if BASE64 strings are decoded correctly and if PluginBridge is
> constructed with an
> 	embedded jnlp.
>
> ==============================================================================
>
>
> [...snip..]
>
>> Review of bugfix & unit test:
>>
>>> diff --git a/configure.ac b/configure.ac
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -77,6 +77,8 @@
>>>   IT_CHECK_FOR_CLASS(SUN_NET_WWW_PROTOCOL_JAR_URLJARFILECALLBACK,
>>> [sun.net.www.protocol.jar.URLJarFileCallBack])
>>>   IT_CHECK_FOR_CLASS(SUN_AWT_X11_XEMBEDDEDFRAME, [sun.awt.X11.XEmbeddedFrame])
>>>   IT_CHECK_FOR_CLASS(SUN_MISC_REF, [sun.misc.Ref])
>>> +IT_CHECK_FOR_CLASS(SUN_MISC_BASE64ENCODER, [sun.misc.BASE64Encoder])
> I have removed this check after Omair's suggestion to use
> net.sourceforge.jnlp.util.replacements.BASE64Encoder instead
> of sun.misc.Base64Encoder.
>
>>> +IT_CHECK_FOR_CLASS(SUN_MISC_BASE64DECODER, [sun.misc.BASE64Decoder])
>> It's neither here nor there, but I wonder if these classes will still be
>> available here once http://openjdk.java.net/jeps/135 rolls around.
> I am not too sure what will happen to sun.misc.BASE64Decoder after that is
> implemented. Perhaps the class will be marked as deprecated and at that point we
> can change this if there are any issues. But as of right now, I don't see any
> real problem to avoid using this class.
>
>>>   IT_CHECK_FOR_CLASS(COM_SUN_JNDI_TOOLKIT_URL_URLUTIL,
>>> [com.sun.jndi.toolkit.url.UrlUtil])
>>>   IT_CHECK_FOR_CLASS(SUN_APPLET_APPLETIMAGEREF, [sun.applet.AppletImageRef])
>>>   IT_CHECK_FOR_APPLETVIEWERPANEL_HOLE
> [...snip...]
>
>>> +
>>> +                if (jnlpFile.isApplet())
>>> +                    main = jnlpFile.getApplet().getMainClass();
>> Considering these two lines are new, could they safely be part of the
>> 'containsKey("jnlp_embedded") ' case ? It'd be a little clearer in my opinion.
>>
> I think this is fine IMO as these two lines of code are also used in the PR1189
> fix to determine the main class of a non-embedded jnlp file. As we previously
> discussed on IRC, the main-class within the jnlp file is _always_ chosen over
> the main class specified within the code attribute of the applet tag (if
> specified, of course).
>
> PR1189 patch (which you also reviewed):
> <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2012-October/020492.html>
>
>

Thanks for the patch updates! Reproducer + patch + unit tests look good 
to me now, all OK for HEAD. Thanks for sticking with it.

- Adam