[rfc][icedtea-web] multiple remember actions

[Andrew Azores]

On 04/24/2014 04:36 AM, Jiri Vanek wrote:
> Hi!
>
> Based on previous two enhancements - [rfc][icedtea-web] replace jlabel 
> by  jeditorpane and  Re: [rfc][icedtea-web] conditional disabling of 
> manifes tattributes
>
> this is enabling to save matching alaca action
>  - benefits: one file, one  lock , one id, one table, one metadata and 
> unlimited number of actions
>  - drawbacks - suspiciously simply, maybe not so transparent, once 
> more columns will be added, new editing
>
> The patch grown due to changed signatures of methods. I will try to 
> summarize:
>
>
> * 
> netx/net/sourceforge/jnlp/controlpanel/UnsignedAppletActionTableModel.java"
>  - original columnrnemaed to "Unsigned applet action" added "Library 
> action" column
>  - the value of those two columns is get from 
> source.getAppletSecurityActions().* methods
>
> * 
> netx/net/sourceforge/jnlp/controlpanel/UnsignedAppletsTrustingListPanel.java
>  - adapted "toString" method
>  - added unset to list of combobox values for first two columns cell 
> editors
>  - filter of  "byValue" are now selecting via *or* - if any of action 
> have the desired value, then it is true

See AppletSecurityActions comment about Iterable - if that advice is 
taken then it can be used here already.

Rather than:
+                boolean r = false;
+                for (int i = 0; i < 
AppletSecurityActions.REMEBER_COLUMNS_COUNT; i++) {
+                    ExecuteAppletAction o = (ExecuteAppletAction) 
entry.getModel().getValueAt(entry.getIdentifier(), i);
+                    r = r || (o.equals(ExecuteAppletAction.ALWAYS) || 
o.equals(ExecuteAppletAction.NEVER));
+                }
+                return r;

perhaps:

+                for (int i = 0; i < 
AppletSecurityActions.REMEBER_COLUMNS_COUNT; i++) {
+                    ExecuteAppletAction o = (ExecuteAppletAction) 
entry.getModel().getValueAt(entry.getIdentifier(), i);
+                    if (o.equals(ExecuteAppletAction.ALWAYS) || 
o.equals(ExecuteAppletAction.NEVER)) {
+                        return true;
+                    }
+                }
+                return false;

but this is mostly just stylistic. In the first case the parens around 
the second disjunction are also not really needed :)

>
>
> * netx/net/sourceforge/jnlp/resources/Messages.properties"
>  - enabled date in SUnsignedAllowedBefore and SUnsignedRejectedBefore  
> - thsi was actualy doone in ocde. I do not know why this was forgotten)
>  - removed SAppletTitle - it was nasty :)
>  - added nnew name of column 1 and new name fornew column 2

"Missing Resource: APPEXTSECguiTableModelTableColumnActionUB" ?

>
> * netx/net/sourceforge/jnlp/resources/Messages_cs.properties
>  also removed  SAppletTitle
>
> * netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
>  - added stup for return of international date (jnlpruntime is holding 
> the locales, thats why it its here)

There is a trailing semicolon after the closing brace on this new function

>
>
>  * netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java
>  -  showMatchingALACAttributePanel is receiving whole file (for 
> purposes of reuse in reusable dialogue)) instead of just file.title
>
> * netx/net/sourceforge/jnlp/security/SecurityDialogs.java few minor 
> warning-clean up which I could not look onto.
>  - showMatchingALACAttributePanel - to get file instead file.title and 
> adapted to new  dialogue
>   - checking action for always/never
>   - using text for yes/no
>   - getting response
>   - storing response
>
> * 
> netx/net/sourceforge/jnlp/security/appletextendedsecurity/AppletSecurityActions.java
>   - allowed public access via set(int id) and get(int id) - needed for 
> generalization

"Remember" misspelled as "Remeber" in constant name.

Can this class be made to implement Iterable so getRealCount() + 
getAction(int) isn't the only way?

>
> * 
> netx/net/sourceforge/jnlp/security/appletextendedsecurity/UnsignedAppletActionEntry.java
>   - changed signatures of :
>   ExecuteAppletAction getUnsignedAppletAction() -> 
> AppletSecurityActions getAppletSecurityActions()
>  - and
>  setUnsignedAppletAction(ExecuteAppletAction unsignedAppletAction) -> 
> etAppletSecurityActions(AppletSecurityActions actions)
>
>
> * 
> netx/net/sourceforge/jnlp/security/appletextendedsecurity/UnsignedAppletActionStorage.java
>  - added ID parameter to
>   getMatchingItem
>   getMatchingItemByDocumentBase
>   getMatchingItemByCodeBase
>   getMatchingItemByBases
>  - this is a bit unhappy. But th e mechanism needs to find strong 
> value first. So it mus know whichone it is looking fr :( so we cannot 
> jsut return Actions and select later )
>
> * 
> netx/net/sourceforge/jnlp/security/appletextendedsecurity/UnsignedAppletTrustConfirmation.java
>  - get and forward  the ID parameter
>  - separated get action and get entry logic
>  - one bug fix - when somebody  have visited applet, and have not 
> clicked remember, and later he selected remember for codebase, then 
> the "codebase" change never propagated. Now it does.
>  - forwarding correct id into depth
>
>
> * 
> netx/net/sourceforge/jnlp/security/appletextendedsecurity/impl/UnsignedAppletActionStorageExtendedImpl.java
>  - used the *or* action as in table
>  - made aware of columns abstraction
>
> * 
> netx/net/sourceforge/jnlp/security/appletextendedsecurity/impl/UnsignedAppletActionStorageImpl.java
>  - again propagated ID

What is the use case for a null ID? IOW why Integer and not just int? It 
seems to me that in other places, the ID really just is an int as well...

>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/MatchingALACAttributePanel.java
>  - moved to new impl into apptrustwarningpanel  with other similar panels
>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/apptrustwarningpanel/AppTrustWarningDialog.java:
>  - added shower for MatchingAlaca
>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/apptrustwarningpanel/AppTrustWarningPanel.java
>  - added pluginbridge back - maybe best to o here is to abstract 
> getTitle() into jnlpfile. But as naother work...
>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/apptrustwarningpanel/MatchingALACAttributePanel.java:
>  - actual extension of AppTrustWarningPanel for matching alaca
>  - getAppletTitle is emty, as it is handle in plain text
>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/apptrustwarningpanel/PartiallySignedAppTrustWarningPanel.java
>  - removed duplicate code of getAppletTitle
>  - shown the date of action

With the changed main text, the window is no longer tall enough on my 
setup. The last line of text has the bottom of it clipped off because 
it's too tall. I guess this is actually a problem with the parent class 
not setting its window size correctly.

>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/apptrustwarningpanel/UnsignedAppletTrustWarningDialog.java
>  - removed - this is actually bug. It should be removed when 
> generalized "remember" dialogue came in
>
>
> * 
> netx/net/sourceforge/jnlp/security/dialogs/apptrustwarningpanel/UnsignedAppletTrustWarningPanel.java
>  - shown the date of action
>
>
> * 
> tests/netx/unit/net/sourceforge/jnlp/security/appletextendedsecurity/impl/UnsignedAppletActionStorageImplTest.java
>  - tests adapted to new ID style
>  - well righ now I realised the high-level of tests of new logic are 
> missing (I added the test datat but forgot to add also testcases ) - 
> well wil be added in next round
>
>
> Thanx!
>
> J.

There are a few spots with bad formatting eg one too many or one too few 
spaces of indent:
- UnsignedAppletActionTableModel
- AppletSecurityActions
- MatchingALACAttributePanel

please just "Fix Formatting" on at least these files before push.

The "Unsigned action" classes should really be renamed now, and you even 
have a comment to this effect in UnsignedAppletActionEntry. Can this 
also be done? Maybe just as a separate changeset, but I do think it 
should be done.

Overall okay I think, I can't find much to actually complain about in 
this one other than style nitpicks :D

Thanks,

-- 
Andrew A