db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r1609039 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/util/ engine/org/apache/derby/security/ testing/org/apache/derbyTesting/unitTests/junit/
Date Wed, 09 Jul 2014 07:36:39 GMT
Author: kahatlen
Date: Wed Jul  9 07:36:39 2014
New Revision: 1609039

URL: http://svn.apache.org/r1609039
Log:
DERBY-3476: Improve serialization of DatabasePermission

Store the URL only once (remove the url field, as the URL is already
stored in Permission.name).

Make the pathType field transient, since it is recalculated on
deserialization anyway.

Store the actions portion of the permission.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/StringUtil.java
    db/derby/code/trunk/java/engine/org/apache/derby/security/DatabasePermission.java
    db/derby/code/trunk/java/engine/org/apache/derby/security/SystemPermission.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/SystemPrivilegesPermissionTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/StringUtil.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/StringUtil.java?rev=1609039&r1=1609038&r2=1609039&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/StringUtil.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/StringUtil.java Wed Jul  9
07:36:39 2014
@@ -22,46 +22,12 @@
 package org.apache.derby.iapi.util;
 
 import java.util.Locale;
-import java.util.StringTokenizer;
 
 /**
 	A set of public static methods for dealing with Strings
 */
 public class StringUtil 
 {
-    /**
-     * Splits a string around matches of the given delimiter character.
-     *
-     * Where applicable, this method can be used as a substitute for
-     * <code>String.split(String regex)</code>, which is not available
-     * on a JSR169/Java ME platform.
-     *
-     * @param str the string to be split
-     * @param delim the delimiter
-     * @throws NullPointerException if str is null
-     */
-    static public String[] split(String str, char delim)
-    {
-        if (str == null) {
-            throw new NullPointerException("str can't be null");
-        }
-
-        // Note the javadoc on StringTokenizer:
-        //     StringTokenizer is a legacy class that is retained for
-        //     compatibility reasons although its use is discouraged in
-        //     new code.
-        // In other words, if StringTokenizer is ever removed from the JDK,
-        // we need to have a look at String.split() (or java.util.regex)
-        // if it is supported on a JSR169/Java ME platform by then.
-        StringTokenizer st = new StringTokenizer(str, String.valueOf(delim));
-        int n = st.countTokens();
-        String[] s = new String[n];
-        for (int i = 0; i < n; i++) {
-            s[i] = st.nextToken();
-        }
-        return s;
-    }
-
 	/**
 	 * Used to print out a string for error messages, 
 	 * chops is off at 60 chars for historical reasons.

Modified: db/derby/code/trunk/java/engine/org/apache/derby/security/DatabasePermission.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/security/DatabasePermission.java?rev=1609039&r1=1609038&r2=1609039&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/security/DatabasePermission.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/security/DatabasePermission.java Wed
Jul  9 07:36:39 2014
@@ -26,11 +26,10 @@ import java.security.PrivilegedAction;
 import java.security.PrivilegedExceptionAction;
 import java.security.PrivilegedActionException;
 import java.security.AccessController;
-import org.apache.derby.iapi.util.StringUtil;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Set;
-import java.util.HashSet;
-import java.util.Locale;
 
 import java.io.File;
 import java.io.IOException;
@@ -119,16 +118,16 @@ final public class DatabasePermission ex
     /**
      * The legal database permission action names.
      */
-    static protected final Set<String> LEGAL_ACTIONS = new HashSet<String>();
+    static protected final List<String> LEGAL_ACTIONS = new ArrayList<String>();
     static {
         // when adding new actions, check: implies(Permission), getActions()
         LEGAL_ACTIONS.add(CREATE);
     };
 
     /**
-     * The original location URL passed to constructor.
+     * The actions of this permission, as returned by {@link #getActions()}.
      */
-    private final String url;
+    private String actions;
 
     /**
      * This permission's canonical directory path.
@@ -166,8 +165,10 @@ final public class DatabasePermission ex
      * this field's value is URL_PATH_INCLUSIVE_CHAR, URL_PATH_RECURSIVE_CHAR,
      * or URL_PATH_WILDCARD_CHAR, respectively; otherwise, it's
      * URL_PATH_SEPARATOR_CHAR denoting a single location.
+     *
+     * This field gets recomputed upon deserialization.
      */
-    private char pathType;
+    private transient char pathType;
 
     /**
      * Creates a new DatabasePermission with the specified URL and actions.
@@ -211,9 +212,6 @@ final public class DatabasePermission ex
         super(url);
         initActions(actions);
         initLocation(url);
-
-        // store original URL for reconstructing path at deserialization
-        this.url = url;
     }
 
     /**
@@ -233,17 +231,24 @@ final public class DatabasePermission ex
             throw new IllegalArgumentException("actions can't be empty");
         }
 
+        // Get all the actions specified in the actions string
+        Set<String> actionSet = SystemPermission.parseActions(actions);
+
         // check for any illegal actions
-        actions = actions.toLowerCase(Locale.ENGLISH);
-        final String[] s = StringUtil.split(actions, ',');
-        for (int i = 0; i < s.length; i++) {
-            final String action = s[i].trim();
+        for (String action : actionSet) {
             if (!LEGAL_ACTIONS.contains(action)) {
                 // report illegal action
                 final String msg = "Illegal action '" + action + "'";
                 throw new IllegalArgumentException(msg);
             }
         }
+
+        // Get all the legal actions that are in actionSet, in the order
+        // of LEGAL_ACTIONS.
+        List<String> legalActions = new ArrayList<String>(LEGAL_ACTIONS);
+        legalActions.retainAll(actionSet);
+
+        this.actions = SystemPermission.buildActionsString(legalActions);
     }
 
     /**
@@ -466,8 +471,7 @@ final public class DatabasePermission ex
      * @see Permission#getActions()
      */
     public String getActions() {
-        // currently, the only supported action
-        return CREATE;
+        return actions;
     }
 
 
@@ -490,7 +494,12 @@ final public class DatabasePermission ex
     {
         // read the non-static and non-transient fields from the stream
         s.defaultReadObject();
+
+        // Validate the URL read from the object stream, and
         // restore the platform-dependent path from the original URL
-        initLocation(url);
+        initLocation(getName());
+
+        // Validate and normalize the actions read from the stream.
+        initActions(getActions());
     }
 }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/security/SystemPermission.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/security/SystemPermission.java?rev=1609039&r1=1609038&r2=1609039&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/security/SystemPermission.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/security/SystemPermission.java Wed Jul
 9 07:36:39 2014
@@ -160,30 +160,51 @@ final public class SystemPermission exte
     /**
      * Return a canonical form of the passed in actions.
      * Actions are lower-cased, in the order of LEGAL_ACTIONS
-     * and on;ly appear once.
+     * and only appear once.
      */
     private static String getCanonicalForm(String actions) {
-        actions = actions.trim().toLowerCase(Locale.ENGLISH);
-        
-        boolean[] seenAction = new boolean[LEGAL_ACTIONS.size()];
-        StringTokenizer st = new StringTokenizer(actions, ",");
-        while (st.hasMoreTokens()) {
-            String action = st.nextToken().trim().toLowerCase(Locale.ENGLISH);
-            int validAction = LEGAL_ACTIONS.indexOf(action);
-            if (validAction != -1)
-                seenAction[validAction] = true;
+        Set<String> actionSet = parseActions(actions);
+
+        // Get all the legal actions that are in actionSet, in the order
+        // of LEGAL_ACTIONS.
+        List<String> legalActions = new ArrayList<String>(LEGAL_ACTIONS);
+        legalActions.retainAll(actionSet);
+
+        return buildActionsString(legalActions);
+    }
+
+    /**
+     * Get a set of all actions specified in a string. Actions are transformed
+     * to lower-case, and leading and trailing blanks are stripped off.
+     *
+     * @param actions the specified actions string
+     * @return a set of all the specified actions
+     */
+    static Set<String> parseActions(String actions) {
+        HashSet<String> actionSet = new HashSet<String>();
+        for (String s : actions.split(",", -1)) {
+            actionSet.add(s.trim().toLowerCase(Locale.ENGLISH));
         }
-        
-        StringBuffer sb = new StringBuffer();
-        for (int sa = 0; sa < seenAction.length; sa++)
-        {
-            if (seenAction[sa]) {
-                if (sb.length() != 0)
-                    sb.append(",");
-                sb.append(LEGAL_ACTIONS.get(sa));
+        return actionSet;
+    }
+
+    /**
+     * Build a comma-separated actions string suitable for returning from
+     * {@code getActions()}.
+     *
+     * @param actions the list of actions
+     * @return comma-separated string with the actions
+     */
+    static String buildActionsString(Iterable<String> actions) {
+        StringBuilder sb = new StringBuilder();
+
+        for (String action : actions) {
+            if (sb.length() > 0) {
+                sb.append(',');
             }
+            sb.append(action);
         }
-        
+
         return sb.toString();
     }
 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/SystemPrivilegesPermissionTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/SystemPrivilegesPermissionTest.java?rev=1609039&r1=1609038&r2=1609039&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/SystemPrivilegesPermissionTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/unitTests/junit/SystemPrivilegesPermissionTest.java
Wed Jul  9 07:36:39 2014
@@ -33,6 +33,7 @@ import java.security.AccessController;
 import java.security.Permission;
 import java.security.PrivilegedAction;
 import java.util.HashSet;
+import java.util.Arrays;
 import java.util.Locale;
 import java.util.Set;
 import javax.security.auth.Subject;
@@ -282,6 +283,9 @@ public class SystemPrivilegesPermissionT
         assertEquals("control,monitor",
                      new SystemPermission("server", "control, monitor, control")
                              .getActions());
+        assertEquals("control,monitor",
+                     new SystemPermission("server", "monitor, control, monitor")
+                             .getActions());
         assertEquals("control",
                      new SystemPermission("server", "CoNtRoL")
                              .getActions());
@@ -430,7 +434,15 @@ public class SystemPrivilegesPermissionT
         } catch (IllegalArgumentException ex) {
             // expected exception
         }
-        
+
+        // test DatabasePermission with unsupported protocol
+        try {
+            new DatabasePermission("unknown:test", DatabasePermission.CREATE);
+            fail("expected IllegalArgumentException");
+        } catch (IllegalArgumentException ex) {
+            // expected exception
+        }
+
         // this test's commented out because it's platform-dependent
         // (no reliable way to make it pass on Unix)
         // test DatabasePermission with non-canonicalizable URL
@@ -476,6 +488,14 @@ public class SystemPrivilegesPermissionT
             // expected exception
         }
     
+        // test DatabasePermission with illegal action list
+        try {
+            new DatabasePermission("directory:dir", "illegal,create,action");
+            fail("expected IllegalArgumentException");
+        } catch (IllegalArgumentException ex) {
+            // expected exception
+        }
+
         // test DatabasePermission on illegal action list
         try {
             new DatabasePermission("directory:dir", "illegal;action");
@@ -484,6 +504,30 @@ public class SystemPrivilegesPermissionT
             // expected exception
         }
 
+        // test DatabasePermission with illegal action list
+        try {
+            new DatabasePermission("directory:dir", ",");
+            fail("expected IllegalArgumentException");
+        } catch (IllegalArgumentException ex) {
+            // expected exception
+        }
+
+        // test DatabasePermission with illegal action list
+        try {
+            new DatabasePermission("directory:dir", " ");
+            fail("expected IllegalArgumentException");
+        } catch (IllegalArgumentException ex) {
+            // expected exception
+        }
+
+        // test DatabasePermission with illegal action list
+        try {
+            new DatabasePermission("directory:dir", "create,");
+            fail("expected IllegalArgumentException");
+        } catch (IllegalArgumentException ex) {
+            // expected exception
+        }
+
         // test DatabasePermission on relative directory paths
         final DatabasePermission[] relDirPathPerms
             = new DatabasePermission[relDirPaths.length];
@@ -563,6 +607,14 @@ public class SystemPrivilegesPermissionT
         checkImplies(inclPerms, absDirPathAliasPerms, allTrue);
         checkImplies(absDirPathAliasPerms, inclPerms, allFalse);
 
+        // Actions string is washed (lower-cased, trimmed) and duplicates
+        // are removed.
+        DatabasePermission perm =
+                new DatabasePermission("directory:dir", "create, create");
+        assertEquals("create", perm.getActions());
+        perm = new DatabasePermission("directory:dir", "  CrEaTe  ");
+        assertEquals("create", perm.getActions());
+
         // DERBY-3476: The DatabasePermission class should be final.
         assertTrue(Modifier.isFinal(DatabasePermission.class.getModifiers()));
     }
@@ -613,7 +665,89 @@ public class SystemPrivilegesPermissionT
      * deserialization of invalid objects fails.
      */
     public void testSerialization() throws IOException {
+        testDatabasePermissionSerialization();
+        testSystemPermissionSerialization();
+    }
+
+    /**
+     * Test serialization and deserialization of DatabasePermission objects.
+     */
+    private void testDatabasePermissionSerialization() throws IOException {
+        // Simple test of serialization/deserialization of a valid object
+        DatabasePermission perm =
+                new DatabasePermission("directory:dir", "create");
+        assertEquals(perm, serializeDeserialize(perm, null));
+
+        // Test of relative paths
+        for (String url : relDirPaths) {
+            perm = new DatabasePermission(url, "create");
+            assertEquals(perm, serializeDeserialize(perm, null));
+        }
+
+        // Test of relative path aliases
+        for (String url : relDirPathAliases) {
+            perm = new DatabasePermission(url, "create");
+            assertEquals(perm, serializeDeserialize(perm, null));
+        }
+
+        // Test of absolute paths
+        for (String url : absDirPaths) {
+            perm = new DatabasePermission(url, "create");
+            assertEquals(perm, serializeDeserialize(perm, null));
+        }
+
+        // Test of absolute path aliases
+        for (String url : absDirPathAliases) {
+            perm = new DatabasePermission(url, "create");
+            assertEquals(perm, serializeDeserialize(perm, null));
+        }
+
+        // Actions should be normalized when read from the stream.
+        for (String actions :
+                Arrays.asList("create", "CrEaTe", " create ,  create")) {
+            perm = serializeDeserialize(
+                    createDBPermNoCheck("directory:dir", actions),
+                    null);
+            assertEquals("create", perm.getActions());
+        }
+
+        // Null URL should fail on deserialization (didn't before DERBY-3476)
+        perm = createDBPermNoCheck(null, "create");
+        serializeDeserialize(perm, NullPointerException.class);
+
+        // Empty URL should fail on deserialization (didn't before DERBY-3476)
+        perm = createDBPermNoCheck("", "create");
+        serializeDeserialize(perm, IllegalArgumentException.class);
 
+        // Unsupported protocol should fail on deserialization (didn't before
+        // DERBY-3476)
+        perm = createDBPermNoCheck("unknown:test", "create");
+        serializeDeserialize(perm, IllegalArgumentException.class);
+
+        // Null actions should fail on deserialization
+        serializeDeserialize(createDBPermNoCheck("directory:dir", null),
+                             NullPointerException.class);
+
+        // Empty and invalid actions should fail on deserialization
+        serializeDeserialize(createDBPermNoCheck("directory:dir", ""),
+                             IllegalArgumentException.class);
+        serializeDeserialize(createDBPermNoCheck("directory:dir", " "),
+                             IllegalArgumentException.class);
+        serializeDeserialize(createDBPermNoCheck("directory:dir", ","),
+                             IllegalArgumentException.class);
+        serializeDeserialize(createDBPermNoCheck("directory:dir", "create,"),
+                             IllegalArgumentException.class);
+        serializeDeserialize(createDBPermNoCheck("directory:dir", "invalid"),
+                             IllegalArgumentException.class);
+        serializeDeserialize(createDBPermNoCheck("directory:dir",
+                                                 "create,invalid"),
+                             IllegalArgumentException.class);
+    }
+
+    /**
+     * Test serialization and deserialization of SystemPermission objects.
+     */
+    private void testSystemPermissionSerialization() throws IOException {
         // Test all valid name/action combinations. All should succeed to
         // serialize and deserialize.
         for (String name : VALID_SYSPERM_NAMES) {
@@ -680,6 +814,29 @@ public class SystemPrivilegesPermissionT
     }
 
     /**
+     * Create a DatabasePermission object without checking that the URL
+     * and the actions are valid.
+     *
+     * @param url the URL of the permission
+     * @param actions the actions of the permission
+     * @return a DatabasePermission instance
+     */
+    private static DatabasePermission createDBPermNoCheck(
+            String url, String actions) throws IOException {
+        // First create a valid permission object, so that the checks in
+        // the constructor are happy.
+        DatabasePermission perm =
+                new DatabasePermission("directory:dir", "create");
+
+        // Then use reflection to override the values of the fields with
+        // potentially invalid values.
+        setField(Permission.class, "name", perm, url);
+        setField(DatabasePermission.class, "actions", perm, actions);
+
+        return perm;
+    }
+
+    /**
      * Create a new SystemPermission object without checking that the name
      * and actions are valid.
      *



Mime
View raw message