[rfc][icedtea-web] conditional disabling of manifes tattributes

Wed May 7 08:26:01 UTC 2014

On 04/16/2014 04:01 PM, Andrew Azores wrote:
> On 04/16/2014 03:08 AM, Jiri Vanek wrote:
>> On 04/15/2014 05:48 PM, Jiri Vanek wrote:
>>> On 04/11/2014 06:49 PM, Andrew Azores wrote:
>>>> On 04/11/2014 09:07 AM, Jiri Vanek wrote:
>>>>> On 04/10/2014 04:44 PM, Andrew Azores wrote:
>>>>>> On 04/10/2014 08:43 AM, Jiri Vanek wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>> This patch can disable the check for new manifest attributes.
>>>>>>> sevral motivations
>>>>>>>   -  I hate this feature
>>>>>>>   - the remembering of this actions may take time to implement
>>>>>>>   - and the most important  - the testsuite...
>>>>>>
>>>>>> Most important indeed.
>>>>>>
>>>>>>>
>>>>>>> will go both head and 1.5.
>>>>>>>
>>>>>>> I'm hesitating whether to make this property public via itw-settings... Well if so, then as
>>>>>>> another changeset. Any opinion about publish it in itw-settings?
>>>>>>>
>>>>>>> J
>>>>>>
>>>>>> Ok to push, so that we can clean up some of the noise in the test suite.
>>>>>>
>>>>>> RE putting it in itweb-settings - I think the cleanest way to do this, if it's to be done at
>>>>>> all, is
>>>>>> probably to add another security level. 'Minimal' or something perhaps? Because I don't think it
>>>>>> makes sense to add it in as a checkbox, for example, and have it possible to set security to
>>>>>> High,
>>>>>> but disable manifest checks. I'm not sure if we really want to allow this, though. I can imagine
>>>>>> most users leaving the security level at Minimal once they discover that this gives them the
>>>>>> least
>>>>>> amount of clicking to do before getting their applets to run :)
>>>>>
>>>>>
>>>>> Oh yes, that can be correct thing to do. But looking to the issue from this perspective, it is
>>>>> NO-GO  argument for it as it is done. Well all correct applications should be updated to support
>>>>> new attributes.. but the legaxy, and still used ones?
>>>
>>> I have pushed at the end. This shortcut for testsuite is needed.
>>>>>
>>>>> So I have another idea here - really extend ExtendedAppletsSecurity, and to allow "do not check
>>>>> manifest attributes" on the application/codebase  - So all those dialogs will be moreover based on
>>>>> your abstraction of Extended Applets security. Also remember alaca checkbox issue will be solved
>>>>> by this.
>>>>>
>>>>> Bu the extension of ExtendedAppletsSecurity will needed to be done really carefully - and prepared
>>>>> before implementation.
>>>>>
>>>>> What do you think?
>>>>
>>>> Sounds like it could be a lot of work, but should be worthwhile in the end. I think it will be very
>>>> nice if we can unify all of our dialogs like this so that they can all make use of the same
>>>> remembered action storage. Or at least, the ones that need to be able to remember actions. I'm not
>>>> sure about having the manifest attribute check be an option to be selected on the dialog though,
>>>> even with an option to remember it... but maybe it's the right thing to do. It just seems like the
>>>> dialogs are getting busier and busier and more confusing looking, but you're right that this option
>>>> will (hopefully) be used mostly for allowing some old applets which haven't been updated to be used
>>>> still. In that kind of situation, which I think is the target, then applying the setting to all
>>>> applets/globally maybe isn't the right choice. So as ugly and cluttered as the UI might become, I
>>>> suppose this really should be done on a per-applet basis (maybe in addition to having it
>>>> configurable globally?).
>>>>
>>>
>>> Hi attached patch is adding the basic support for multiple types of actions. Well it apeared that
>>> the implementation itself will be quit easy (if this approach will be used) but most troubles will
>>> be in itw-settings. The table will requires serious revork on level of filtering, sorting, delting
>>> and especially editing of first - action - column.
>>>
>>> General idea is - instead of one   ExecuteAppletAction unsignedAppletAction will be list of them,
>>> encapsualted in  AppletSecurityActions unsignedAppletAction;
>>> In file it will be encoded instead of current one char, as one-word string - see the tests.
>>> To read/write the correct action for given type, there will be set of getters/setters  - tight now
>>> there is
>>>   public ExecuteAppletAction getUnsignedAppletAction() {
>>> +        return getAction(0);
>>> +    }
>>> +
>>> +    public void setUnsignedAppletAction(ExecuteAppletAction a) {
>>> +        actions.set(0,a);
>>> +    }
>>>
>>> And they will encapsulate the order in list.
>>>
>>>
>>> If nothing else...This is nicely backward compatible, and scaleable for growth of "remember me"
>>> dialogs :)
>>>
>>> Once this is aproved -if ever -  I will add few more "on top of it" work like -table or rember for
>>> ALACAto se hw it works - before push itself.
>>> J.
>>>
>>>
>>
>> This is a bit better - get rid of paranoid constant of MAX_LENGTH.
>>
>> J.
>>
>
> Just a few nits:

Here we go!

>
> - APPEXTSECunsetAppletActionNo should probably be renamed, why is the 'No' there?
fixed

> -- Should also be reworded, maybe "This applet has not been visited before"

kept - es expalined before

> - AppletSecurityActions.fromString() - why 'break' on whitespace? I can understand if it was only
> internal whitespace, but even leading whitespace? eg the string " A" means UNSET? Maybe do a
> String#trim() first.

well, fixed, but see the comment in code

> -- Please move the private empty constructor to the top of the class as well
removed completely

> - ExecuteAppletAction.fromString() - the RuntimeException message here should be reworded.
> "Undefined zero-length ExecuteAppletAction String representation" ?
fixed

> - ExecuteAppletAction should be refactored now that it has both a toChar and fromChar. I would
> suggest retaining the corresponding char as a field (so make a one-arg constructor), then toChar
> simply returns this and fromChar iterates over the enum values. If there is a matching enum value's
> char, return that enum value, otherwise return UNSET

Unless you really insist, I kept my original approach. I like it more.

> - Rather than doing 'for (int i = 0; i < str.length(); i++)' and 's.charAt(i)', how about for (char
> ch : s.toCharArray())?

ok :)
>
> Thanks,
>

The unittest was enhanced to test backward compatibility and new feature.

Thank you again.

J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: multipleActionsBasePatch4.patch
Type: text/x-patch
Size: 34009 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140507/c80f4bd2/multipleActionsBasePatch4-0001.patch>