changeset 927:932c096d423c

PolicyEditor external file change checking * netx/net/sourceforge/jnlp/resources/Messages.properties: (PEFileModified, PEFileModifiedDetail) new messages * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java: (fileWatcher, openAndParsePolicyFile, savePolicyFile) update to use MD5SumWatcher to check if the file has changed externally since being opened * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java: URLs changed to example.com
author Andrew Azores <aazores@redhat.com>
date Wed, 12 Mar 2014 09:43:36 -0400
parents 78f6e65ed7b3
children 9417633d1f86
files ChangeLog netx/net/sourceforge/jnlp/resources/Messages.properties netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
diffstat 4 files changed, 89 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Wed Mar 12 09:35:48 2014 -0400
+++ b/ChangeLog	Wed Mar 12 09:43:36 2014 -0400
@@ -1,3 +1,14 @@
+2014-03-12  Andrew Azores  <aazores@redhat.com>
+
+	* netx/net/sourceforge/jnlp/resources/Messages.properties:
+	(PEFileModified, PEFileModifiedDetail) new messages
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java:
+	(fileWatcher, openAndParsePolicyFile, savePolicyFile) update to use
+	MD5SumWatcher to check if the file has changed externally since being
+	opened
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java:
+	URLs changed to example.com
+
 2014-03-12  Andrew Azores  <aazores@redhat.com>
 
 	* netx/net/sourceforge/jnlp/resources/Messages.properties:
--- a/netx/net/sourceforge/jnlp/resources/Messages.properties	Wed Mar 12 09:35:48 2014 -0400
+++ b/netx/net/sourceforge/jnlp/resources/Messages.properties	Wed Mar 12 09:43:36 2014 -0400
@@ -503,6 +503,8 @@
 PEExitMenuItem=Exit
 PEViewMenu=View
 PECustomPermissionsItem=Custom Permissions...
+PEFileModified=File Modification Warning
+PEFileModifiedDetail=The policy file at {0} has been modified since it was opened. Reload and re-edit before saving?
 
 # Policy Editor CustomPolicyViewer
 PECPTitle=Custom Policy Viewer
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Wed Mar 12 09:35:48 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Wed Mar 12 09:43:36 2014 -0400
@@ -94,6 +94,7 @@
 
 import net.sourceforge.jnlp.util.FileUtils;
 import net.sourceforge.jnlp.util.FileUtils.OpenFileResult;
+import net.sourceforge.jnlp.util.MD5SumWatcher;
 import net.sourceforge.jnlp.util.logging.OutputController;
 
 /**
@@ -164,6 +165,7 @@
     private final JFileChooser fileChooser;
     private CustomPolicyViewer cpViewer = null;
     private final WeakReference<PolicyEditor> weakThis = new WeakReference<PolicyEditor>(this);
+    private MD5SumWatcher fileWatcher;
 
     private final ActionListener okButtonAction, closeButtonAction, addCodebaseButtonAction,
             removeCodebaseButtonAction, openButtonAction, saveAsButtonAction, viewCustomButtonAction;
@@ -593,12 +595,25 @@
             for (final ActionListener l : box.getActionListeners()) {
                 box.removeActionListener(l);
             }
-            box.setSelected(codebasePermissionsMap.get(codebase).get(perm));
+            initializeMapForCodebase(codebase);
+            final Map<PolicyEditorPermissions, Boolean> map = codebasePermissionsMap.get(codebase);
+            final boolean state;
+            if (map != null) {
+                final Boolean s = map.get(perm);
+                if (s != null) {
+                    state = s;
+                } else {
+                    state = false;
+                }
+            } else {
+                state = false;
+            }
+            box.setSelected(state);
             box.addActionListener(new ActionListener() {
                 @Override
                 public void actionPerformed(final ActionEvent e) {
                     changesMade = true;
-                    codebasePermissionsMap.get(codebase).put(perm, box.isSelected());
+                    map.put(perm, box.isSelected());
                 }
             });
         }
@@ -797,6 +812,10 @@
                 }
                 final String contents;
                 try {
+                    fileWatcher = new MD5SumWatcher(file);
+                    fileWatcher.update();
+                    // User-level policy files are expected to be short enough that loading them in as a String
+                    // should not actually be *too* bad, and it's easy to work with.
                     contents = FileUtils.loadFileAsString(file);
                 } catch (final IOException e) {
                     OutputController.getLogger().log(e);
@@ -804,10 +823,12 @@
                     FileUtils.showCouldNotOpenDialog(weakThis.get(), R("PECouldNotOpen"));
                     return;
                 }
+                codebasePermissionsMap.clear();
+                customPermissionsMap.clear();
                 // Split on newlines, both \r\n and \n style, for platform-independence
                 final String[] lines = contents.split("\\r?\\n+");
                 String codebase = "";
-                FileLock fileLock = null;
+                final FileLock fileLock;
                 try {
                     fileLock = FileUtils.getFileLock(file.getAbsolutePath(), false, true);
                 } catch (final FileNotFoundException e) {
@@ -828,13 +849,16 @@
                         openBlock = true;
                     }
 
+                    // Matches '};', the closing block delimiter, with any amount of whitespace on either side
                     boolean commentLine = false;
                     if (line.matches("\\s*\\};\\s*")) {
                         openBlock = false;
                     }
+                    // Matches '/*', the start of a block comment
                     if (line.matches(".*/\\*.*")) {
                         commentBlock = true;
                     }
+                    // Matches '*/', the end of a block comment, and '//', a single-line comment
                     if (line.matches(".*\\*/.*")) {
                         commentBlock = false;
                     }
@@ -899,6 +923,28 @@
         new Thread() {
             @Override
             public void run() {
+                try {
+                    final int response = updateMd5WithDialog();
+                    switch (response) {
+                        case JOptionPane.YES_OPTION:
+                            openAndParsePolicyFile();
+                            return;
+                        case JOptionPane.NO_OPTION:
+                            break;
+                        case JOptionPane.CANCEL_OPTION:
+                            return;
+                        default:
+                            break;
+                    }
+                } catch (final FileNotFoundException e) {
+                    // File on disk has been somehow removed since we first checked. Attempt to save to it
+                    // anyway then. If we can't, then the failure simply occurs later in this method
+                    OutputController.getLogger().log(e);
+                } catch (final IOException e) {
+                    OutputController.getLogger().log(e);
+                    showCouldNotSaveDialog();
+                    return;
+                }
                 final StringBuilder sb = new StringBuilder();
                 sb.append(AUTOGENERATED_NOTICE);
                 sb.append("\n/* Generated by PolicyEditor at " + new SimpleDateFormat("yyyy-MM-dd HH:mm:ss")
@@ -929,6 +975,7 @@
 
                 try {
                     FileUtils.saveFile(sb.toString(), file);
+                    fileWatcher.update();
                     changesMade = false;
                     showChangesSavedDialog();
                 } catch (final IOException e) {
@@ -964,6 +1011,21 @@
     }
 
     /**
+     * Detect if the file's MD5 has changed. If so, track its new sum, and prompt the user on how to proceed
+     * @return the user's choice (Yes/No/Cancel - see JOptionPane constants). "No" if the file hasn't changed.
+     * @throws FileNotFoundException if the watched file does not exist
+     * @throws IOException if the file cannot be read
+     */
+    public int updateMd5WithDialog() throws FileNotFoundException, IOException {
+        final boolean changed = fileWatcher.update();
+        if (changed) {
+            return JOptionPane.showConfirmDialog(weakThis.get(), R("PEFileModifiedDetail", file.getCanonicalPath()),
+                    R("PEFileModified"), JOptionPane.YES_NO_CANCEL_OPTION);
+        }
+        return JOptionPane.NO_OPTION;
+    }
+
+    /**
      * Start a Policy Editor instance.
      * @param args "-file $FILENAME" and/or "-codebase $CODEBASE" are accepted flag/value pairs.
      * -file specifies a file path to be opened by the editor. If none is provided, the default
--- a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java	Wed Mar 12 09:35:48 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java	Wed Mar 12 09:43:36 2014 -0400
@@ -69,18 +69,18 @@
 
     @Test
     public void testAddCodebase() throws Exception {
-        final String redHatUrlString = "http://redhat.com";
-        editor.addNewCodebase(redHatUrlString);
+        final String urlString = "http://example.com";
+        editor.addNewCodebase(urlString);
         final Collection<String> codebases = editor.getCodebases();
         assertTrue("Editor should have default codebase", codebases.contains(""));
-        assertTrue("Editor should have http://redhat.com", codebases.contains(redHatUrlString));
+        assertTrue("Editor should have http://example.com", codebases.contains(urlString));
         assertTrue("Editor should only have two codebases", codebases.size() == 2);
     }
 
     @Test
     public void addMultipleCodebases() throws Exception {
         final Set<String> toAdd = new HashSet<String>();
-        toAdd.add("http://redhat.com");
+        toAdd.add("http://example.com");
         toAdd.add("http://icedtea.classpath.org");
         editor.addNewCodebases(toAdd);
         final Collection<String> codebases = editor.getCodebases();
@@ -132,8 +132,8 @@
     @Test
     public void testDefaultPermissionsAllFalse() throws Exception {
         final Map<PolicyEditorPermissions, Boolean> defaultMap = editor.getPermissions("");
-        editor.addNewCodebase("http://redhat.com");
-        final Map<PolicyEditorPermissions, Boolean> addedMap = editor.getPermissions("http://redhat.com");
+        editor.addNewCodebase("http://example.com");
+        final Map<PolicyEditorPermissions, Boolean> addedMap = editor.getPermissions("http://example.com");
         for (final Map.Entry<PolicyEditorPermissions, Boolean> entry : defaultMap.entrySet()) {
             assertFalse("Permission " + entry.getKey() + " should be false", entry.getValue());
         }
@@ -145,8 +145,8 @@
     @Test
     public void testAllPermissionsRepresented() throws Exception {
         final Map<PolicyEditorPermissions, Boolean> defaultMap = editor.getPermissions("");
-        editor.addNewCodebase("http://redhat.com");
-        final Map<PolicyEditorPermissions, Boolean> addedMap = editor.getPermissions("http://redhat.com");
+        editor.addNewCodebase("http://example.com");
+        final Map<PolicyEditorPermissions, Boolean> addedMap = editor.getPermissions("http://example.com");
         assertTrue("Default codebase permissions keyset should be the same size as enum values set",
                 defaultMap.keySet().size() == PolicyEditorPermissions.values().length);
         assertTrue("Added codebase permissions keyset should be the same size as enum values set",
@@ -175,7 +175,7 @@
     @Test
     public void testArgsToMap() throws Exception {
         final String[] args = new String[] {
-                "-codebase", "http://redhat.com http://icedtea.classpath.org",
+                "-codebase", "http://example.com http://icedtea.classpath.org",
                 "-file", "/tmp/some-policy-file.tmp",
                 "-help"
         };
@@ -187,8 +187,8 @@
         assertTrue("Value for -file should be /tmp/some-policy-file.tmp but was " + map.get("-file"),
                 map.get("-file").equals("/tmp/some-policy-file.tmp"));
         assertTrue("Args map should contain codebase flag", map.containsKey("-codebase"));
-        assertTrue("Value for codebase flag should be \"http://redhat.com http://icedtea.classpath.org\" but was " + map.get("-codebase"),
-                map.get("-codebase").equals("http://redhat.com http://icedtea.classpath.org"));
+        assertTrue("Value for codebase flag should be \"http://example.com http://icedtea.classpath.org\" but was " + map.get("-codebase"),
+                map.get("-codebase").equals("http://example.com http://icedtea.classpath.org"));
     }
 
 }
\ No newline at end of file