[rfc][icedtea-web] extended reflection tests

[Danesh Dadachanji]

On 06/03/12 05:41 AM, Jiri Vanek wrote:
> On 03/05/2012 11:23 PM, Danesh Dadachanji wrote:
>> Hi Jiri,
>>
>> Thanks for the tests!
>>
>> On 05/03/12 07:55 AM, Jiri Vanek wrote:
>>> Hi! Few more test to reflection
>>>
>>> What I'm surprised is, that unsigned application can use reflection at
>>> all. I tough they can not....
>>
>> I thought so too..
> Pavel have thoughts taht it should be correct..adding him to cc for case
> I'm lying. Also Omair cced as he had something to do with reflection too..
>>
>>> This text is expecting them unsigned ones using reflection as correct as
>>> it is current behaviour. But feel free to correct me if unsigned
>>> applications should have reflection forbidden.
>>
>> I'll let someone who knows more comment on this. Until then, I have
>> some comments on the patch below.
> see above
>>
>>>
>>> 2012-03-05 Jiri Vanek <jvanek at redhat.com>
>>>
>>> Improved reflection test:
>>> *
>>> /home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/testcases/AccessClassInPackageTest.java:
>>>
>>>
>>> This testcase was extended for three more unsigned reflection tries
>>> and four signed
>>> */home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/srcs/AccessClassInPackage.java:
>>>
>>> now accepting class to be findByName as argument. Four new jnlp
>>> files in
>>> signed a four in simple are then passing those argument
>>> */home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageSUNSEC.jnlp:
>>>
>>> */home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageNETSF.jnlp:
>>>
>>> */home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageJAVAXJNLP.jnlp:
>>>
>>> */home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageSELF.jnlp:
>>>
>>> */home/jvanek/hg/icedtea-web/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp:
>>>
>>> removed
>>
>> Don't think we need your full path of "/home/jvanek/hg/icedtea-web/" ;)
> Damn, thanx for catch.
>>
>> Please can you add <vendor> to all the jnlp files. It'd work right now
>> but once the changeset I have in review is accepted, it'll start
>> failing to run javaws altogether.
>
> Done for this case, But I'm afraid you will break more tests. Before
> commit of your changeset you should ensure (fix missing vendors in
> current tests) that no new failures appears.
> I will also add vendors in all current and future tests.

Yeah I suspected that was the case. I'll update my patch to include 
fixes for all the current tests.

>>
>> I believe you also need to add the copyright header for many of the
>> files.
> done. I always forgotr about jnlp files :-/
>>
>>> + private String[] badExceptions = {
>>> + "accessClassInPackage.javax.jnlp.ServiceManager)",
>>> + "accessClassInPackage.AccessClassInPackage)",
>>> + "accessClassInPackage.net.sourceforge.jnlp)",
>>> + "accessClassInPackage.sun.security.internal.spec)"
>>
>> [snip]
>>
>>> + private void testShouldFail(ServerAccess.ProcessResult pr, String s) {
>>> + String c = "java.security.AccessControlException: access denied
>>> (java.lang.RuntimePermission " + s;
>>> + Assert.assertTrue("stderr should contains `" + c + "`, but didn't
>>> ", pr.stderr.contains(c));
>>> + }
>>> + private void testShouldNOTFail(ServerAccess.ProcessResult pr,
>>> String s) {
>>> + String c = "java.security.AccessControlException: access denied
>>> (java.lang.RuntimePermission " + s;
>>> + Assert.assertFalse("stderr should NOT contains `" + c + "`, but did
>>> ", pr.stderr.contains(c));
>>> + }
>>
>>
>> I noticed testShouldFail asserts were always failing when the methods
>> were being called. I believe you need to add double quotes around
>> java.lang.RuntimePermission and s in order for contains() to pass.
>>
>> stderr.log outputted:
>> [...] access denied ("java.lang.RuntimePermission"
>> "accessClassInPackage.sun.security.internal.spec")
>>
>> Change the String c's to (\"java.lang.RuntimePermission\" and wrap
>> each element of badExceptions similarly, that should take care of the
>> problem.
> hmm.. this scares me a lot. I have changed catching from contains to
> match, to have more vague regex inside. But what scares me is that I
> have never seen this kind of exception in format you have described...
> Attached is my exception format (same on f14 and f16) and new patch.

Interesting, my F16 has "" around it. I built it with stock 6 (did not 
use --with-jdk-home).

I don't think using matches with .* is the best idea. In this case, 
pr.stderr is a String, right? That means that if let's say stderr 
contains "java.security.AccessControlException" and then somewhere later 
on contains the other part we're searching for, it will return true.

Perhaps an alternative solution for now is to match 0 or 1 '"' chars 
around the exceptions, instead of .* being used. Maybe leave a warning 
in comments saying stderr could be formatted differently. Then, if 
someone runs into this later on down the line, we can adjust to their 
output too.

Attached is the exception I'm seeing. Hope this helps!

>>
>>
>> Once the above's sorted out, this looks good to me for pushing.
>>
>> Regards,
>> Danesh
>
>
> 2012-03-06 Jiri Vanek <jvanek at redhat.com>
>
> Improved reflection test:
> *
> tests/jnlp_tests/simple/AccessClassInPackage/testcases/AccessClassInPackageTest.java:
>
> This testcase was extended for three more unsigned reflection tries and
> four signed
> *tests/jnlp_tests/simple/AccessClassInPackage/srcs/AccessClassInPackage.java:
>
> now accepting class to be findByName as argument. Four new jnlp files in
> signed a four in simple are then passing those argument
> *tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageSUNSEC.jnlp:
>
> *tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageNETSF.jnlp:
>
> *tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageJAVAXJNLP.jnlp:
>
> *tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackageSELF.jnlp:
>
> *tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp:
>
> removed
> *
> tests/jnlp_tests/signed/AccessClassInPackageSigned/srcs/AccessClassInPackageSigned.java
>
> signed variation of AccessClassInPackage, tescase is also in
> AccessClassInPackage
> *
> tests/jnlp_tests/signed/AccessClassInPackageSigned/resources/AccessClassInPackageSignedSELF.jnlp
>
> *
> tests/jnlp_tests/signed/AccessClassInPackageSigned/resources/AccessClassInPackageSignedNETSF.jnlp
>
> *
> tests/jnlp_tests/signed/AccessClassInPackageSigned/resources/AccessClassInPackageSignedSUNSEC.jnlp
>
> *
> tests/jnlp_tests/signed/AccessClassInPackageSigned/resources/AccessClassInPackageSignedJAVAXJNLP.jnlp
>
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: exception-F16-danesh.txt
Url: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120306/84f8d7a6/exception-F16-danesh.txt