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

[Andrew Azores]

On 04/17/2014 10:17 AM, Andrew Azores wrote:
> 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,
>

Is another version of this patch coming?

Thanks,

-- 
Andrew A