[RFC][icedtea-web]: Added signed jnlp tests for applications using multiple jars

[Adam Domurad]

Thanks for the clarification and sorry for the late reply. It can be
rather hard reviewing when you're not familiar with the aspects being
tested! It's currently causing me a bit of head-ache because of the many
files with subtle naming differences. 

The SignedJnlpResource2 folder should probably be named something like
JnlpTemplateResource from what I can gather from its contents. To me,
the 'signed' is implied by its location in the signed folder. A rule of
thumb for naming is to avoid similar names only differentiated by
number, like foo, foo2, etc.

Your ChangeLog entry:

> tests/reproducers/signed/SignedJnlpResource2/srcs/SignedJnlpApplicationTemplate.java:
>      A simple java class that outputs a string.
is misleading. This class uses reflection to call SignedJarResource &
SignedJnlpResource, it does not merely output a string without these
classes. Maybe name this class so it clearly states this ? The current
name is confusing to me.


> tests/reproducers/signed/SignedJnlpMultipleJars/srcs/SignedJnlpApplication.java:
>      A simple java class that outputs a string.
Similar to above. How is this any different from the above class ?
You'll have to forgive my ignorance here I'm afraid I require a bit
more description of the layout of all the resources here. They seem to
span across a number of folders.

On Tue, 2012-07-24 at 15:32 -0400, Saad Mohammad wrote:
> Hi Adam,
> 
> Thanks for looking over this patch. I have answered your questions below.
> 
> On 07/17/2012 04:37 PM, adomurad wrote:
> > Hi Saad. Thanks for the patches! General question, what motivated you to
> > test these aspects (which all seem to pass?) You'll have to forgive my
> > ignorance of jnlp templates, I would appreciate a brief elaboration on
> > the aspects under test here.
> > 
> 
> I have added these tests to ensure applications with multiple jars and signed
> jnlp(s) are always working correctly.
> 
> The rules that are being tested:
>       -> A signed jnlp file is checked and validated only if found in the main
> 	 jar and the main jar is fully signed. If an application has multiple
>          jars for its resources, it will _only_ check/validate the main jar for
>          a signed jnlp.
I'm afraid you'll have to elaborate on this because I'm not aware of
when jnlp files are classified as signed. 
> 
>       -> If the signed jnlp does not match the launching jnlp file, the launch
>          of the application is aborted. (An exception is also thrown).
Why is this tested here if its already covered in SignedJnlpCaseTestOne
& SignedJnlpCaseTestTwo ?
> 
> The reproducers check these rules using both methods of signed jnlps
> (application and template). The tests ensures _only_ the main jar is being
> checked and validated for a signed jnlp, especially in cases where an
> application has multiple signed jnlps within various jars. It should never
> validate signed jnlp files in non-main jars.
> 
> A small misbehaviour with the handling of signed jnlps in IcedTea-Web can easily
> terminate the launch of an application. This is why I feel that these tests are
> very important.
> 
> > Comments inline.
> > 
> > On Wed, 2012-07-04 at 17:05 -0400, Saad Mohammad wrote:
> >> Hello,
> >>
> >> I have attached the patch that tests the launching of applications which 
> >> have multiple jars containing signed jnlp files.
> >>
> >> ChangeLog:
> >>
> >> 2012-07-05  Saad Mohammad  <smohammad at redhat.com>
> >>
> >>      Added multiple jars with signed jnlp tests.
> >>      * 
> >> tests/reproducers/signed/SignedJarResource/resources/SignedJarResource.jnlp:
> >>      Launches SignedJarResource class directly.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpMultipleJars/resources/MainJarWithMatchingSignedJnlp.jnlp:
> >>      * 
> >> tests/reproducers/signed/SignedJnlpMultipleJars/resources/MainJarWithUnmatchingSignedJnlp.jnlp:
> >>      * 
> >> tests/reproducers/signed/SignedJnlpMultipleJars/resources/MainJarWithoutSignedJnlp.jnlp:
> >>      Launching jnlp file that contains multiple jars as its resource.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpMultipleJars/srcs/JNLP-INF/APPLICATION.jnlp:
> >>      Signed jnlp file.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpMultipleJars/srcs/SignedJnlpApplication.java:
> >>      A simple java class that outputs a string.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpMultipleJars/testcases/SignedJnlpMultipleJarsTest.java:
> >>      Testcase that tests the launching of jnlp files containing multple jar
> >>      as its resources.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpResource/resources/SignedJnlpResource.jnlp:
> >>      Launches SignedJnlpResource class.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpResource2/resources/MainJarWithMatchingSignedJnlpTemplate.jnlp:
Changelog entries with something like "Same as above" are preferred vs
empty ones.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpResource2/resources/MainJarWithUnmatchingSignedJnlpTemplate.jnlp:
> >>      Launching jnlp file that contains multiple jars as its resource.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpResource2/srcs/JNLP-INF/APPLICATION_TEMPLATE.jnlp:
> >>      Signed jnlp file.
> >>      * 
> >> tests/reproducers/signed/SignedJnlpResource2/srcs/SignedJnlpApplicationTemplate.java:
> >>      A simple java class that outputs a string.
> >>
> > 
> > 
> >> /*  SignedJnlpMultipleJarsTest.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 java.util.Arrays;
> >> import java.util.Collections;
> >> import java.util.List;
> >> import net.sourceforge.jnlp.ServerAccess;
> >> import org.junit.Assert;
> >> import org.junit.Test;
> >>
> >> public class SignedJnlpMultipleJarsTest {
> >>
> >>     private static ServerAccess server = new ServerAccess();
> >>     private final List<String> l = Collections.unmodifiableList(Arrays.asList(new String[] { "-Xtrustall" }));
> >>     private final String signedJnlpException = "net.sourceforge.jnlp.LaunchException: Fatal: Application Error: "
> >>             + "The signed JNLP file did not match the launching JNLP file. Missing Resource: Signed Application "
> >>             + "did not match launching JNLP File";
> >>
> >>     @Test
> >>     public void checkingForRequiredResources() throws Exception {
> >>         String s = "Running SignedJarResource..";
> >>         ServerAccess.ProcessResult pr = server.executeJavawsHeadless(l, "/SignedJarResource.jnlp");
> >>         Assert.assertTrue("Could not locate SignedJarResource class within SignedJarResource jar", pr.stdout.contains(s));
> > The failure messages here and below assume too much based on the condition.
> >>
> >>         s = "Running SignedJnlpResource..";
> >>         pr = server.executeJavawsHeadless(l, "/SignedJnlpResource.jnlp");
> >>         Assert.assertTrue("Could not locate SignedJnlpResource class within SignedJnlpResource jar", pr.stdout.contains(s));
> > What functionality does this test exactly ? It seems like SignedJnlpResource more or less just loads a .jar in a fashion that is already well tested.
> 
> This just ensure the resources required to run the rest of the test have been
> found. If any of these resources are missing, it will fail most of the other tests.
> 
> >>
> >>         pr = server.executeJavawsHeadless(l, "/MainJarWithMatchingSignedJnlpTemplate.jnlp");
> >>         s = "Ending signed application_template in main";
> >>         Assert.assertTrue("Could not locate SignedJnlpApplicationTemplate class within MainJarWithMatchingSignedJnlpTemplate jar", pr.stdout.contains(s));
> > 
> > How is MainJarWithMatchingSignedJnlpTemplate.jnlp being used in the test other than this ? 
> > If this is aiming to test that it runs correctly than it should be its own test imo.
> 
> This is used to ensure SignedJnlpApplicationTemplate class within
> SignedJnlpResource2.jar is found (not really testing the functionality). The
> above assert message is incorrect and misleading, I will definitely change this
> when I update the patch.

I still feel this would be better in its own test named 'mainJarWithMatchingSignedJnlpApplication'. Your preference.

> 
> This is the only place that MainJarWithMatchingSignedJnlpTemplate.jnlp is being
> used in the test to determine whether SignedJnlpResource2.jar (with
> SignedJnlpApplicationTemplate.class) is found in the test build directory. If we
> did this check with a jnlp file that did NOT match the signed jnlp, an exception
> would be thrown and the launch would be aborted. But if the signed jnlp
> validator ever malfunctions, it may fail to throw an exception. That's why using
> a matching jnlp is much more reasonable for this check IMO.
> 
> I don't think we would need to use this jnlp to test anything else since it also
> indirectly validates whether an application runs 'normally' with a matching jnlp.
> 
> > 
> >>
> >>         pr = server.executeJavawsHeadless(l, "/MainJarWithMatchingSignedJnlp.jnlp");
> >>         s = "Ending signed application in main";
> >>         Assert.assertTrue("Could not locate SignedJnlpApplication class within SignedJnlpMultipleJars jar", pr.stdout.contains(
> > 
> > 
> >>     }
> >>
> >>     @Test
> >>     public void mainJarWithUnmatchingSignedJnlpApplication() throws Exception {
> >>         ServerAccess.ProcessResult pr = server.executeJavawsHeadless(l, "/MainJarWithUnmatchingSignedJnlp.jnlp");
> >>         Assert.assertTrue("Stdout should contains " + signedJnlpException + " but did not", pr.stderr.contains(signedJnlpException));
> > contains -> contain, same below.
> 
> Hehe :P
> 
> > 
> 
> [snip]
> 

Thanks again for the patch! Sorry it's taking me so long to grasp