You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by mi...@apache.org on 2006/10/07 17:33:25 UTC

svn commit: r453935 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/conn/ testing/org/apache/derbyTesting/functionTests/master/ testing/org/apache/derbyTesting/functionTests/tests/lang/

Author: mikem
Date: Sat Oct  7 08:33:24 2006
New Revision: 453935

URL: http://svn.apache.org/viewvc?view=rev&rev=453935
Log:
DERBY-1716
contributed by Yip Ng
patch: derby1716-trunk-diff03.txt

Unlike other descriptors, when privilege(s) get revoked from user,
the statement is not subject to recompilation, so then we are back to square one
since the previous patch attempts to bring in the permission descriptor(s) into
the permission cache at compilation time to avoid reading from system tables at
execution time.

I believe the proper proposal fix is to use internal nested read-only transaction
when the system is reading permission descriptors from the system tables. At a
high level, a statement undergoes the following typical steps for it to get executed
by the system:

1. Statement Compilation Phase
a) Parse the statement
b) Bind the statement and collects required permissions for it to be executed.
c) Optimize the statement
d) Generate the activation for the statement


2. Statement Execution Phase
a) Check if the authoration id has the required privileges to execute the statement.
b) Execute the statement


The problem lies in permissions checking step at statement execution phase. Before a statement can be executed in SQL authorization mode, the authorization id's privileges needs to be check against the permission cache or if the privileges are not available in the cache, the system needs to read this metadata information from the system tables. But the system is using *user transaction* to do this, so the shared locks that got acquired by the user transaction may not get released immediately; therefore, leading to lock timeout when the grantor attempts to revoke the user's privilege. To resolve this issue, the system now will start an internal read-only nested transaction(same lock space as the parent transaction) to read permission related info from the system tables and release the shared locks
as soon as the permissions check is completed before statement execution. This tackles the root of the stated problem. 



Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/conn/GenericAuthorizer.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/conn/GenericAuthorizer.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/conn/GenericAuthorizer.java?view=diff&rev=453935&r1=453934&r2=453935
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/conn/GenericAuthorizer.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/conn/GenericAuthorizer.java Sat Oct  7 08:33:24 2006
@@ -150,17 +150,61 @@
 
             // Database Owner can access any object. Ignore 
             // requiredPermissionsList for Database Owner
-            if( requiredPermissionsList != null && ! requiredPermissionsList.isEmpty() && 
+            if( requiredPermissionsList != null    && 
+                !requiredPermissionsList.isEmpty() && 
 				!authorizationId.equals(dd.getAuthorizationDatabaseOwner()))
             {
-                for( Iterator iter = requiredPermissionsList.iterator();
-                     iter.hasNext();)
+                int ddMode = dd.startReading(lcc);
+                
+                 /*
+                  * The system may need to read the permission descriptor(s) 
+                  * from the system table(s) if they are not available in the 
+                  * permission cache.  So start an internal read-only nested 
+                  * transaction for this.
+                  * 
+                  * The reason to use a nested transaction here is to not hold
+                  * locks on system tables on a user transaction.  e.g.:  when
+                  * attempting to revoke an user, the statement may time out
+                  * since the user-to-be-revoked transaction may have acquired 
+                  * shared locks on the permission system tables; hence, this
+                  * may not be desirable.  
+                  * 
+                  * All locks acquired by StatementPermission object's check()
+                  * method will be released when the system ends the nested 
+                  * transaction.
+                  * 
+                  * In Derby, the locks from read nested transactions come from
+                  * the same space as the parent transaction; hence, they do not
+                  * conflict with parent locks.
+                  */  
+                lcc.beginNestedTransaction(true);
+            	
+                try 
                 {
-                    ((StatementPermission) iter.next()).check( lcc, authorizationId, false);
-                }                    
+                    try 
+                    {
+                    	// perform the permission checking
+                        for (Iterator iter = requiredPermissionsList.iterator(); 
+                            iter.hasNext();) 
+                        {
+                            ((StatementPermission) iter.next()).check(lcc, 
+                                authorizationId, false);
+                        }
+                    } 
+                    finally 
+                    {
+                        dd.doneReading(ddMode, lcc);
+                    }
+                } 
+                finally 
+                {
+                	// make sure we commit; otherwise, we will end up with 
+                	// mismatch nested level in the language connection context.
+                    lcc.commitNestedTransaction();
+                }
             }
-		}
-	}
+        }
+    }
 
 	private static StandardException externalRoutineException(int operation, int sqlAllowed) {
 
@@ -282,4 +326,5 @@
 		if (userAccessLevel == NO_ACCESS)
 			throw StandardException.newException(SQLState.AUTH_DATABASE_CONNECTION_REFUSED);
 	}
+	
 }

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out?view=diff&rev=453935&r1=453934&r2=453935
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/grantRevokeDDL.out Sat Oct  7 08:33:24 2006
@@ -3135,4 +3135,83 @@
 ij(MAMTA2)> set connection mamta3;
 ij(MAMTA3)> select c3 from mamta2.t1Derby1847;
 ERROR: Failed with SQLSTATE 42X05
-ij(MAMTA3)> 
+ij(MAMTA3)> -- DERBY-1716
+-- Revoking select privilege from a user times out when that user still have
+-- a cursor open before the patch.
+set connection user1;
+ij(USER1)> drop table t1;
+ERROR: Failed with SQLSTATE 42Y55
+ij(USER1)> create table t1 (c varchar(1));
+0 rows inserted/updated/deleted
+ij(USER1)> insert into t1 values 'a', 'b', 'c';
+3 rows inserted/updated/deleted
+ij(USER1)> grant select on t1 to user2;
+0 rows inserted/updated/deleted
+ij(USER1)> set connection user2;
+ij(USER2)> autocommit off;
+ij(USER2)> GET CURSOR crs1 AS 'select * from user1.t1';
+ij(USER2)> next crs1;
+C   
+----
+a   
+ij(USER2)> set connection user1;
+ij(USER1)> -- should succeed without blocking
+revoke select on t1 from user2;
+0 rows inserted/updated/deleted
+ij(USER1)> set connection user2;
+ij(USER2)> -- still ok to fetch.
+next crs1;
+C   
+----
+b   
+ij(USER2)> next crs1;
+C   
+----
+c   
+ij(USER2)> close crs1;
+ij(USER2)> commit;
+ij(USER2)> -- should fail since select privilege got revoked
+GET CURSOR crs1 AS 'select * from user1.t1';
+ERROR: Failed with SQLSTATE 28508
+ij(USER2)> next crs1;
+IJ ERROR: Unable to establish cursor
+ij(USER2)> close crs1;
+IJ ERROR: Unable to establish cursor
+ij(USER2)> autocommit on;
+ij(USER2)> -- repeat the scenario
+set connection user1;
+ij(USER1)> grant select on t1 to user2;
+0 rows inserted/updated/deleted
+ij(USER1)> set connection user2;
+ij(USER2)> autocommit off;
+ij(USER2)> GET CURSOR crs1 AS 'select * from user1.t1';
+ij(USER2)> next crs1;
+C   
+----
+a   
+ij(USER2)> set connection user1;
+ij(USER1)> -- should succeed without blocking
+revoke select on t1 from user2;
+0 rows inserted/updated/deleted
+ij(USER1)> set connection user2;
+ij(USER2)> -- still ok to fetch.
+next crs1;
+C   
+----
+b   
+ij(USER2)> next crs1;
+C   
+----
+c   
+ij(USER2)> close crs1;
+ij(USER2)> commit;
+ij(USER2)> -- should fail since select privilege got revoked
+GET CURSOR crs1 AS 'select * from user1.t1';
+ERROR: Failed with SQLSTATE 28508
+ij(USER2)> next crs1;
+IJ ERROR: Unable to establish cursor
+ij(USER2)> close crs1;
+IJ ERROR: Unable to establish cursor
+ij(USER2)> autocommit on;
+ij(USER2)> set connection user1;
+ij(USER1)> 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql?view=diff&rev=453935&r1=453934&r2=453935
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL.sql Sat Oct  7 08:33:24 2006
@@ -1978,3 +1978,52 @@
 drop table t1Derby1847; 
 set connection mamta3;
 select c3 from mamta2.t1Derby1847; 
+
+-- DERBY-1716
+-- Revoking select privilege from a user times out when that user still have
+-- a cursor open before the patch.
+set connection user1;
+drop table t1;
+create table t1 (c varchar(1));
+insert into t1 values 'a', 'b', 'c';
+grant select on t1 to user2;
+set connection user2;
+autocommit off;
+GET CURSOR crs1 AS 'select * from user1.t1';
+next crs1;
+set connection user1;
+-- should succeed without blocking
+revoke select on t1 from user2;
+set connection user2;
+-- still ok to fetch.
+next crs1;
+next crs1;
+close crs1;
+commit;
+-- should fail since select privilege got revoked
+GET CURSOR crs1 AS 'select * from user1.t1';
+next crs1;
+close crs1;
+autocommit on;
+-- repeat the scenario
+set connection user1;
+grant select on t1 to user2;
+set connection user2;
+autocommit off;
+GET CURSOR crs1 AS 'select * from user1.t1';
+next crs1;
+set connection user1;
+-- should succeed without blocking
+revoke select on t1 from user2;
+set connection user2;
+-- still ok to fetch.
+next crs1;
+next crs1;
+close crs1;
+commit;
+-- should fail since select privilege got revoked
+GET CURSOR crs1 AS 'select * from user1.t1';
+next crs1;
+close crs1;
+autocommit on;
+set connection user1;

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties?view=diff&rev=453935&r1=453934&r2=453935
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/grantRevokeDDL_app.properties Sat Oct  7 08:33:24 2006
@@ -2,6 +2,9 @@
 ij.showNoConnectionsAtStart=true
 
 derby.database.sqlAuthorization=true
+derby.locks.deadlockTimeout=5
+derby.locks.waitTimeout=2
+
 useextdirs=true
 
 # DataSource properties, only used if ij.dataSource is set