[rfc][icedtea-web] PR1592 reproducers update

[Jiri Vanek]

On 01/24/2014 10:41 PM, Andrew Azores wrote:
> On 01/22/2014 10:22 AM, Jiri Vanek wrote:
>> On 01/22/2014 03:37 PM, Andrew Azores wrote:
>>> On 01/22/2014 07:25 AM, Jiri Vanek wrote:
>>>> On 01/21/2014 03:43 PM, Andrew Azores wrote:
>>>>> On 01/21/2014 06:16 AM, Jiri Vanek wrote:
>>>>>> On 01/20/2014 09:49 PM, Andrew Azores wrote:
>>>>>>> On 01/20/2014 11:07 AM, Jiri Vanek wrote:
>>>>>>>> On 01/09/2014 08:06 PM, Andrew Azores wrote:
>>>>>>>>> On 01/09/2014 11:17 AM, Andrew Azores wrote:
>>>>>>>>>> On 01/03/2014 02:43 PM, Andrew Azores wrote:
>>>>>>>>>>> Updated PR1592 tests, using a custom reproducer rather than split simple/signed. This allows
>>>>>>>>>>> method calls to be made in the normal way as well as via reflection. JNLP includes both
>>>>>>>>>>> applications and applets now, and they close properly as well.
>>>>>>>>>>>
>>>>>>>>>>> (snip)
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Andrew A
>>>>>>>>>>
>>>>>>>>>> Went back over this and realized one of the tests was written wrong. The
>>>>>>>>>> assertAccessControlException helper method in the testcase file is now a little stricter
>>>>>>>>>> about the
>>>>>>>>>> type of AccessControlException (so that the exceptions due to applets not being allowed to
>>>>>>>>>> call
>>>>>>>>>> System.exit don't falsely fulfill this assertion), and
>>>>>>>>>> MixedSigningAppletHelper.attackDoPrivileged
>>>>>>>>>> now properly calls MixedSigningAppletSigned#testSignedReadPropertiesDoPrivileged, as it
>>>>>>>>>> should
>>>>>>>>>> have been doing. In this case, the Unsigned JAR actually *is* meant to be able to retrieve
>>>>>>>>>> data
>>>>>>>>>> from the Signed JAR (as is the point of the AccessController.doPrivileged call), so the
>>>>>>>>>> testcases
>>>>>>>>>> now expect this test to successfully read from System.getProperty, rather than receive an
>>>>>>>>>> AccessControlException. However, the tests still verify that in situations where the
>>>>>>>>>> Signed JAR
>>>>>>>>>> has a method call that involves a privileged action *without* being placed inside a
>>>>>>>>>> doPrivileged
>>>>>>>>>> call, an AccessControlException will be thrown if the Unsigned code attempts to access it, as
>>>>>>>>>> expected.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sorry, please ignore the previous patch. The extra changes were not made based on the most
>>>>>>>>> recent
>>>>>>>>> other changes. Attached are the properly rebased patches, also split into three as they were
>>>>>>>>> originally.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanx for ping.
>>>>>>>> There is really many of jnlps which are nearly similar. Maybe better idea can be to have one
>>>>>>>> template, and generate all the rest from it?
>>>>>>>>
>>>>>>>> I altready did this - and generated them ion BeforeClass.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> I like this idea, but I didn't know we were okay with having reproducers do tricks like this ;)
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>
>>>>>>
>>>>>> On seriosu flaw: you have "private static final ServerAccess server = new ServerAccess();"
>>>>>> declard. by this you are owerwritting the one in BrowserTest, so no browser test will work,
>>>>>> and wil fial 'can not lunch unset browser".
>>>>>> Just remove this line.
>>>>>
>>>>> Oops, right.
>>>> [2]
>>>>>
>>>>>>
>>>>>> Also - non of the jnlp have security element specified. It i s intentional?? I thought it was
>>>>>> an reason for this test to be redone.
>>>>>>
>>>>>> After fixed first, and explined second, ok to head.
>>>>>>
>>>>>> J.
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2013-December/025635.html
>>>>>
>>>>> The JNLP spec seems a little bit ambiguous here - it seems to say that "All JARs must be
>>>>> signed" if requesting All-Permission, but doesn't explicitly say if full signing is required to
>>>>> be able to request permissions at all. But as I explained in that other thread, I think it's
>>>>> correct to use the security tag when you have full signing, and not correct to use it when you
>>>>> have partial signing.
>>>>
>>>>
>>>> Hmhm. And what should itw do with such an malicious (security tag) file and contne (not all jars
>>>> signed)t? It should die...
>>>>
>>>> And thats exactly what the reproducers should check.
>>>>
>>>> Or not?
>>>>
>>>> So from my side (and after reading the [1]) I'm +1 to add the tests.
>>>>
>>>> J.
>>>>
>>>> [2] dont forget on it in case of new round :)
>>>
>>> It does die, which is correct IMO.
>> yes its the most correct.
>>
>> > Clearly it would be wrong if we actually respected that security tag, and I think it would be
>> ambiguous of us and kind of useless to silently ignore the security tag - doing this would
>> probably just end up annoying some app developer. Best to explicitly fail with the launch
>> exception describing the exact problem. If you think it's worthy of a new patch/review round then
>> I can also add that. But how in-depth do you want it to be... ? Just one test to ensure that the
>> security tag causes a failed launch, or repeat all existing tests with an addition security tag,
>> or somewhere in between?
>>
>> Yes. its worth to add reproducer with security tag, and one more round of review.
>
> Well, the testcase file has now ballooned to ridiculous proportion.
>

Yes, it does :(


Anyway looks good to go. Thank you!

J.