[RFC][icedtea-web]: Fix PR1166 - Embedded JNLP File is not supported in applet tag
Adam Domurad
adomurad at redhat.com
Tue Oct 2 12:47:50 PDT 2012
Re-sending on-list.
On 09/28/2012 04:11 PM, Saad Mohammad wrote:
> Hello,
>
> The reproducer for this bug is attached. I also found a small bug with embedded
> jnlps that use "." as its codebase, therefore I attached an updated bug fix that
> resolves this issue.(Added try/catch in Parser)
>
> Thanks.
>
> PR1166 <http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1166>
>
> ==============================================================================
> Bug Fix Changelog:
>
> 2012-09-28 Saad Mohammad <smohammad at redhat.com>
>
> Fix PR1166: Embedded JNLP File is not supported in applet tag.
> * 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.
>
> ==============================================================================
> Reproducer Changelog:
>
> 2012-09-28 Saad Mohammad <smohammad at redhat.com>
>
> Added reproducer for PR1166.
> * tests/reproducers/simple/EmbeddedJnlpInApplet/resources/EmbeddedJnlp.jnlp:
> Launching jnlp file that is used by jnlp_href in applet tag
> *
> tests/reproducers/simple/EmbeddedJnlpInApplet/resources/EmbeddedJnlpInAppletNoCodebase.html:
> Applet with an embedded jnlp file with no codebase specified
> *
> tests/reproducers/simple/EmbeddedJnlpInApplet/resources/EmbeddedJnlpInAppletWithDotCodebase.html:
> Applet with an embedded jnlp file with codebase set as a 'dot'
> * tests/reproducers/simple/EmbeddedJnlpInApplet/resources/JnlpInApplet.html:
> Applet with jnlp_href file.
> * tests/reproducers/simple/EmbeddedJnlpInApplet/srcs/EmbeddedJnlp.java:
> Simple class that outputs strings.
> *
> tests/reproducers/simple/EmbeddedJnlpInApplet/testcases/EmbeddedJnlpInAppletTest.java:
> Testcase that tests embedded jnlps in html pages.
>
> ==============================================================================
>
Hey Saad thanks for the reproducer & fix!
Comments on the reproducer to start off:
Overall it looks like a good reproducer, but I was unable to get it to
pass by applying your patch ? It could be a fault on my end, but they
generally did not get to the 'applet destroyed' message. They seemed OK
when tested manually.
> diff --git a/tests/reproducers/simple/EmbeddedJnlpInApplet/resources/EmbeddedJnlp.jnlp b/tests/reproducers/simple/EmbeddedJnlpInApplet/resources/EmbeddedJnlp.jnlp
> new file mode 100644
> --- /dev/null
> +++ b/tests/reproducers/simple/EmbeddedJnlpInApplet/resources/EmbeddedJnlp.jnlp
> @@ -0,0 +1,61 @@
> +<!--
> +
> +This file is part of IcedTea.
> +
> +IcedTea is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 2, or (at your option)
> +any later version.
> +
> +IcedTea is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with IcedTea; see the file COPYING. If not, write to the
> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +02110-1301 USA.
> +
> +Linking this library statically or dynamically with other modules is
> +making a combined work based on this library. Thus, the terms and
> +conditions of the GNU General Public License cover the whole
> +combination.
> +
> +As a special exception, the copyright holders of this library give you
> +permission to link this library with independent modules to produce an
> +executable, regardless of the license terms of these independent
> +modules, and to copy and distribute the resulting executable under
> +terms of your choice, provided that you also meet, for each linked
> +independent module, the terms and conditions of the license of that
> +module. An independent module is a module which is not derived from
> +or based on this library. If you modify this library, you may extend
> +this exception to your version of the library, but you are not
> +obligated to do so. If you do not wish to do so, delete this
> +exception statement from your version.
> +
> + -->
> +<?xml version="1.0" encoding="utf-8"?>
> +<jnlp spec="1.0" href="EmbeddedJnlp.jnlp">
> + <information>
> + <title>SignedAppletTest</title>
Title & description here seem inaccurate
> + <vendor>IcedTea</vendor>
> + <homepage href="http://icedtea.classpath.org/wiki/IcedTea-Web#Testing_IcedTea-Web "/>
> + <description>SignedAppletTest</description>
> + <offline/>
> + </information>
> + <resources>
> + <j2se version="1.4+"/>
> + <jar href="EmbeddedJnlpInApplet.jar"/>
> + </resources>
> + <applet-desc
> + documentBase="."
> + name="EmbeddedJnlp"
> + main-class="EmbeddedJnlp"
> + width="100"
> + height="100">
> + </applet-desc>
> +</jnlp>
> +
> +
> +</applet-desc>
> [ ... snipped ... ]
> +
> +import java.applet.Applet;
> +
> +public class EmbeddedJnlp extends Applet {
> +
> + private static final long serialVersionUID = -2473695081641587179L;
If this isn't actually meant to be serializable, I'd prefer a
SuppressWarning annotation if the warning bugs you.
> +
> + @Override
> + public void init() {
> + System.out.println("Starting EmbeddedJnlp Class");
> + }
> +
> + @Override
> + public void start() {
> + System.out.println("EmbeddedJnlp was started");
> + }
> +
> + @Override
> + public void stop() {
> + System.out.println("Stopping EmbeddedJnlp");
> + }
> +
> + @Override
> + public void destroy() {
> + System.out.println("Destroying EmbeddedJnlp");
If you are only searching for one string, it's a little simpler to just
have one message, "*** APPLET FINISHED ***" (which is now the 'standard'
applet finishing string.) But, as you will.
> + }
> +}
> diff --git a/tests/reproducers/simple/EmbeddedJnlpInApplet/testcases/EmbeddedJnlpInAppletTest.java b/tests/reproducers/simple/EmbeddedJnlpInApplet/testcases/EmbeddedJnlpInAppletTest.java
> new file mode 100644
> --- /dev/null
> +++ b/tests/reproducers/simple/EmbeddedJnlpInApplet/testcases/EmbeddedJnlpInAppletTest.java
> @@ -0,0 +1,71 @@
> +/* EmbeddedJnlpInAppletTest.java
> +Copyright (C) 2012 Red Hat, Inc.
> +
> +This file is part of IcedTea.
> +
> +IcedTea is free software; you can redistribute it and/or
> +modify it under the terms of the GNU General Public License as published by
> +the Free Software Foundation, version 2.
> +
> +IcedTea is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with IcedTea; see the file COPYING. If not, write to
> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +02110-1301 USA.
> +
> +Linking this library statically or dynamically with other modules is
> +making a combined work based on this library. Thus, the terms and
> +conditions of the GNU General Public License cover the whole
> +combination.
> +
> +As a special exception, the copyright holders of this library give you
> +permission to link this library with independent modules to produce an
> +executable, regardless of the license terms of these independent
> +modules, and to copy and distribute the resulting executable under
> +terms of your choice, provided that you also meet, for each linked
> +independent module, the terms and conditions of the license of that
> +module. An independent module is a module which is not derived from
> +or based on this library. If you modify this library, you may extend
> +this exception to your version of the library, but you are not
> +obligated to do so. If you do not wish to do so, delete this
> +exception statement from your version.
> + */
> +
> +import junit.framework.Assert;
> +import net.sourceforge.jnlp.ProcessResult;
> +import net.sourceforge.jnlp.browsertesting.BrowserTest;
> +import net.sourceforge.jnlp.browsertesting.Browsers;
> +import net.sourceforge.jnlp.annotations.TestInBrowsers;
> +import org.junit.Test;
> +
> +public class EmbeddedJnlpInAppletTest extends BrowserTest {
> +
> + final String s = "Destroying EmbeddedJnlp";
'static' here, to nit.
> +
> + @Test
> + @TestInBrowsers(testIn = { Browsers.one })
> + public void JnlpInApplet() throws Exception {
> + ProcessResult pr = server.executeBrowser("/JnlpInApplet.html");
If you do my suggestion of checking for the (new) 'standard' applet
closing message, you can also simply change this call to
server.executeBrowser("/JnlpInApplet.html", AutoClose.CLOSE_ON_CORRECT_END);
And it will automagically close within reasonable time.
> + Assert.assertTrue("EmbeddedJnlp.class was not correctly launched in JnlpInApplet.html: " + pr.stdout + pr.stderr, pr.stdout.contains(s));
> + }
> +
> + @Test
> + @TestInBrowsers(testIn = { Browsers.one })
> + public void EmbeddedJnlpInAppletWithADotAsCodebase() throws Exception {
> + ProcessResult pr = server.executeBrowser("/EmbeddedJnlpInAppletWithDotCodebase.html");
As above, also consider this for any future executeBrowser calls, as it
will greatly speed up the reproducer system if applied thoroughly.
> + Assert.assertTrue("EmbeddedJnlp.class was not correctly launched in EmbeddedJnlpInAppletWithDotCodebase.html: " + pr.stdout + pr.stderr,
> + pr.stdout.contains(s));
> + }
> +
> + @Test
> + @TestInBrowsers(testIn = { Browsers.one })
> + public void EmbeddedJnlpInAppletWithNoCodebase() throws Exception {
> + ProcessResult pr = server.executeBrowser("/EmbeddedJnlpInAppletWithNoCodebase.html");
As above.
> + Assert.assertTrue("EmbeddedJnlp.class was not correctly launched in EmbeddedJnlpInAppletWithDotCodebase.html: " + pr.stdout + pr.stderr,
> + pr.stdout.contains(s));
> + }
> +}
Cheers,
- Adam
More information about the distro-pkg-dev
mailing list