/hg/icedtea-web: PolicyEditor persists empty non-default codebas...

[Jiri Vanek]

On 06/24/2014 08:25 PM, Andrew Azores wrote:
> On 06/17/2014 11:40 AM, Andrew Azores wrote:
>> On 06/03/2014 05:03 PM, aazores at icedtea.classpath.org wrote:
>>> changeset f02ad0ae6d86 in /hg/icedtea-web
>>> details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=f02ad0ae6d86
>>> author: Andrew Azores <aazores at redhat.com>
>>> date: Tue Jun 03 17:02:56 2014 -0400
>>>
>>>     PolicyEditor persists empty non-default codebase entries
>>>
>>>     2014-06-03  Andrew Azores  <aazores at redhat.com>
>>>
>>>         PolicyEditor persists empty non-default codebase entries
>>>         * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEntry.java
>>>         (toString): return empty string only if the codebase both has no
>>>         permissions assigned and is also the default "All Applets" codebase
>>>         * netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java
>>>         (savePolicyFile): release fileLock with try/finally
>>>
>>>
>>> diffstat:
>>>
>>> ChangeLog |   9 ++
>>> netx/net/sourceforge/jnlp/security/policyeditor/PolicyEntry.java | 4 +-
>>> netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java |  31 ++++++---
>>>   3 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diffs (75 lines):
>>>
>>> diff -r 8ddd8d86855c -r f02ad0ae6d86 ChangeLog
>>> --- a/ChangeLog    Tue Jun 03 14:25:41 2014 -0400
>>> +++ b/ChangeLog    Tue Jun 03 17:02:56 2014 -0400
>>> @@ -1,3 +1,12 @@
>>> +2014-06-03  Andrew Azores  <aazores at redhat.com>
>>> +
>>> +    PolicyEditor persists empty non-default codebase entries
>>> +    * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEntry.java
>>> +    (toString): return empty string only if the codebase both has no
>>> +    permissions assigned and is also the default "All Applets" codebase
>>> +    * netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java
>>> +    (savePolicyFile): release fileLock with try/finally
>>> +
>>>   2014-06-03  Lukasz Dracz  <ldracz at redhat.com>
>>>         This test expects a Null pointer exception however does not
>>> diff -r 8ddd8d86855c -r f02ad0ae6d86
>>> netx/net/sourceforge/jnlp/security/policyeditor/PolicyEntry.java
>>> --- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEntry.java Tue Jun 03 14:25:41 2014
>>> -0400
>>> +++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEntry.java Tue Jun 03 17:02:56 2014
>>> -0400
>>> @@ -69,7 +69,9 @@
>>>         @Override
>>>       public String toString() {
>>> -        if (permissions.isEmpty() && customPermissions.isEmpty()) {
>>> +        // Empty codebase is the default "All Applets" codebase. If there are no permissions
>>> +        // applied to it, then don't bother recording it in the policy file.
>>> +        if (codebase.isEmpty() && permissions.isEmpty() && customPermissions.isEmpty()) {
>>>               return "";
>>>           }
>>>           final String newline = System.getProperty("line.separator");
>>> diff -r 8ddd8d86855c -r f02ad0ae6d86
>>> netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java
>>> --- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java Tue Jun 03 14:25:41
>>> 2014 -0400
>>> +++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java Tue Jun 03 17:02:56
>>> 2014 -0400
>>> @@ -169,21 +169,30 @@
>>>           final String currentDate = new SimpleDateFormat("yyyy-MM-dd
>>> HH:mm:ss").format(Calendar.getInstance().getTime());
>>>           sb.append("\n/* Generated by PolicyEditor at ").append(currentDate).append(" */");
>>>           sb.append(System.getProperty("line.separator"));
>>> -        final Set<PolicyEditorPermissions> enabledPermissions = new HashSet<>();
>>> -        FileLock fileLock = FileUtils.getFileLock(file.getAbsolutePath(), false, true);
>>> -        for (final String codebase : codebasePermissionsMap.keySet()) {
>>> -            enabledPermissions.clear();
>>> -            for (final Map.Entry<PolicyEditorPermissions, Boolean> entry :
>>> codebasePermissionsMap.get(codebase).entrySet()) {
>>> -                if (entry.getValue()) {
>>> -                    enabledPermissions.add(entry.getKey());
>>> +        FileLock fileLock = null;
>>> +        try {
>>> +            fileLock = FileUtils.getFileLock(file.getAbsolutePath(), false, true);
>>> +            final Set<PolicyEditorPermissions> enabledPermissions = new HashSet<>();
>>> +            for (final String codebase : codebasePermissionsMap.keySet()) {
>>> +                enabledPermissions.clear();
>>> +                for (final Map.Entry<PolicyEditorPermissions, Boolean> entry :
>>> codebasePermissionsMap.get(codebase).entrySet()) {
>>> +                    if (entry.getValue()) {
>>> +                        enabledPermissions.add(entry.getKey());
>>> +                    }
>>>                   }
>>> +                sb.append(new PolicyEntry(codebase, enabledPermissions,
>>> customPermissionsMap.get(codebase)).toString());
>>>               }
>>> -            sb.append(new PolicyEntry(codebase, enabledPermissions,
>>> customPermissionsMap.get(codebase)).toString());
>>> -        }
>>> -        try {
>>> -            fileLock.release();
>>>           } catch (final IOException e) {
>>>               OutputController.getLogger().log(e);
>>> +        } finally {
>>> +            if (fileLock != null) {
>>> +                try {
>>> +                    fileLock.release();
>>> +                } catch (final IOException e) {
>>> +                    OutputController.getLogger().log(e);
>>> +                }
>>> +
>>> +            }
>>>           }
>>>             FileUtils.saveFile(sb.toString(), file);
>>
>> Proposing to backport this for 1.5.1.
>>

Overlooked. Sure.
Go on. Please dont forget NEWS :) (were added later to head)