<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#ffffff" text="#000000">
    On 06/27/2011 06:42 PM, Deepak Bhole wrote:
    <blockquote cite="mid:20110627224240.GL2952@redhat.com" type="cite">
      <pre wrap="">The indentation above is incorrect... it should follow the same format
as other entries in the ChangeLog. Also, not sure what the [2] means..
it shouldn't be there.
</pre>
    </blockquote>
    Not sure, why the changelog is out of format. Nor do I know what the
    [2] is for, but here is the same formatted changelog I sent in the
    first email =):<br>
    <font size="-1"><br>
      2011-06-23&nbsp; Saad Mohammad&nbsp; <a class="moz-txt-link-rfc2396E"
        href="mailto:smohammad@redhat.com">&lt;smohammad@redhat.com&gt;</a><br>
      <br>
      &nbsp;&nbsp;&nbsp; * netx/net/sourceforge/jnlp/JNLPVerify.java:<br>
      &nbsp;&nbsp;&nbsp; &nbsp; Created this class to compares signed JNLP file with the
      launching JNLP file.<br>
      &nbsp;&nbsp;&nbsp; &nbsp; It is able to compare both method of signing of a JNLP file:
      APPLICATION_TEMPLATE.JNLP<br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; and APPLICATION.JNLP<br>
      &nbsp;&nbsp;&nbsp; * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:<br>
      &nbsp;&nbsp;&nbsp; &nbsp; Added a custom exception: JNLPVerifyException. Also added
      JNLPVerifyException to methods<br>
      &nbsp;&nbsp;&nbsp; &nbsp; that throws the custom exception.<br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; (initializeResources): Checks if there is any signed JNLP
      file within the signed jars.<br>
      &nbsp;&nbsp;&nbsp;&nbsp; &nbsp; If signed JNLP file fails to match or fails to be verified,
      the application throws <br>
      &nbsp;&nbsp;&nbsp; &nbsp; a JNLPVerifyException.<br>
      &nbsp;&nbsp;&nbsp; * netx/net/sourceforge/jnlp/Node.java:<br>
      &nbsp;&nbsp;&nbsp; &nbsp; Created a method that retrieves the attribute names of the
      Node in a<br>
      &nbsp;&nbsp;&nbsp; &nbsp; private string [] member. The method returns the attribute
      names.<br>
      &nbsp;&nbsp;&nbsp; * netx/net/sourceforge/nanoxml/XMLElement.java:<br>
      &nbsp;&nbsp;&nbsp; &nbsp; (sanitizeInput): Changed parameter type of isr from
      InputStreamReader to Reader.<br>
      <br>
    </font>
    <blockquote cite="mid:20110627224240.GL2952@redhat.com" type="cite">
      <pre wrap="">
</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>References
<span class="moz-txt-citetags">&gt; </span>
<span class="moz-txt-citetags">&gt; </span>   1. <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://jcp.org/aboutJava/communityprocess/maintenance/jsr056/jnlp-7_0-changes.html">http://jcp.org/aboutJava/communityprocess/maintenance/jsr056/jnlp-7_0-changes.html</a>
<span class="moz-txt-citetags">&gt; </span>   2. <a moz-do-not-send="true" class="moz-txt-link-freetext" href="mailto:smohammad@redhat.com">mailto:smohammad@redhat.com</a>
</pre>
      </blockquote>
      <pre wrap="">...
</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>+    public static JNLPVerify getInstanceOf(Reader appTemplateFile, Reader launchJNLPFile,
<span class="moz-txt-citetags">&gt; </span>+            boolean isTemplate) throws Exception {
</pre>
      </blockquote>
      <pre wrap="">Jiri already pointed this out.. but throwing a generic Exception is a
bad idea because it forces callers to catch all exceptions when they may
not want to -- consider throwing a more restricted exception.

Also, getInstanceOf suggests this that is a singleton class which it is
not. The method name should be changed. Though frankly, I think this
method shouldn't exist at all. Let the caller create the class and
handle any exceptions from the constructor rather than having a wrapper
just to re-throw the exception.

</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>+        try {
<span class="moz-txt-citetags">&gt; </span>+            JNLPVerify ret = new JNLPVerify(appTemplateFile, launchJNLPFile, isTemplate);
<span class="moz-txt-citetags">&gt; </span>+            return ret;
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+        } catch (Exception e) {
<span class="moz-txt-citetags">&gt; </span>+            throw new Exception(
<span class="moz-txt-citetags">&gt; </span>+                    "Failed to create an instance of JNLPVerify with specified Readers: "
<span class="moz-txt-citetags">&gt; </span>+                            + e.getMessage());
<span class="moz-txt-citetags">&gt; </span>+        }
<span class="moz-txt-citetags">&gt; </span>+    }
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+    /**
<span class="moz-txt-citetags">&gt; </span>+     * Private constructor
<span class="moz-txt-citetags">&gt; </span>+     * 
<span class="moz-txt-citetags">&gt; </span>+     * @param appTemplate
<span class="moz-txt-citetags">&gt; </span>+     *            the reader stream of the signed APPLICATION.jnlp or
<span class="moz-txt-citetags">&gt; </span>+     *            APPLICATION_TEMPLATE.jnlp
<span class="moz-txt-citetags">&gt; </span>+     * @param launchJNLP
<span class="moz-txt-citetags">&gt; </span>+     *            the reader stream of the launching JNLP file
<span class="moz-txt-citetags">&gt; </span>+     * @param isTemplate
<span class="moz-txt-citetags">&gt; </span>+     *            a boolean that specifies if appTemplateFile is a template
<span class="moz-txt-citetags">&gt; </span>+     * 
<span class="moz-txt-citetags">&gt; </span>+     * @throws XMLParseException
<span class="moz-txt-citetags">&gt; </span>+     *             if appTemplate and launchJNLP could not be parsed
<span class="moz-txt-citetags">&gt; </span>+     * @throws IOException
<span class="moz-txt-citetags">&gt; </span>+     *             if PipedOutputStream or InputStreamReader failed to be
<span class="moz-txt-citetags">&gt; </span>+     *             created
<span class="moz-txt-citetags">&gt; </span>+     */
<span class="moz-txt-citetags">&gt; </span>+    private JNLPVerify(Reader appTemplate, Reader launchJNLP, boolean isTemplate)
<span class="moz-txt-citetags">&gt; </span>+            throws XMLParseException, IOException {
<span class="moz-txt-citetags">&gt; </span>+
</pre>
      </blockquote>
      <pre wrap="">This is not a singleton class nor is it a factory and all of the
constructor parameters are available to the caller, so there is no real
need for a private constructor.

...
</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+        if (appTemplate != null &amp;&amp; launchJNLP != null) {
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+            Node appTemplateNode = (Node) appTemplate;
<span class="moz-txt-citetags">&gt; </span>+            Node launchJNLPNode = (Node) launchJNLP; 
<span class="moz-txt-citetags">&gt; </span>+            
<span class="moz-txt-citetags">&gt; </span>+            //Store children of Node
<span class="moz-txt-citetags">&gt; </span>+            LinkedList&lt;Node&gt; appTemplateChild = new LinkedList&lt;Node&gt;(Arrays.asList(appTemplateNode
<span class="moz-txt-citetags">&gt; </span>+                    .getChildNodes()));
<span class="moz-txt-citetags">&gt; </span>+            LinkedList&lt;Node&gt; launchJNLPChild = new LinkedList&lt;Node&gt;(Arrays.asList(launchJNLPNode
<span class="moz-txt-citetags">&gt; </span>+                    .getChildNodes()));
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+            // Compare only if both Nodes have the same name, else return false
<span class="moz-txt-citetags">&gt; </span>+            if (appTemplateNode.getNodeName().equals(launchJNLPNode.getNodeName())) {
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                if (appTemplateChild.size() == launchJNLPChild.size()) { // Compare children
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                    int childLength = appTemplateChild.size();
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                    for (int i = 0; i &lt; childLength;) {
<span class="moz-txt-citetags">&gt; </span>+                        for (int j = 0; j &lt; childLength; j++) {
<span class="moz-txt-citetags">&gt; </span>+                            boolean isSame = compareNodes(appTemplateChild.get(i),
<span class="moz-txt-citetags">&gt; </span>+                                    launchJNLPChild.get(j));
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                            if (!isSame &amp;&amp; j == childLength - 1)
<span class="moz-txt-citetags">&gt; </span>+                                return false;
<span class="moz-txt-citetags">&gt; </span>+                            else if (isSame) { // If both child matches, remove them from the list of children
<span class="moz-txt-citetags">&gt; </span>+                                appTemplateChild.remove(i);
<span class="moz-txt-citetags">&gt; </span>+                                launchJNLPChild.remove(j);
<span class="moz-txt-citetags">&gt; </span>+                                --childLength;
<span class="moz-txt-citetags">&gt; </span>+                                break;
<span class="moz-txt-citetags">&gt; </span>+                            }
<span class="moz-txt-citetags">&gt; </span>+                        }
<span class="moz-txt-citetags">&gt; </span>+                    }
<span class="moz-txt-citetags">&gt; </span>+
</pre>
      </blockquote>
      <pre wrap="">
This logic seems more complex than it needs to be and is O(n<sup class="moz-txt-sup">2</sup>). You can
define a comparator and use Arrays.sort to sort the arrays that the
getChildNodes() methods return. Then you only need to iterate over the
array once -- resulting in O(nlogn) performance and much cleaner code.

...
</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>+            
<span class="moz-txt-citetags">&gt; </span>+            ArrayList &lt;String&gt; appTemplateAttributes = new ArrayList &lt;String&gt;(Arrays.asList(appTemplateNode.getAttributeNames()));
<span class="moz-txt-citetags">&gt; </span>+            ArrayList &lt;String&gt; launchJNLPAttributes = new ArrayList &lt;String&gt;(Arrays.asList(launchJNLPNode.getAttributeNames()));
<span class="moz-txt-citetags">&gt; </span>+
</pre>
      </blockquote>
      <pre wrap="">Same change as above can be made here.

...

</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>         
<span class="moz-txt-citetags">&gt; </span>+            if (js.anyJarsSigned()) { 
<span class="moz-txt-citetags">&gt; </span>+                //If there are any signed Jars, check if JNLP file is signed
<span class="moz-txt-citetags">&gt; </span>+                
<span class="moz-txt-citetags">&gt; </span>+                if (JNLPRuntime.isDebug())
<span class="moz-txt-citetags">&gt; </span>+                    System.out.println("STARTING check for signed JNLP file...");
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                final String template = "JNLP-INF/APPLICATION_TEMPLATE.JNLP";
<span class="moz-txt-citetags">&gt; </span>+                final String application = "JNLP-INF/APPLICATION.JNLP";
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                for (int i = 0; i &lt; jars.length; i++) {
<span class="moz-txt-citetags">&gt; </span>+                    List&lt;JARDesc&gt; eachJar = new ArrayList&lt;JARDesc&gt;();
<span class="moz-txt-citetags">&gt; </span>+                    JarSigner signer = new JarSigner();
<span class="moz-txt-citetags">&gt; </span>+                    eachJar.add(jars[i]); //Adds only the single jar to check if the jar has a valid signature
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                    tracker.addResource(jars[i].getLocation(), jars[i].getVersion(),
<span class="moz-txt-citetags">&gt; </span>+                            getDownloadOptionsForJar(jars[i]),
<span class="moz-txt-citetags">&gt; </span>+                            jars[i].isCacheable() ? JNLPRuntime.getDefaultUpdatePolicy()
<span class="moz-txt-citetags">&gt; </span>+                                    : UpdatePolicy.FORCE);
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                    try {
<span class="moz-txt-citetags">&gt; </span>+                        signer.verifyJars(eachJar, tracker);
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                        if (signer.allJarsSigned()) { //If the jar is signed
<span class="moz-txt-citetags">&gt; </span>+                            URL location = jars[i].getLocation();
<span class="moz-txt-citetags">&gt; </span>+                            File localFile = tracker.getCacheFile(location);
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                            if (localFile != null) {
</pre>
      </blockquote>
      <pre wrap="">There is a security issue here -- what if local file is null? Then no check will happen?

</pre>
      <blockquote type="cite" style="color: rgb(0, 0, 0);">
        <pre wrap=""><span class="moz-txt-citetags">&gt; </span>+                                try {
<span class="moz-txt-citetags">&gt; </span>+                                    JarFile jarFile = new JarFile(localFile);
<span class="moz-txt-citetags">&gt; </span>+                                    Enumeration&lt;JarEntry&gt; entries = jarFile.entries();
<span class="moz-txt-citetags">&gt; </span>+                                    JarEntry je;
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                                    while (entries.hasMoreElements()) {
<span class="moz-txt-citetags">&gt; </span>+                                        je = entries.nextElement();
<span class="moz-txt-citetags">&gt; </span>+                                        String jeName = je.getName().toUpperCase();
<span class="moz-txt-citetags">&gt; </span>+
<span class="moz-txt-citetags">&gt; </span>+                                        if (jeName.equals(template) || jeName.equals(application)) {
<span class="moz-txt-citetags">&gt; </span>+
</pre>
      </blockquote>
      <pre wrap="">The specification states that the file names should be recognized
regardless of case. The above code does not allow that.

</pre>
    </blockquote>
    jeName is used to store the jar entry name in all uppercase before
    comparing it in the if statement. "String jeName =
    je.getName().toUpperCase();"<br>
    I think this should work regardless of the case!<br>
    <pre class="moz-signature" cols="72">-- 
Cheers,
Saad Mohammad</pre>
  </body>
</html>