You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jetspeed-dev@portals.apache.org by rw...@apache.org on 2006/01/07 07:12:49 UTC

svn commit: r366687 - in /portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components: page-manager/src/java/org/apache/jetspeed/om/page/ page-manager/src/java/org/apache/jetspeed/om/page/impl/ page-manager/src/java/org/apache/jetspeed/page/document/i...

Author: rwatler
Date: Fri Jan  6 22:12:43 2006
New Revision: 366687

URL: http://svn.apache.org/viewcvs?rev=366687&view=rev
Log:
correct DB PageManager Security Constraints implementation bugs: NPE, JS2-456, and JS2-458

Modified:
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java
    portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java Fri Jan  6 22:12:43 2006
@@ -76,6 +76,7 @@
      */
     public String getUsersAsString()
     {
+        // get from users list if not immediately available
         if ((users == null) && (usersList != null) && !usersList.isEmpty())
         {
             users = formatCSVList(usersList);
@@ -90,6 +91,7 @@
      */
     public void setUsersAsString(String users)
     {
+        // set and propagate to users list setting
         this.users = users;
         usersList = parseCSVList(users);
     }
@@ -101,6 +103,7 @@
      */
     public String getRolesAsString()
     {
+        // get from roles list if not immediately available
         if ((roles == null) && (rolesList != null) && !rolesList.isEmpty())
         {
             roles = formatCSVList(rolesList);
@@ -115,6 +118,7 @@
      */
     public void setRolesAsString(String roles)
     {
+        // set and propagate to roles list setting
         this.roles = roles;
         rolesList = parseCSVList(roles);
     }
@@ -126,6 +130,7 @@
      */
     public String getGroupsAsString()
     {
+        // get from groups list if not immediately available
         if ((groups == null) && (groupsList != null) && !groupsList.isEmpty())
         {
             groups = formatCSVList(groupsList);
@@ -140,6 +145,7 @@
      */
     public void setGroupsAsString(String groups)
     {
+        // set and propagate to groups list setting
         this.groups = groups;
         groupsList = parseCSVList(groups);
     }
@@ -147,10 +153,11 @@
     /**
      * getPermissionsAsString
      *
-     * @return permmissions CSV list
+     * @return permissions CSV list
      */
     public String getPermissionsAsString()
     {
+        // get from permissions list if not immediately available
         if ((permissions == null) && (permissionsList != null) && !permissionsList.isEmpty())
         {
             permissions = formatCSVList(permissionsList);
@@ -161,10 +168,11 @@
     /**
      * setPermissionsAsString
      *
-     * @param permissions permmissions CSV list
+     * @param permissions permissions CSV list
      */
     public void setPermissionsAsString(String permissions)
     {
+        // set and propagate to permissions list setting
         this.permissions = permissions;
         permissionsList = parseCSVList(permissions);
     }
@@ -192,8 +200,9 @@
      */
     public void setUsers(List users)
     {
-        this.users = formatCSVList(users);
+        // set and clear potentially stale string representation
         usersList = users;
+        this.users = null;
     }
 
     /**
@@ -219,8 +228,9 @@
      */
     public void setRoles(List roles)
     {
-        this.roles = formatCSVList(roles);
+        // set and clear potentially stale string representation
         rolesList = roles;
+        this.roles = null;
     }
 
     /**
@@ -246,8 +256,9 @@
      */
     public void setGroups(List groups)
     {
-        this.groups = formatCSVList(groups);
+        // set and clear potentially stale string representation
         groupsList = groups;
+        this.groups = null;
     }
 
     /**
@@ -260,9 +271,6 @@
      */
     public List getPermissions()
     {
-    	if (permissionsList != null && !permissionsList.isEmpty()) {
-    		this.permissions = this.getPermissionsAsString();
-    	}
         return permissionsList;
     }
     
@@ -276,8 +284,9 @@
      */
     public void setPermissions(List permissions)
     {
-        this.permissions = formatCSVList(permissions);
+        // set and clear potentially stale string representation
         permissionsList = permissions;
+        this.permissions = null;
     }
 
     /**
@@ -296,13 +305,15 @@
      */
     public boolean principalsMatch(List userPrincipals, List rolePrincipals, List groupPrincipals, boolean allowDefault)
     {
-    	return ((allowDefault && (getUsersAsString() == null) && (getRolesAsString() == null) && (getGroupsAsString() == null)) ||
-                ((users != null) && (userPrincipals != null) &&
-                 (users.equals(WILD_CHAR) || containsAny(usersList, userPrincipals))) ||
-                ((roles != null) && (rolePrincipals != null) &&
-                 (roles.equals(WILD_CHAR) || containsAny(rolesList, rolePrincipals))) ||
-                ((groups != null) && (groupPrincipals != null) &&
-                 (groups.equals(WILD_CHAR) || containsAny(groupsList, groupPrincipals))));
+        // test match using users, roles, and groups list members
+        // since these are the master representation in this impl
+        return ((allowDefault && (usersList == null) && (rolesList == null) && (groupsList == null)) ||
+                ((usersList != null) && (userPrincipals != null) &&
+                 (containsAny(usersList, userPrincipals) || usersList.contains(WILD_CHAR))) ||
+                ((rolesList != null) && (rolePrincipals != null) &&
+                 (containsAny(rolesList, rolePrincipals) || rolesList.contains(WILD_CHAR))) ||
+                ((groupsList != null) && (groupPrincipals != null) &&
+                 (containsAny(groupsList, groupPrincipals) || groupsList.contains(WILD_CHAR))));
     }
 
     /**
@@ -318,8 +329,10 @@
      */
     public boolean actionMatch(String action)
     {
-        return ((permissions != null) &&
-                (permissions.equals(WILD_CHAR) || permissionsList.contains(action)));
+        // test match using permissions list member since
+        // this is the master representation in this impl
+        return ((permissionsList != null) &&
+                (permissionsList.contains(action) || permissionsList.contains(WILD_CHAR)));
     }
 
     /**
@@ -335,7 +348,7 @@
      */
     public static List parseCSVList(String csv)
     {
-        if ((csv != null) && !csv.equals(WILD_CHAR))
+        if (csv != null)
         {
             List csvList = new ArrayList(4);
             if (csv.indexOf(',') != -1)

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java Fri Jan  6 22:12:43 2006
@@ -375,6 +375,9 @@
      */
     public boolean isEmpty()
     {
-    	return ((owner == null) && getSecurityConstraints().isEmpty() && getSecurityConstraintsRefs().isEmpty());
+        // test only persistent members for any specified constraints
+        return ((owner == null) &&
+                ((constraints == null) || constraints.isEmpty()) &&
+                ((constraintsRefs == null) || constraintsRefs.isEmpty()));
     }
 }

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java Fri Jan  6 22:12:43 2006
@@ -130,11 +130,11 @@
      */
     public Iterator iterator()
     {
-        if (nodes != null)
+        if (nodes == null)
         {
-            return nodes.values().iterator();
+            nodes = new TreeMap(comparator);
         }
-        return null;
+        return nodes.values().iterator();
     }
     
     /* (non-Javadoc)
@@ -142,17 +142,13 @@
      */
     public NodeSet subset(String type)
     {
-        NodeSetImpl subset = null;
-        Iterator nodeItr = nodes.values().iterator();
+        NodeSetImpl subset = new NodeSetImpl(comparator);
+        Iterator nodeItr = iterator();
         while (nodeItr.hasNext())
         {
             Node node = (Node) nodeItr.next();
             if (node.getType().equals(type))
             {
-                if (subset == null)
-                {
-                    subset = new NodeSetImpl(comparator);
-                }
                 subset.add(node);
             }
         }
@@ -165,17 +161,13 @@
     public NodeSet inclusiveSubset(String regex)
     {
         Pattern pattern = getCachedPattern(regex);
-        NodeSetImpl subset = null;
-        Iterator nodeItr = nodes.values().iterator();
+        NodeSetImpl subset = new NodeSetImpl(comparator);
+        Iterator nodeItr = iterator();
         while (nodeItr.hasNext())
         {
             Node node = (Node) nodeItr.next();
             if (pattern.matcher(node.getName()).matches())
             {
-                if (subset == null)
-                {
-                    subset = new NodeSetImpl(comparator);
-                }
                 subset.add(node);
             }
         }
@@ -188,17 +180,13 @@
     public NodeSet exclusiveSubset(String regex)
     {
         Pattern pattern = getCachedPattern(regex);
-        NodeSetImpl subset = null;
-        Iterator nodeItr = nodes.values().iterator();
+        NodeSetImpl subset = new NodeSetImpl(comparator);
+        Iterator nodeItr = iterator();
         while (nodeItr.hasNext())
         {
             Node node = (Node) nodeItr.next();
             if (!pattern.matcher(node.getName()).matches())
             {
-                if (subset == null)
-                {
-                    subset = new NodeSetImpl(comparator);
-                }
                 subset.add(node);
             }
         }

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java Fri Jan  6 22:12:43 2006
@@ -132,7 +132,7 @@
          */
         static List makeListFromCSV(String csv)
         {
-            if (!csv.equals("*"))
+            if (csv != null)
             {
                 List csvList = new ArrayList();
                 if (csv.indexOf(',') != -1)

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java Fri Jan  6 22:12:43 2006
@@ -917,7 +917,9 @@
         assertTrue(constraint.getUsers() != null);
         assertTrue(constraint.getUsers().size() == 2);
         assertTrue(Shared.makeCSVFromList(constraint.getUsers()).equals("user10,user11"));
-        assertTrue(constraint.getRoles() == null);
+        assertTrue(constraint.getRoles() != null);
+        assertTrue(constraint.getRoles().size() == 1);
+        assertTrue(Shared.makeCSVFromList(constraint.getRoles()).equals("*"));
         assertTrue(constraint.getPermissions() != null);
         assertTrue(constraint.getPermissions().size() == 2);
         assertTrue(Shared.makeCSVFromList(constraint.getPermissions()).equals("edit,view"));

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java Fri Jan  6 22:12:43 2006
@@ -496,7 +496,8 @@
         assertEquals(3, folder.getAll().subset(Page.DOCUMENT_TYPE).size());
         assertNotNull(folder.getAll().inclusiveSubset(".*other.*"));
         assertEquals(2, folder.getAll().inclusiveSubset(".*other.*").size());
-        assertNull(folder.getAll().inclusiveSubset("nomatch"));
+        assertNotNull(folder.getAll().inclusiveSubset("nomatch"));
+        assertEquals(0, folder.getAll().inclusiveSubset("nomatch").size());
         assertNotNull(folder.getAll().exclusiveSubset(".*-page.psml"));
         assertEquals(3, folder.getAll().exclusiveSubset(".*-page.psml").size());
     }
@@ -522,6 +523,7 @@
             assertEquals("public-view", ((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getName());
             assertNotNull(((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints());
             assertEquals(1, ((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints().size());
+            assertEquals("*", Shared.makeCSVFromList(((SecurityConstraint)((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints().get(0)).getUsers()));
             assertEquals("view", Shared.makeCSVFromList(((SecurityConstraint)((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints().get(0)).getPermissions()));
             assertNotNull(check.getGlobalSecurityConstraintsRefs());
             assertEquals(2, check.getGlobalSecurityConstraintsRefs().size());
@@ -682,7 +684,7 @@
             assertEquals(2, check.getSecurityConstraints().getSecurityConstraints().size());
             assertEquals("user,admin", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getUsers()));
             assertEquals("manager", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getRoles()));
-            assertNull(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getGroups());
+            assertEquals("*", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getGroups()));
             assertEquals("edit", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(1)).getPermissions()));
             assertNotNull(check.getDocumentOrder());
             assertEquals(2, check.getDocumentOrder().size());

Modified: portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java?rev=366687&r1=366686&r2=366687&view=diff
==============================================================================
--- portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java (original)
+++ portals/jetspeed-2/branches/JETSPEED-BRANCH-2.0.1/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java Fri Jan  6 22:12:43 2006
@@ -749,8 +749,6 @@
                 // create and save proxies for concrete children
                 NodeSet children = folder.getAll();
                 Iterator childrenIter = children.iterator();
-                if (childrenIter == null)
-                    continue;
                 while (childrenIter.hasNext())
                 {
                     Node child = (Node)childrenIter.next();



---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@portals.apache.org
For additional commands, e-mail: jetspeed-dev-help@portals.apache.org