changeset 916:17c6e5a59602

PolicyEditor parsing enhancements, new tests, and bug fixes * NEWS: added entry for PolicyEditor * netx/net/sourceforge/jnlp/resources/Messages.properties: (PESaveAsMenuItemMnemonic, PEExitMenuItemMnemonic) changed mnemonic keys due to masking with ctrl rather than alt * netx/net/sourceforge/jnlp/security/policyeditor/CustomPermission.java: (ACTIONS_PERMISSION, TARGET_PERMISSION, fromString) use regexes to parse * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java: (file) keep reference to File rather than String filePath. (getPermissions) returns empty map rather than null. (setComponentMnemonic) new method. (getCustomPermissions) new function. (openAndParsePolicyFile) check for OpenFileResult FAILURE and NOT_FILE rather than null. (setupLayout) File, Save, SaveAs, and Exit items modifier mask changed to Ctrl rather than Alt * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissions.java: (fromString) use regexes to parse, using CustomPermission as intermediate representation * netx/net/sourceforge/jnlp/util/FileUtils.java: (testDirectoryPermissions) add check for getCanonicalFile and null safeguarding. (testFilePermissions) add check for getCanonicalFile and return FAILURE rather than null * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPermissionTest.java: (testMissingQuotationMarks) new test * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java: (testReturnedCustomPermissionsSetIsCopy, testCodebaseTrailingSlashesDoNotMatch) new tests * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorParsingTest.java: new tests
author Andrew Azores <aazores@redhat.com>
date Mon, 10 Mar 2014 12:17:56 -0400
parents 2157f0e06002
children 483ab446ea4c
files ChangeLog NEWS netx/net/sourceforge/jnlp/resources/Messages.properties netx/net/sourceforge/jnlp/security/policyeditor/CustomPermission.java netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissions.java netx/net/sourceforge/jnlp/util/FileUtils.java tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPermissionTest.java tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorParsingTest.java tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissionsTest.java tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
diffstat 11 files changed, 641 insertions(+), 115 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Mon Mar 10 11:57:12 2014 -0400
+++ b/ChangeLog	Mon Mar 10 12:17:56 2014 -0400
@@ -1,3 +1,33 @@
+2014-03-10  Andrew Azores  <aazores@redhat.com>
+
+	PolicyEditor parsing enhancements, new tests, and bugfixes
+	* NEWS: added entry for PolicyEditor
+	* netx/net/sourceforge/jnlp/resources/Messages.properties:
+	(PESaveAsMenuItemMnemonic, PEExitMenuItemMnemonic) changed mnemonic keys
+	due to masking with ctrl rather than alt
+	* netx/net/sourceforge/jnlp/security/policyeditor/CustomPermission.java:
+	(ACTIONS_PERMISSION, TARGET_PERMISSION, fromString) use regexes to parse
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java:
+	(file) keep reference to File rather than String filePath. (getPermissions)
+	returns empty map rather than null. (setComponentMnemonic) new method.
+	(getCustomPermissions) new function. (openAndParsePolicyFile) check for
+	OpenFileResult FAILURE and NOT_FILE rather than null. (setupLayout) File,
+	Save, SaveAs, and Exit items modifier mask changed to Ctrl rather than Alt
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissions.java:
+	(fromString) use regexes to parse, using CustomPermission as intermediate
+	representation
+	* netx/net/sourceforge/jnlp/util/FileUtils.java:
+	(testDirectoryPermissions) add check for getCanonicalFile and null
+	safeguarding. (testFilePermissions) add check for getCanonicalFile and
+	return FAILURE rather than null
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPermissionTest.java:
+	(testMissingQuotationMarks) new test
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java:
+	(testReturnedCustomPermissionsSetIsCopy,
+	testCodebaseTrailingSlashesDoNotMatch) new tests
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorParsingTest.java:
+	new tests
+
 2014-03-10  Omair Majid  <omajid@redhat.com>
 
 	* Makefile.am (install-data-local): Install itweb-settings.1.
--- a/NEWS	Mon Mar 10 11:57:12 2014 -0400
+++ b/NEWS	Mon Mar 10 12:17:56 2014 -0400
@@ -17,6 +17,7 @@
 * Support for u45 and u51 new manifest attributes (Application-Name, Codebase)
 * Custom applet permission policies panel in itweb-settings control panel
 * javaws -version flag
+* New PolicyEditor for easily adding/removing permissions to individual applets
 * Cache Viewer
   - Can be closed by ESC key
   - Enabling and disabling of operational buttons is handled properly
--- a/netx/net/sourceforge/jnlp/resources/Messages.properties	Mon Mar 10 11:57:12 2014 -0400
+++ b/netx/net/sourceforge/jnlp/resources/Messages.properties	Mon Mar 10 12:17:56 2014 -0400
@@ -519,10 +519,10 @@
 PEOpenMenuItemMnemonic=79
 # S
 PESaveMenuItemMnemonic=83
-# V
-PESaveAsMenuItemMnemonic=86
-# E
-PEExitMenuItemMnemonic=69
+# A
+PESaveAsMenuItemMnemonic=65
+# X
+PEExitMenuItemMnemonic=88
 # U
 PECustomPermissionsItemMnemonic=85
 
--- a/netx/net/sourceforge/jnlp/security/policyeditor/CustomPermission.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/CustomPermission.java	Mon Mar 10 12:17:56 2014 -0400
@@ -36,6 +36,9 @@
 
 package net.sourceforge.jnlp.security.policyeditor;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * This class models a permission entry in a policy file which is not included
  * in the default set of permissions used by the PolicyEditor, ie, permissions
@@ -43,6 +46,18 @@
  */
 public class CustomPermission implements Comparable<CustomPermission> {
 
+    /* Matches eg 'permission java.io.FilePermission "${user.home}${/}*", "read";'
+     * eg permissions that have a permission type, target, and actions set.
+     */
+    public static final Pattern ACTIONS_PERMISSION =
+            Pattern.compile("\\s*permission\\s+([\\w\\.]+)\\s+\"([^\"]+)\",\\s*\"([^\"]*)\";.*");
+
+    /* Matches eg 'permission java.lang.RuntimePermission "queuePrintJob";'
+     * eg permissions that have a permission type and target, but no actions.
+     */
+    public static final Pattern TARGET_PERMISSION =
+            Pattern.compile("\\s*permission\\s+([\\w\\.]+)\\s+\"([^\"]+)\";.*");
+
     public final String type, target, actions;
 
     /**
@@ -63,16 +78,23 @@
      * @return a CustomPermission representing this string
      */
     public static CustomPermission fromString(final String string) {
-        final String[] parts = string.trim().split(" ");
-        if (!parts[0].equals("permission") || parts.length < 3 || !string.trim().endsWith(";")) {
-            return null;
+        final String typeStr, targetStr, actionsStr;
+
+        final Matcher actionMatcher = ACTIONS_PERMISSION.matcher(string);
+        if (actionMatcher.matches()) {
+            typeStr = actionMatcher.group(1);
+            targetStr = actionMatcher.group(2);
+            actionsStr = actionMatcher.group(3);
+        } else {
+            final Matcher targetMatcher = TARGET_PERMISSION.matcher(string);
+            if (!targetMatcher.matches()) {
+                return null;
+            }
+            typeStr = targetMatcher.group(1);
+            targetStr = targetMatcher.group(2);
+            actionsStr = "";
         }
-        final String typeStr = removeQuotes(parts[1]);
-        final String targetStr = removeQuotes(removeSemicolon(removeComma(parts[2])));
-        String actionsStr = "";
-        if (parts.length > 3) {
-            actionsStr = removeQuotes(removeSemicolon(parts[3]));
-        }
+
         return new CustomPermission(typeStr, targetStr, actionsStr);
     }
 
@@ -96,18 +118,6 @@
         return sb.toString();
     }
 
-    private static String removeQuotes(final String string) {
-        return string.replaceAll("\"", "");
-    }
-
-    private static String removeSemicolon(final String string) {
-        return string.replaceAll(";", "");
-    }
-
-    private static String removeComma(final String string) {
-        return string.replaceAll(",", "");
-    }
-
     @Override
     public int compareTo(final CustomPermission o) {
         if (this == o) {
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Mon Mar 10 12:17:56 2014 -0400
@@ -56,6 +56,7 @@
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -66,6 +67,7 @@
 import java.util.regex.Pattern;
 
 import javax.swing.AbstractAction;
+import javax.swing.AbstractButton;
 import javax.swing.Action;
 import javax.swing.DefaultListModel;
 import javax.swing.JButton;
@@ -146,7 +148,7 @@
 
     private static final String AUTOGENERATED_NOTICE = "/* DO NOT MODIFY! AUTO-GENERATED */";
 
-    private String filePath;
+    private File file;
     private boolean changesMade = false;
     private boolean closed = false;
     private final Map<String, Map<PolicyEditorPermissions, Boolean>> codebasePermissionsMap = new HashMap<String, Map<PolicyEditorPermissions, Boolean>>();
@@ -169,10 +171,6 @@
         super();
         setLayout(new GridBagLayout());
 
-        this.filePath = filepath;
-
-        fileChooser = new JFileChooser(filePath);
-
         for (final PolicyEditorPermissions perm : PolicyEditorPermissions.values()) {
             final JCheckBox box = new JCheckBox();
             box.setText(perm.getName());
@@ -180,16 +178,14 @@
             checkboxMap.put(perm, box);
         }
 
-        if (filePath != null) {
-            try {
-                new URL("file://" + filePath);
-                openAndParsePolicyFile();
-            } catch (final MalformedURLException e) {
-                OutputController.getLogger().log(e);
-                FileUtils.showCouldNotOpenFilepathDialog(weakThis.get(), filepath);
-            }
+        if (filepath != null) {
+            file = new File(filepath);
+            openAndParsePolicyFile();
         }
 
+        fileChooser = new JFileChooser(file);
+        fileChooser.setFileHidingEnabled(false);
+
         initializeMapForCodebase("");
         listModel.addElement(R("PEGlobalSettings"));
         updateCheckboxes("");
@@ -197,15 +193,15 @@
         okButtonAction = new ActionListener() {
             @Override
             public void actionPerformed(final ActionEvent event) {
-                if (filePath == null) {
+                if (file == null) {
                     final int choice = fileChooser.showOpenDialog(weakThis.get());
                     if (choice == JFileChooser.APPROVE_OPTION) {
-                        filePath = fileChooser.getSelectedFile().getAbsolutePath();
+                        file = fileChooser.getSelectedFile();
                     }
                 }
 
                 // May still be null if user cancelled the file chooser
-                if (filePath != null) {
+                if (file != null) {
                     savePolicyFile();
                 }
             }
@@ -253,7 +249,7 @@
                 }
                 final int choice = fileChooser.showOpenDialog(weakThis.get());
                 if (choice == JFileChooser.APPROVE_OPTION) {
-                    filePath = fileChooser.getSelectedFile().getAbsolutePath();
+                    file = fileChooser.getSelectedFile();
                     openAndParsePolicyFile();
                 }
             }
@@ -264,7 +260,7 @@
             public void actionPerformed(final ActionEvent e) {
                 final int choice = fileChooser.showSaveDialog(weakThis.get());
                 if (choice == JFileChooser.APPROVE_OPTION) {
-                    filePath = fileChooser.getSelectedFile().getAbsolutePath();
+                    file = fileChooser.getSelectedFile();
                     savePolicyFile();
                 }
             }
@@ -315,10 +311,18 @@
         closed = true;
     }
 
+    /**
+     * Check if the PolicyEditor instance has been visually closed
+     * @return if the PolicyEditor instance has been closed
+     */
     public boolean isClosed() {
         return closed;
     }
 
+    /**
+     * Called by the Custom Policy Viewer on its parent Policy Editor when
+     * the Custom Policy Viewer is closing
+     */
     void customPolicyViewerClosing() {
         cpViewer = null;
     }
@@ -341,6 +345,26 @@
      * @param action to be performed
      * @param identifier an identifier for the action
      */
+    private void setAccelerator(final String trigger, final int modifiers, final Action action, final String identifier) {
+        final int trig;
+        try {
+            trig = Integer.parseInt(trigger);
+        } catch (final NumberFormatException nfe) {
+            OutputController.getLogger().log("Unable to set accelerator action \""
+                    + identifier + "\" for trigger \"" + trigger + "\"");
+            OutputController.getLogger().log(nfe);
+            return;
+        }
+        setAccelerator(trig, modifiers, action, identifier);
+    }
+
+    /**
+     * Set a key accelerator
+     * @param trigger the accelerator key
+     * @param modifiers Alt, Ctrl, or other modifiers to be held with the trigger
+     * @param action to be performed
+     * @param identifier an identifier for the action
+     */
     private void setAccelerator(final int trigger, final int modifiers, final Action action, final String identifier) {
         final KeyStroke key = KeyStroke.getKeyStroke(trigger, modifiers);
         final JRootPane root = getRootPane();
@@ -371,7 +395,7 @@
                 interactivelyAddCodebase();
             }
         };
-        setAccelerator(Integer.parseInt(R("PEAddCodebaseMnemonic")), ActionEvent.ALT_MASK, act, "AddCodebaseAccelerator");
+        setAccelerator(R("PEAddCodebaseMnemonic"), ActionEvent.ALT_MASK, act, "AddCodebaseAccelerator");
     }
 
     /**
@@ -384,7 +408,7 @@
                 removeCodebase((String) list.getSelectedValue());
             }
         };
-        setAccelerator(Integer.parseInt(R("PERemoveCodebaseMnemonic")), ActionEvent.ALT_MASK, act, "RemoveCodebaseAccelerator");
+        setAccelerator(R("PERemoveCodebaseMnemonic"), ActionEvent.ALT_MASK, act, "RemoveCodebaseAccelerator");
     }
 
     /**
@@ -397,7 +421,7 @@
                 savePolicyFile();
             }
         };
-        setAccelerator(Integer.parseInt(R("PEOkButtonMnemonic")), ActionEvent.ALT_MASK, act, "OkButtonAccelerator");
+        setAccelerator(R("PEOkButtonMnemonic"), ActionEvent.ALT_MASK, act, "OkButtonAccelerator");
     }
 
     /**
@@ -410,7 +434,7 @@
                 quit();
             }
         };
-        setAccelerator(Integer.parseInt(R("PECancelButtonMnemonic")), ActionEvent.ALT_MASK, act, "CancelButtonAccelerator");
+        setAccelerator(R("PECancelButtonMnemonic"), ActionEvent.ALT_MASK, act, "CancelButtonAccelerator");
     }
 
     /**
@@ -533,7 +557,29 @@
      * @return a map of permissions to whether these permissions are set for the given codebase
      */
     public Map<PolicyEditorPermissions, Boolean> getPermissions(final String codebase) {
-        return new HashMap<PolicyEditorPermissions, Boolean>(codebasePermissionsMap.get(codebase));
+        final Map<PolicyEditorPermissions, Boolean> permissions = codebasePermissionsMap.get(codebase);
+        if (permissions != null) {
+            return new HashMap<PolicyEditorPermissions, Boolean>(permissions);
+        } else {
+            final Map<PolicyEditorPermissions, Boolean> blank = new HashMap<PolicyEditorPermissions, Boolean>();
+            for (final PolicyEditorPermissions perm : PolicyEditorPermissions.values()) {
+                blank.put(perm, false);
+            }
+            return blank;
+        }
+    }
+
+    /**
+     * @param codebase the codebase to query
+     * @return a collection of CustomPermissions granted to the given codebase
+     */
+    public Collection<CustomPermission> getCustomPermissions(final String codebase) {
+        final Collection<CustomPermission> permissions = customPermissionsMap.get(codebase);
+        if (permissions != null) {
+            return new HashSet<CustomPermission>(permissions);
+        } else {
+            return Collections.emptySet();
+        }
     }
 
     /**
@@ -558,37 +604,53 @@
     }
 
     /**
+     * Set a mnemonic key for a menu item or button
+     * @param component the component for which to set a mnemonic
+     * @param mnemonic the mnemonic to set
+     */
+    private void setComponentMnemonic(final AbstractButton component, final String mnemonic) {
+        final int trig;
+        try {
+            trig = Integer.parseInt(mnemonic);
+        } catch (final NumberFormatException nfe) {
+            OutputController.getLogger().log(nfe);
+            return;
+        }
+        component.setMnemonic(trig);
+    }
+
+    /**
      * Lay out all controls, tooltips, etc.
      */
     private void setupLayout() {
         final JMenu fileMenu = new JMenu(R("PEFileMenu"));
-        fileMenu.setMnemonic(Integer.parseInt(R("PEFileMenuMnemonic")));
+        setComponentMnemonic(fileMenu, R("PEFileMenuMnemonic"));
         final JMenuItem openItem = new JMenuItem(R("PEOpenMenuItem"));
-        openItem.setMnemonic(Integer.parseInt(R("PEOpenMenuItemMnemonic")));
-        openItem.setAccelerator(KeyStroke.getKeyStroke(openItem.getMnemonic(), ActionEvent.ALT_MASK));
+        setComponentMnemonic(openItem, R("PEOpenMenuItemMnemonic"));
+        openItem.setAccelerator(KeyStroke.getKeyStroke(openItem.getMnemonic(), ActionEvent.CTRL_MASK));
         openItem.addActionListener(openButtonAction);
         fileMenu.add(openItem);
         final JMenuItem saveItem = new JMenuItem(R("PESaveMenuItem"));
-        saveItem.setMnemonic(Integer.parseInt(R("PESaveMenuItemMnemonic")));
-        saveItem.setAccelerator(KeyStroke.getKeyStroke(saveItem.getMnemonic(), ActionEvent.ALT_MASK));
+        setComponentMnemonic(saveItem, R("PESaveMenuItemMnemonic"));
+        saveItem.setAccelerator(KeyStroke.getKeyStroke(saveItem.getMnemonic(), ActionEvent.CTRL_MASK));
         saveItem.addActionListener(okButtonAction);
         fileMenu.add(saveItem);
         final JMenuItem saveAsItem = new JMenuItem(R("PESaveAsMenuItem"));
-        saveAsItem.setMnemonic(Integer.parseInt(R("PESaveAsMenuItemMnemonic")));
-        saveAsItem.setAccelerator(KeyStroke.getKeyStroke(saveAsItem.getMnemonic(), ActionEvent.ALT_MASK));
+        setComponentMnemonic(saveAsItem, R("PESaveAsMenuItemMnemonic"));
+        saveAsItem.setAccelerator(KeyStroke.getKeyStroke(saveAsItem.getMnemonic(), ActionEvent.CTRL_MASK));
         saveAsItem.addActionListener(saveAsButtonAction);
         fileMenu.add(saveAsItem);
         final JMenuItem exitItem = new JMenuItem(R("PEExitMenuItem"));
-        exitItem.setMnemonic(Integer.parseInt(R("PEExitMenuItemMnemonic")));
-        exitItem.setAccelerator(KeyStroke.getKeyStroke(exitItem.getMnemonic(), ActionEvent.ALT_MASK));
+        setComponentMnemonic(exitItem, R("PEExitMenuItemMnemonic"));
+        exitItem.setAccelerator(KeyStroke.getKeyStroke(exitItem.getMnemonic(), ActionEvent.CTRL_MASK));
         exitItem.addActionListener(closeButtonAction);
         fileMenu.add(exitItem);
         menuBar.add(fileMenu);
 
         final JMenu viewMenu = new JMenu(R("PEViewMenu"));
-        viewMenu.setMnemonic(Integer.parseInt(R("PEViewMenuMnemonic")));
+        setComponentMnemonic(viewMenu, R("PEViewMenuMnemonic"));
         final JMenuItem customPermissionsItem = new JMenuItem(R("PECustomPermissionsItem"));
-        customPermissionsItem.setMnemonic(Integer.parseInt(R("PECustomPermissionsItemMnemonic")));
+        setComponentMnemonic(customPermissionsItem, R("PECustomPermissionsItemMnemonic"));
         customPermissionsItem.setAccelerator(KeyStroke.getKeyStroke(customPermissionsItem.getMnemonic(), ActionEvent.ALT_MASK));
         customPermissionsItem.addActionListener(viewCustomButtonAction);
 
@@ -668,34 +730,40 @@
         addCodebaseButtonConstraints.fill = GridBagConstraints.HORIZONTAL;
         addCodebaseButtonConstraints.gridx = 0;
         addCodebaseButtonConstraints.gridy = listConstraints.gridy + listConstraints.gridheight + 1;
-        addCodebaseButton.setMnemonic(Integer.parseInt(R("PEAddCodebaseMnemonic")));
+        setComponentMnemonic(addCodebaseButton, R("PEAddCodebaseMnemonic"));
         add(addCodebaseButton, addCodebaseButtonConstraints);
 
         final GridBagConstraints removeCodebaseButtonConstraints = new GridBagConstraints();
         removeCodebaseButtonConstraints.fill = GridBagConstraints.HORIZONTAL;
         removeCodebaseButtonConstraints.gridx = addCodebaseButtonConstraints.gridx + 1;
         removeCodebaseButtonConstraints.gridy = addCodebaseButtonConstraints.gridy;
-        removeCodebaseButton.setMnemonic(Integer.parseInt(R("PERemoveCodebaseMnemonic")));
+        setComponentMnemonic(removeCodebaseButton, R("PERemoveCodebaseMnemonic"));
         add(removeCodebaseButton, removeCodebaseButtonConstraints);
 
         final GridBagConstraints okButtonConstraints = new GridBagConstraints();
         okButtonConstraints.fill = GridBagConstraints.HORIZONTAL;
         okButtonConstraints.gridx = removeCodebaseButtonConstraints.gridx + 2;
         okButtonConstraints.gridy = removeCodebaseButtonConstraints.gridy;
-        okButton.setMnemonic(Integer.parseInt(R("PEOkButtonMnemonic")));
+        setComponentMnemonic(okButton, R("PEOkButtonMnemonic"));
         add(okButton, okButtonConstraints);
 
         final GridBagConstraints cancelButtonConstraints = new GridBagConstraints();
         cancelButtonConstraints.fill = GridBagConstraints.HORIZONTAL;
         cancelButtonConstraints.gridx = okButtonConstraints.gridx + 1;
         cancelButtonConstraints.gridy = okButtonConstraints.gridy;
-        closeButton.setMnemonic(Integer.parseInt(R("PECancelButtonMnemonic")));
+        setComponentMnemonic(closeButton, R("PECancelButtonMnemonic"));
         add(closeButton, cancelButtonConstraints);
 
         setMinimumSize(getPreferredSize());
         pack();
     }
 
+    /**
+     * Update the custom permissions map. Used by the Custom Policy Viewer to update its parent
+     * PolicyEditor to changes it has made
+     * @param codebase the codebase for which changes were made
+     * @param permissions the permissions granted to this codebase
+     */
     void updateCustomPermissions(final String codebase, final Collection<CustomPermission> permissions) {
         changesMade = true;
         customPermissionsMap.get(codebase).clear();
@@ -710,9 +778,17 @@
         new Thread() {
             @Override
             public void run() {
-                OpenFileResult ofr = FileUtils.testFilePermissions(new File(filePath));
-                if (ofr == null) {
-                    FileUtils.showCouldNotOpenFilepathDialog(weakThis.get(), filePath);
+                if (!file.exists()) {
+                    try {
+                        file.createNewFile();
+                    } catch (final IOException e) {
+                        OutputController.getLogger().log(e);
+                        // If this fails we'll end up handling it a few lines down anyway.
+                    }
+                }
+                OpenFileResult ofr = FileUtils.testFilePermissions(file);
+                if (ofr == OpenFileResult.FAILURE || ofr == OpenFileResult.NOT_FILE) {
+                    FileUtils.showCouldNotOpenFilepathDialog(weakThis.get(), file.getPath());
                     return;
                 }
                 if (ofr == OpenFileResult.CANT_WRITE) {
@@ -720,24 +796,25 @@
                 }
                 final String contents;
                 try {
-                    contents = FileUtils.loadFileAsString(new File(filePath));
+                    contents = FileUtils.loadFileAsString(file);
                 } catch (final IOException e) {
                     OutputController.getLogger().log(e);
-                    OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RCantOpenFile", filePath));
+                    OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RCantOpenFile", file.getPath()));
                     FileUtils.showCouldNotOpenDialog(weakThis.get(), R("PECouldNotOpen"));
                     return;
                 }
                 // Split on newlines, both \r\n and \n style, for platform-independence
-                final String[] lines = contents.split("[\\r\\n]+");
+                final String[] lines = contents.split("\\r?\\n+");
                 String codebase = "";
                 FileLock fileLock = null;
                 try {
-                    fileLock = FileUtils.getFileLock(filePath, false, true);
+                    fileLock = FileUtils.getFileLock(file.getAbsolutePath(), false, true);
                 } catch (final FileNotFoundException e) {
                     OutputController.getLogger().log(e);
                     FileUtils.showCouldNotOpenDialog(weakThis.get(), R("PECouldNotOpen"));
                     return;
                 }
+                boolean openBlock = false, commentBlock = false;
                 for (final String line : lines) {
                     // Matches eg `grant {` as well as `grant codeBase "http://redhat.com" {`
                     final Pattern openBlockPattern = Pattern.compile("grant\\s*\"?\\s*(?:codeBase)?\\s*\"?([^\"\\s]*)\"?\\s*\\{");
@@ -747,17 +824,29 @@
                         codebase = openBlockMatcher.group(1);
                         initializeMapForCodebase(codebase);
                         listModel.addElement(codebase);
-                        continue;
+                        openBlock = true;
                     }
 
+                    boolean commentLine = false;
                     if (line.matches("\\s*\\};\\s*")) {
-                        continue;
-                    } else if (line.matches("\\s*/\\*.*")) {
-                        continue;
-                    } else if (line.matches(".*\\*/.*") || line.matches("\\s*//.*")) {
+                        openBlock = false;
+                    }
+                    if (line.matches(".*/\\*.*")) {
+                        commentBlock = true;
+                    }
+                    if (line.matches(".*\\*/.*")) {
+                        commentBlock = false;
+                    }
+                    if (line.matches(".*/\\*.*") && line.matches(".*\\*/.*")) {
+                        commentLine = true;
+                    }
+                    if (line.matches("\\s*//.*")) {
+                        commentLine = true;
+                    }
+
+                    if (!openBlock || commentBlock || commentLine) {
                         continue;
                     }
-
                     final PolicyEditorPermissions perm = PolicyEditorPermissions.fromString(line);
                     if (perm != null) {
                         codebasePermissionsMap.get(codebase).put(perm, true);
@@ -816,7 +905,7 @@
                 final Set<PolicyEditorPermissions> enabledPermissions = new HashSet<PolicyEditorPermissions>();
                 FileLock fileLock = null;
                 try {
-                    fileLock = FileUtils.getFileLock(filePath, false, true);
+                    fileLock = FileUtils.getFileLock(file.getAbsolutePath(), false, true);
                 } catch (final FileNotFoundException e) {
                     OutputController.getLogger().log(e);
                     showCouldNotSaveDialog();
@@ -838,11 +927,11 @@
                 }
 
                 try {
-                    FileUtils.saveFile(sb.toString(), new File(filePath));
+                    FileUtils.saveFile(sb.toString(), file);
                     changesMade = false;
                     showChangesSavedDialog();
                 } catch (final IOException e) {
-                    OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RCantWriteFile", filePath));
+                    OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RCantWriteFile", file.getPath()));
                     showCouldNotSaveDialog();
                 }
             }
@@ -916,10 +1005,21 @@
         });
     }
 
+    /**
+     * Create a new PolicyEditor instance without passing argv. The returned instance is not
+     * yet set visible.
+     * @param filepath a policy file to open at start, or null if no file to load
+     * @return a reference to a new PolicyEditor instance
+     */
     public static PolicyEditor createInstance(final String filepath) {
         return new PolicyEditor(filepath);
     }
 
+    /**
+     * Create a Map out of argv
+     * @param args command line flags and parameters given to the program
+     * @return a Map representation of the command line arguments
+     */
     static Map<String, String> argsToMap(final String[] args) {
         final List<String> argsList = Arrays.<String> asList(args);
         final Map<String, String> map = new HashMap<String, String>();
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissions.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissions.java	Mon Mar 10 12:17:56 2014 -0400
@@ -38,6 +38,9 @@
 
 import static net.sourceforge.jnlp.runtime.Translator.R;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * Defines the set of default permissions for PolicyEditor, ie the ones which are assigned
  * dedicated checkboxes
@@ -159,19 +162,14 @@
      * @return the PolicyEditorPermissions value matching the input String, or null if no such match is found
      */
     public static PolicyEditorPermissions fromString(final String string) {
-        final String[] parts = string.split(" ");
-        if (parts.length < 3) {
+        final CustomPermission tmpPerm = CustomPermission.fromString(string);
+        if (tmpPerm == null) {
             return null;
         }
-        final String typeStr = removeQuotes(parts[1]);
-        final String targetStr = removeQuotes(removeSemicolon(removeComma(parts[2])));
-        String actionsStr = "";
-        if (parts.length > 3) {
-            actionsStr = removeQuotes(removeSemicolon(parts[3]));
-        }
-        final PermissionType type = PermissionType.fromString(typeStr);
-        final PermissionTarget target = PermissionTarget.fromString(targetStr);
-        final PermissionActions actions = PermissionActions.fromString(actionsStr);
+
+        final PermissionType type = PermissionType.fromString(tmpPerm.type);
+        final PermissionTarget target = PermissionTarget.fromString(tmpPerm.target);
+        final PermissionActions actions = PermissionActions.fromString(tmpPerm.actions);
 
         for (final PolicyEditorPermissions perm : PolicyEditorPermissions.values()) {
             final boolean sameType = perm.type.equals(type);
@@ -185,19 +183,4 @@
         return null;
     }
 
-    private static String removeQuotes(final String string) {
-        return string.replaceAll("\"", "");
-    }
-
-    private static String removeSemicolon(final String string) {
-        if (string.contains(";")) {
-            return string.substring(0, string.lastIndexOf(';'));
-        } else {
-            return string;
-        }
-    }
-
-    private static String removeComma(final String string) {
-        return string.replaceAll(",", "");
-    }
 }
--- a/netx/net/sourceforge/jnlp/util/FileUtils.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/netx/net/sourceforge/jnlp/util/FileUtils.java	Mon Mar 10 12:17:56 2014 -0400
@@ -285,8 +285,14 @@
      * @param file the {@link File} representing a Java Policy file to test
      * @return a {@link DirectoryCheckResults} object representing the results of the test
      */
-    public static DirectoryCheckResults testDirectoryPermissions(final File file) {
-        if (file == null || !file.getParentFile().exists()) {
+    public static DirectoryCheckResults testDirectoryPermissions(File file) {
+        try {
+            file = file.getCanonicalFile();
+        } catch (final IOException e) {
+            OutputController.getLogger().log(e);
+            return null;
+        }
+        if (file == null || file.getParentFile() == null || !file.getParentFile().exists()) {
             return null;
         }
         final List<File> policyDirectory = new ArrayList<File>();
@@ -299,15 +305,17 @@
 
     /**
      * Verify that a given file object points to a real, accessible plain file.
-     * As a side effect, if the file is accessible but does not yet exist, it will be created
-     * as an empty plain file.
      * @param file the {@link File} to verify
      * @return an {@link OpenFileResult} representing the accessibility level of the file
-     * @throws IOException if the file is not accessible
      */
-    public static OpenFileResult testFilePermissions(final File file) {
+    public static OpenFileResult testFilePermissions(File file) {
         if (file == null || !file.exists()) {
-            return null;
+            return OpenFileResult.FAILURE;
+        }
+        try {
+            file = file.getCanonicalFile();
+        } catch (final IOException e) {
+            return OpenFileResult.FAILURE;
         }
         final DirectoryCheckResults dcr = FileUtils.testDirectoryPermissions(file);
         if (dcr != null && dcr.getFailures() == 0) {
--- a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPermissionTest.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPermissionTest.java	Mon Mar 10 12:17:56 2014 -0400
@@ -37,6 +37,7 @@
 package net.sourceforge.jnlp.security.policyeditor;
 
 import static org.junit.Assert.assertTrue;
+import net.sourceforge.jnlp.security.policyeditor.CustomPermission;
 
 import org.junit.Test;
 
@@ -67,6 +68,47 @@
     }
 
     @Test
+    public void testMissingQuotationMarks() throws Exception {
+        final CustomPermission cp = CustomPermission.fromString("permission java.io.FilePermission *, read,write;");
+        assertTrue("Custom permission should be null", cp == null);
+    }
+
+    @Test
+    public void testActionsMissingComma() throws Exception {
+        final String missingComma = "permission java.io.FilePermission \"*\" \"read,write\";";
+        final CustomPermission cp1 = CustomPermission.fromString(missingComma);
+        assertTrue("Custom permission for " + missingComma + " should be null", cp1 == null);
+    }
+
+    @Test
+    public void testActionsMissingFirstQuote() throws Exception {
+        final String missingFirstQuote = "permission java.io.FilePermission \"*\", read,write\";";
+        final CustomPermission cp2 = CustomPermission.fromString(missingFirstQuote);
+        assertTrue("Custom permission for " + missingFirstQuote + " should be null", cp2 == null);
+    }
+
+    @Test
+    public void testActionsMissingSecondQuote() throws Exception {
+        final String missingSecondQuote = "permission java.io.FilePermission \"*\", \"read,write;";
+        final CustomPermission cp3 = CustomPermission.fromString(missingSecondQuote);
+        assertTrue("Custom permission for " + missingSecondQuote + " should be null", cp3 == null);
+    }
+
+    @Test
+    public void testActionsMissingBothQuotes() throws Exception {
+        final String missingBothQuotes = "permission java.io.FilePermission \"*\", read,write;";
+        final CustomPermission cp4 = CustomPermission.fromString(missingBothQuotes);
+        assertTrue("Custom permission for " + missingBothQuotes + " should be null", cp4 == null);
+    }
+
+    @Test
+    public void testActionsMissingAllPunctuation() throws Exception {
+        final String missingAll = "permission java.io.FilePermission \"*\" read,write;";
+        final CustomPermission cp5 = CustomPermission.fromString(missingAll);
+        assertTrue("Custom permission for " + missingAll + " should be null", cp5 == null);
+    }
+
+    @Test
     public void testToString() throws Exception {
         final CustomPermission cp = new CustomPermission("java.io.FilePermission", "*", "read");
         final String expected = "permission java.io.FilePermission \"*\", \"read\";";
@@ -78,8 +120,8 @@
         final CustomPermission cp1 = new CustomPermission("java.io.FilePermission", "*", "read");
         final CustomPermission cp2 = new CustomPermission("java.io.FilePermission", "${user.home}${/}*", "read");
         final CustomPermission cp3 = new CustomPermission("java.lang.RuntimePermission", "queuePrintJob", "");
-        assertTrue(cp1.compareTo(cp2) > 0);
-        assertTrue(cp1.compareTo(cp1) == 0);
-        assertTrue(cp2.compareTo(cp3) < 0);
+        assertTrue("cp1.compareTo(cp2) should be > 0", cp1.compareTo(cp2) > 0);
+        assertTrue("cp1.compareTo(cp1) should be 0", cp1.compareTo(cp1) == 0);
+        assertTrue("cp2.compareTo(cp3) should be < 0", cp2.compareTo(cp3) < 0);
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorParsingTest.java	Mon Mar 10 12:17:56 2014 -0400
@@ -0,0 +1,290 @@
+/*Copyright (C) 2014 Red Hat, Inc.
+
+This file is part of IcedTea.
+
+IcedTea is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License as published by
+the Free Software Foundation, version 2.
+
+IcedTea is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with IcedTea; see the file COPYING.  If not, write to
+the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version.
+ */
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.util.Map;
+import net.sourceforge.jnlp.annotations.KnownToFail;
+import net.sourceforge.jnlp.security.policyeditor.PolicyEditor;
+import net.sourceforge.jnlp.security.policyeditor.PolicyEditorPermissions;
+import net.sourceforge.jnlp.util.FileUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class PolicyEditorParsingTest {
+
+    private File file;
+    private PolicyEditor editor;
+    private Map<PolicyEditorPermissions, Boolean> permissions;
+
+    private static final String LINEBREAK = System.getProperty("line.separator");
+
+    private static final String READ_PERMISSION = "permission java.io.FilePermission \"${user.home}${/}*\", \"read\";";
+    private static final String WRITE_PERMISSION = "permission java.io.FilePermission \"${user.home}${/}*\", \"write\";";
+    private static final String COMMENT_HEADER = "/* TEST COMMENT */" + LINEBREAK;
+
+    private static final String NORMAL_POLICY = "grant {" + LINEBREAK
+        + "\t" + READ_PERMISSION + LINEBREAK
+        + "};" + LINEBREAK;
+
+    private static final String NORMAL_POLICY_CRLF = "grant {" + "\r\n"
+            + "\t" + READ_PERMISSION + "\r\n"
+            + "};" + "\r\n";
+
+    private static final String NORMAL_POLICY_LF = "grant {" + "\n"
+            + "\t" + READ_PERMISSION + "\n"
+            + "};" + "\n";
+
+    private static final String NORMAL_POLICY_MIXED_ENDINGS = "grant {" + "\n\n"
+            + "\t" + READ_PERMISSION + "\r\n"
+            + "};" + "\n";
+
+    private static final String NORMAL_POLICY_WITH_HEADER = COMMENT_HEADER + NORMAL_POLICY;
+
+    private static final String CODEBASE_POLICY = "grant codeBase \"http://example.com\" {" + LINEBREAK
+        + "\t" + READ_PERMISSION + LINEBREAK
+        + "};" + LINEBREAK;
+
+    private static final String MULTIPLE_PERMISSION_POLICY = "grant {" + LINEBREAK
+        + "\t" + READ_PERMISSION + LINEBREAK
+        + "\t" + WRITE_PERMISSION + LINEBREAK
+        + "};" + LINEBREAK;
+
+    private static final String COMMENT_BLOCKED_PERMISSION = "grant {" + LINEBREAK
+        + "\t/*" + READ_PERMISSION + "*/" + LINEBREAK
+        + "};" + LINEBREAK;
+
+    private static final String COMMENT_BLOCKED_POLICY = "/*" + NORMAL_POLICY + "*/" + LINEBREAK;
+
+    private static final String COMMENTED_PERMISSION = "grant {" + LINEBREAK
+        + "\t//" + READ_PERMISSION + LINEBREAK
+        + "};" + LINEBREAK;
+
+    private static final String COMMENT_AFTER_PERMISSION = "grant {" + LINEBREAK
+        + "\t" + READ_PERMISSION + " // comment" + LINEBREAK
+        + "};" + LINEBREAK;
+
+    private static final String MISSING_WHITESPACE_POLICY = "grant { " + READ_PERMISSION + " };";
+
+    private static final String MULTIPLE_PERMISSIONS_PER_LINE = "grant {" + LINEBREAK
+            + "\t" + READ_PERMISSION + " " + WRITE_PERMISSION + LINEBREAK
+            + "};" + LINEBREAK;
+
+    @Before
+    public void createTempFile() throws Exception {
+        file = File.createTempFile("PolicyEditor", ".policy");
+        file.deleteOnExit();
+    }
+
+    private void setupTest(final String policyContents, final String codebase) throws Exception {
+        FileUtils.saveFile(policyContents, file);
+        editor = PolicyEditor.createInstance(file.getCanonicalPath());
+        Thread.sleep(100); // policy editor loads asynch, give it some time to populate
+        permissions = editor.getPermissions(codebase);
+    }
+
+    @Test
+    public void testNormalPolicy() throws Exception {
+        setupTest(NORMAL_POLICY, "");
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testNormalPolicyWithCRLFEndings() throws Exception {
+        // This is the same test as testNormalPolicy on systems where the line separator is \r\n
+        setupTest(NORMAL_POLICY_CRLF, "");
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testNormalPolicyWithLFEndings() throws Exception {
+        // This is the same test as testNormalPolicy on systems where the line separator is \n
+        setupTest(NORMAL_POLICY_LF, "");
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testNormalPolicyWithMixedEndings() throws Exception {
+        // This is the same test as testNormalPolicy on systems where the line separator is \n
+        setupTest(NORMAL_POLICY_MIXED_ENDINGS, "");
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testCommentHeaders() throws Exception {
+        setupTest(COMMENT_HEADER, "");
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+        }
+    }
+
+    @Test
+    public void testCommentBlockedPermission() throws Exception {
+        setupTest(COMMENT_BLOCKED_PERMISSION, "");
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+        }
+    }
+
+    @Test
+    public void testCommentBlockedPolicy() throws Exception {
+        setupTest(COMMENT_BLOCKED_POLICY, "");
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+        }
+    }
+
+    @Test
+    public void testCommentedLine() throws Exception {
+        setupTest(COMMENTED_PERMISSION, "");
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+        }
+    }
+
+    @Test
+    public void testMultiplePermissions() throws Exception {
+        setupTest(MULTIPLE_PERMISSION_POLICY, "");
+
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        assertTrue("Permissions should include WRITE_LOCAL_FILES", permissions.get(PolicyEditorPermissions.WRITE_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES) && !perm.equals(PolicyEditorPermissions.WRITE_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @KnownToFail
+    @Test
+    public void testMultiplePermissionsPerLine() throws Exception {
+        setupTest(MULTIPLE_PERMISSIONS_PER_LINE, "");
+
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        assertTrue("Permissions should include WRITE_LOCAL_FILES", permissions.get(PolicyEditorPermissions.WRITE_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES) && !perm.equals(PolicyEditorPermissions.WRITE_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @KnownToFail
+    @Test
+    public void testMissingWhitespace() throws Exception {
+        setupTest(MISSING_WHITESPACE_POLICY, "");
+
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testPolicyWithCodebase() throws Exception {
+        setupTest(CODEBASE_POLICY, "http://example.com");
+
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testCodebaseTrailingSlashesDoNotMatch() throws Exception {
+        // note the trailing '/' - looks like the same URL but is not. JDK PolicyTool considers these as
+        // different codeBases, so so does PolicyEditor
+        setupTest(CODEBASE_POLICY, "http://example.com/");
+
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testCommentAfterPermission() throws Exception {
+        setupTest(COMMENT_AFTER_PERMISSION, "");
+
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+    @Test
+    public void testNormalPolicyWithHeader() throws Exception {
+        setupTest(NORMAL_POLICY_WITH_HEADER, "");
+        assertTrue("Permissions should include READ_LOCAL_FILES", permissions.get(PolicyEditorPermissions.READ_LOCAL_FILES));
+        for (final PolicyEditorPermissions perm : permissions.keySet()) {
+            if (!perm.equals(PolicyEditorPermissions.READ_LOCAL_FILES)) {
+                assertFalse("Permission " + perm + " should not be granted", permissions.get(perm));
+            }
+        }
+    }
+
+}
--- a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissionsTest.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorPermissionsTest.java	Mon Mar 10 12:17:56 2014 -0400
@@ -37,6 +37,11 @@
 package net.sourceforge.jnlp.security.policyeditor;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.regex.Pattern;
+
+import net.sourceforge.jnlp.security.policyeditor.PolicyEditorPermissions;
 
 import org.junit.Test;
 
@@ -63,4 +68,40 @@
                     perm.toPermissionString().trim().isEmpty());
         }
     }
+
+    @Test
+    public void testActionsRegex() throws Exception {
+    	final Pattern pattern = PolicyEditorPermissions.ACTIONS_PERMISSION;
+
+    	final String actionsPermission = "permission java.io.FilePermission \"${user.home}\", \"read\";";
+    	final String targetPermission = "permission java.io.RuntimePermission \"queuePrintJob\";";
+    	final String badPermission = "permission java.io.FilePermission user.home read;";
+
+    	assertTrue(actionsPermission + " should match", pattern.matcher(actionsPermission).matches());
+    	assertFalse(targetPermission + " should not match", pattern.matcher(targetPermission).matches());
+    	assertFalse(badPermission + " should not match", pattern.matcher(badPermission).matches());
+    }
+
+    @Test
+    public void testTargetRegex() throws Exception {
+    	final Pattern pattern = PolicyEditorPermissions.TARGET_PERMISSION;
+
+    	final String actionsPermission = "permission java.io.FilePermission \"${user.home}\", \"read\";";
+    	final String targetPermission = "permission java.io.RuntimePermission \"queuePrintJob\";";
+    	final String badPermission = "permission java.io.FilePermission user.home read;";
+
+    	assertFalse(actionsPermission + " should not match", pattern.matcher(actionsPermission).matches());
+    	assertTrue(targetPermission + " should match", pattern.matcher(targetPermission).matches());
+    	assertFalse(badPermission + " should not match", pattern.matcher(badPermission).matches());
+    }
+
+    @Test
+    public void testRegexesAgainstBadPermissionNames() throws Exception {
+    	final Pattern targetPattern = PolicyEditorPermissions.TARGET_PERMISSION;
+    	final Pattern actionsPattern = PolicyEditorPermissions.ACTIONS_PERMISSION;
+    	final String badPermission = "permission abc123^$% \"target\", \"actions\"";
+
+    	assertFalse(badPermission + " should not match", targetPattern.matcher(badPermission).matches());
+    	assertFalse(badPermission + " should not match", actionsPattern.matcher(badPermission).matches());
+    }
 }
--- a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java	Mon Mar 10 11:57:12 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java	Mon Mar 10 12:17:56 2014 -0400
@@ -122,6 +122,14 @@
     }
 
     @Test
+    public void testReturnedCustomPermissionsSetIsCopy() throws Exception {
+        final Collection<CustomPermission> original = editor.getCustomPermissions("");
+        original.add(new CustomPermission("java.io.FilePermission", "*", "write"));
+        final Collection<CustomPermission> second = editor.getCustomPermissions("");
+        assertTrue("There should not be any custom permissions", second.isEmpty());
+    }
+
+    @Test
     public void testDefaultPermissionsAllFalse() throws Exception {
         final Map<PolicyEditorPermissions, Boolean> defaultMap = editor.getPermissions("");
         editor.addNewCodebase("http://redhat.com");
@@ -152,6 +160,19 @@
     }
 
     @Test
+    public void testCodebaseTrailingSlashesDoNotMatch() throws Exception {
+        final Set<String> toAdd = new HashSet<String>();
+        toAdd.add("http://redhat.com");
+        toAdd.add("http://redhat.com/");
+        editor.addNewCodebases(toAdd);
+        final Collection<String> codebases = editor.getCodebases();
+        assertTrue("Editor should have default codebase", codebases.contains(""));
+        for (final String codebase : toAdd) {
+            assertTrue("Editor should have " + codebase, codebases.contains(codebase));
+        }
+    }
+
+    @Test
     public void testArgsToMap() throws Exception {
         final String[] args = new String[] {
                 "-codebase", "http://redhat.com http://icedtea.classpath.org",