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

[Andrew Azores]

On 04/17/2014 07:55 AM, Jiri Vanek wrote:
> On 04/16/2014 07:46 PM, Andrew Azores wrote:
>> On 04/16/2014 10:14 AM, Jiri Vanek wrote:
>>> 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:
>>>>
>>>> - APPEXTSECunsetAppletActionNo should probably be renamed, why is 
>>>> the 'No' there?
>>>> -- Should also be reworded, maybe "This applet has not been visited 
>>>> 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.
>>>> -- Please move the private empty constructor to the top of the 
>>>> class as well
>>>> - ExecuteAppletAction.fromString() - the RuntimeException message 
>>>> here should be reworded.
>>>> "Undefined zero-length ExecuteAppletAction String representation" ?
>>>> - 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
>>>> - Rather than doing 'for (int i = 0; i < str.length(); i++)' and 
>>>> 's.charAt(i)', how about for (char
>>>> ch : s.toCharArray())?
>>>>
>>>> Thanks,
>>>>
>>>
>>> And general approach?
>>>
>>
>> It seems okay, although I'm not sure I completely understand the 
>> motivation behind it. Can you
>> explain more why we need to have this multiple-action structure?
>
>
> The main motivation is "rember checboxx" fro alaca - it is now not 
> working, and It have to be working:) .Maybe more attributes-check 
> deserve remember button. In all cases, the table would have same 
> structure as have current ExtendedApplets security. So motivation is - 
> instead of maintaining several tables, only one will be maintained, 
> and will have several actions.
>
> Another approach may be to have really more columns ( I think I will 
> handle it in this way in EAS itw-settings table) or to have really 
> more tbales.
>
> Well more tbales is probably most readable for user. But it will be 
> full of duplicitous! And also all the overhead, inter-processes 
> locking, and whatever. I don't like this.
>
>
> Another is to have more columns. But we may face some problems in case 
> of adding more even endless number of columns...
>
> So I chose this - change string in first column to keep more vlaues. 
> And to dispaly it as several columns in itw-setting (but this may 
> change, it if there will be tomuch of those small sub columns)
>
>
> J.

Thanks for the clarification! Yes, I think this is a good enough 
approach then. I don't like the sound of more tables either, and 
although more columns might be more technically correct sounding, it 
also means the table will become dominated by yes/no/always/never 
columns, over and over again. Being able to mark them as present/not 
present is nice. This way also seems easier for backward compatibility 
than adding new columns, and easier as well if anyone ever wants to edit 
the trust settings by hand (directly with a text editor) for some reason.

Thanks,

-- 
Andrew A