[rfc][icedteaweb] Add comments about permission attributes not being checked in reproducers

[Jiri Vanek]

On 03/23/2015 08:15 PM, Lukasz Dracz wrote:
> Hello,
>
> I have attached the updated patch.
>
>>> I have attached a patch to show, what I was thinking of as a potential fix.
>>>
>>
>> maybe a bit of owerkill, but why not.
>>
>>> I made a new BasicValueValidator that allows combination of options
>>> alongside single options.
>>> I changed attributes check property from a boolean to an enum
>>> with the following options ALL, NONE, PERMISSIONS, TRUSTED, CODEBASE, ALAC
>>> With the new MultipleStringValueValidator I have made it that
>>> attributes check property can either be ALL, NONE or a
>>> combination of PERMISSIONS, TRUSTED, CODEBASE, ALAC (delimited by "," no
>>> spaces)
>>
>> Please add ENTRYPOINT
>
> okay
>
>>>
>>> In this way it can be specified which checks you want to do
>>> for ex. only TRUSTED and CODEBASE attributes would look like
>>> deployment.manifest.attributes.check=TRUSTED,CODEBASE
>>>
>>> The fix for the tests is also included that changes the
>>> deployment.manifest.attributes.check in the beforeClass
>>> and puts back the previous value afterClass. Also deployment.security.level
>>> is changed to ALLOW_UNSIGNED and back in
>>> the same fashion for the Partially Signed tests.
>>>
>>> I have mostly attached the full patch to get feedback on this direction of
>>> a fix ?
>>> Any comments or thoughts would be great.
>>>
>>> For review I think it would be best if I split this patch into two, for
>>> easier reviewing
>>>
>>> - The MultipleStringValueValidator
>>> - The Fix with Manifest Attributes Check and in the tests
>>>
>>> Other option is to go with the original suggestion of just an enum with
>>> true, false, permissions_only with the same
>>> changing the deployment.properties BeforeClass fix. In this case, I would
>>> prefer the enum to be ALL, NONE or
>>> PERMISSIONS (/PERMISSIONS_ONLY) instead.
>>
>> if this fixs the testsm then +1 from me for what you have done here. It is
>> allowing good versatility and actually to test what is necessary.
>>
>>>
>>> Thoughts ?
>>
>> few inline.
>>>
>>>

+        String[] attributesCheck = deploymentProperty.split(",");
Something duplicated here? :)


Assuming it fixed the tests, after fixing the minor above, please push.

thank you!