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

Saad Mohammad smohammad at redhat.com
Fri Nov 9 12:19:43 PST 2012


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>


-- 
Cheers,
Saad Mohammad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-0-4.patch
Type: text/x-patch
Size: 4686 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121109/8352f6e4/fix-0-4.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unittests0-4.patch
Type: text/x-patch
Size: 14908 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121109/8352f6e4/unittests0-4.patch 


More information about the distro-pkg-dev mailing list