[icedtea-web] RFC: parse jnlps containing component-desc as well as application-desc elements

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2010-10-29 11:39]:
> On 10/29/2010 11:20 AM, Deepak Bhole wrote:
> >* Omair Majid<omajid at redhat.com>  [2010-10-28 17:09]:
> >>On 10/27/2010 03:14 PM, Omair Majid wrote:
> >>>Hi,
> >>>
> >>>Some JNLPs such as space weasel [1] contain a "component-desc" element
> >>>along with an "application-desc" element. Netx current fails trying to
> >>>run such an application. The parser error says that having multiple
> >>>*-desc elements is not supported. In the case of "component-desc" this
> >>>does not make much sense. "application-desc", "applet-desc" and
> >>>"installer-desc" describe what should be done with this jnlp file;
> >>>"component-desc" carries no such meaning, so it should be ok to have a
> >>>"component-desc" along with "application-desc" in the same JNLP file.
> >>>The attached patch fixes this.
> >>>
> >>
> >>A very slightly updated patch attached. Instead of using recursion
> >>(and always returning null - my mistake) in getComponent, return a
> >>new ComponentDesc. As far as I know nothing in netx actually uses
> >>ComponentDesc - applications that loaded components/extensions
> >>worked before this patch, work with the patch I posted yesterday and
> >>work with this latest patch.
> >>
> >>>ChangeLog:
> >>>2010-10-27 Omair Majid<omajid at redhat.com>
> >>>
> >>>* netx/net/sourceforge/jnlp/JNLPFile.java: Add component.
> >>>(getLaunchInfo): Modify javadoc to indicate that it does not return
> >>>the ComponentDesc.
> >>>(getComponent): Return component instead of launchType.
> >>>(isComponent): Check if component is not null.
> >>>(parse): Find and set component.
> >>>* netx/net/sourceforge/jnlp/Parser.java
> >>>(getLauncher): Remove all checks for component-desc. Allow having
> >>>none of application-desc, applet-desc and installer-desc.
> >>>(getComponent): Check for more than one component-desc element.
> >>>Read and parse the component-desc.
> >>>
> >>>Any thoughts or comments?
> >>
> >>Anyone?
> >>
> >>>[1] http://spaceweasel.googlecode.com/svn/wiki/weasel.jnlp
> >>>
> >>
> >>Thanks,
> >>Omair
> >
> >>diff -r 5566a5487109 netx/net/sourceforge/jnlp/JNLPFile.java
> >>--- a/netx/net/sourceforge/jnlp/JNLPFile.java	Wed Oct 27 15:55:55 2010 -0700
> >>+++ b/netx/net/sourceforge/jnlp/JNLPFile.java	Thu Oct 28 16:49:24 2010 -0400
> >>@@ -95,6 +95,9 @@
> >>      /** the application description */
> >>      protected Object launchType;
> >>
> >>+    /** the component description */
> >>+    protected ComponentDesc component;
> >>+
> >>      /** the security descriptor */
> >>      protected SecurityDesc security;
> >>
> >>@@ -402,7 +405,7 @@
> >>
> >>      /**
> >>       * Returns an object of one of the following types: AppletDesc,
> >>-     * ApplicationDesc, InstallerDesc, and ComponentDesc.
> >>+     * ApplicationDesc and InstallerDesc
> >>       */
> >>      public Object getLaunchInfo() {
> >>          return launchType;
> >>@@ -441,7 +444,7 @@
> >>          if (!isComponent())
> >>              throw new UnsupportedOperationException(R("JNotComponent"));
> >>
> >>-        return (ComponentDesc) launchType;
> >>+        return component;
> >>      }
> >>
> >>      /**
> >>@@ -474,7 +477,7 @@
> >>       * Returns whether the lauch descriptor describes a Component.
> >>       */
> >>      public boolean isComponent() {
> >>-        return launchType instanceof ComponentDesc;
> >>+        return component != null;
> >>      }
> >>
> >>      /**
> >>@@ -574,6 +577,7 @@
> >>              update = parser.getUpdate(root);
> >>              resources = parser.getResources(root, false); // false == not a j2se/java resources section
> >>              launchType = parser.getLauncher(root);
> >>+            component = parser.getComponent(root);
> >>              security = parser.getSecurity(root);
> >>          }
> >>          catch (ParseException ex) {
> >>diff -r 5566a5487109 netx/net/sourceforge/jnlp/Parser.java
> >>--- a/netx/net/sourceforge/jnlp/Parser.java	Wed Oct 27 15:55:55 2010 -0700
> >>+++ b/netx/net/sourceforge/jnlp/Parser.java	Thu Oct 28 16:49:24 2010 -0400
> >>@@ -598,16 +598,15 @@
> >>
> >>      /**
> >>       * Returns the launch descriptor element, either AppletDesc,
> >>-     * ApplicationDesc, ComponentDesc, or InstallerDesc.
> >>+     * ApplicationDesc, or InstallerDesc.
> >>       *
> >>       * @param parent the parent node
> >>       * @throws ParseException if the JNLP file is invalid
> >>       */
> >>      public Object getLauncher(Node parent) throws ParseException {
> >>          // check for other than one application type
> >>-        if (1 != getChildNodes(parent, "applet-desc").length
> >>+        if (1<  getChildNodes(parent, "applet-desc").length
> >>              + getChildNodes(parent, "application-desc").length
> >>-            + getChildNodes(parent, "component-desc").length
> >>              + getChildNodes(parent, "installer-desc").length)
> >>              throw new ParseException(R("PTwoDescriptors"));
> >
> >
> >Does the spec allow a jnlp without any of the above descriptors?
> >
> 
> Yes. A JNLP extension can have just the <component-desc/> element.
> For example, take a look at the files at
> http://github.com/sgothel/jogl/tree/HEAD/jnlp-files. Of course, a
> file without any of the above elements can not be run directly; it
> must be used as an extension. If a user tries to run such a file
> directly, net.sourceforge.jnlp.Launcher will throw an exception.
> 

Ah okay. Fair enough. Looks fine to me. OK for head.

Deepak

> Thanks,
> Omair
> 
> >>
> >>@@ -619,8 +618,6 @@
> >>                  return getApplet(child);
> >>              if ("application-desc".equals(name))
> >>                  return getApplication(child);
> >>-            if ("component-desc".equals(name))
> >>-                return getComponent(child);
> >>              if ("installer-desc".equals(name))
> >>                  return getInstaller(child);
> >>
> >>@@ -693,8 +690,23 @@
> >>      /**
> >>       * Returns the component descriptor.
> >>       */
> >>-    public ComponentDesc getComponent(Node node) {
> >>-        return new ComponentDesc();
> >>+    public ComponentDesc getComponent(Node parent) throws ParseException {
> >>+
> >>+        if (1<  getChildNodes(parent, "component-desc").length) {
> >>+            throw new ParseException(R("PTwoDescriptors"));
> >>+        }
> >>+
> >>+        Node child = parent.getFirstChild();
> >>+        while (child != null) {
> >>+            String name = child.getNodeName();
> >>+
> >>+            if ("component-desc".equals(name))
> >>+                return new ComponentDesc();
> >>+
> >>+            child = child.getNextSibling();
> >>+        }
> >>+
> >>+        return null;
> >>      }
> >>
> >>      /**
> >
>