RFC [iceedtea-web] added first set of reproducres

[Omair Majid]

Hi Jiri,

On 06/20/2011 05:48 AM, Jiri Vanek wrote:
> All those reproducers are based on older issues found, reproduced and
> fixed by Omair (and maybe others I'm not aware about) - so are passing.
>
> Regards J.
>

Overall, the patch looks fine to me. Please see the comments in-line below.

> diff -r e413e4676929 tests/jnlp_tests/simple/AccessClassInPackage/srcs/AccessClassInPackage.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/jnlp_tests/simple/AccessClassInPackage/srcs/AccessClassInPackage.java	Mon Jun 20 11:38:50 2011 +0200
> @@ -0,0 +1,6 @@
> +public class AccessClassInPackage {
> +
> +    public static void main(String[] args) throws Exception{
> +            Class.forName("sun.security.internal.spec.TlsKeyMaterialSpec");
> +    }
> +}

This test is missing a copyright notice. Is there a reason you are 
adding copyrights to files under tescases but not to those under 
resources/srcs?

> diff -r e413e4676929 tests/jnlp_tests/simple/AccessClassInPackage/testcases/AccessClassInPackageTest.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/jnlp_tests/simple/AccessClassInPackage/testcases/AccessClassInPackageTest.java	Mon Jun 20 11:38:50 2011 +0200
> @@ -0,0 +1,67 @@
> +/* AccessClassInPackageTest.java
> +Copyright (C) 2011 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 net.sourceforge.jnlp.ServerAccess;
> +import org.junit.Assert;
> +
> +import org.junit.Test;
> +
> +public class AccessClassInPackageTest {
> +
> +    private static ServerAccess server = new ServerAccess();
> +
> +
> +
> +    @Test
> +    public void AccessClassInPackageTestLunch1() throws Exception {
> +        System.out.println("connecting AccessClassInPackageTest request");
> +        ServerAccess.ProcessResult pr=server.executeJavawsHeadless(null,"/AccessClassInPackage.jnlp");
> +        System.out.println(pr.stdout);
> +        System.err.println(pr.stderr);
> +        Assert.assertTrue(pr.stderr.contains("java.security.AccessControlException: access denied (java.lang.RuntimePermission accessClassInPackage.sun.security.internal.spec)"));
> +        Assert.assertFalse(pr.stderr.contains("ClassNotFoundException"));
> +        Assert.assertFalse(pr.stdout.length()>2);
> +        Assert.assertFalse(pr.wasTerminated);
> +        Assert.assertEquals((Integer)0, pr.returnValue);
> +    }
> +
> +
> +
> +
> +
> +    }

5 blank lines seems a little too much. Normally we stick with 1 or two 
blanks lines. This is also present in most of the new tests in the 
patch. Please fix this.

> diff -r e413e4676929 tests/jnlp_tests/simple/ReadProperties/srcs/ReadProperties.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/jnlp_tests/simple/ReadProperties/srcs/ReadProperties.java	Mon Jun 20 11:38:50 2011 +0200
> @@ -0,0 +1,10 @@
> +public class ReadProperties {
> +
> +
> +    public static void main(String[] args) {
> +
> +       // System.getProperty("user.name");
> +       // System.getProperty("user.home");
> +          System.getProperty(args[0]);
> +  }
> +}

I am a little curious as to why the commented out lines are present. Is 
there a problem with System.getProperty("user.name")? Or is it just 
leftover code from when you were testing?

Cheers,
Omair