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 da...@apache.org on 2009/01/02 19:02:51 UTC

svn commit: r730802 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/util/ engine/org/apache/derby/impl/sql/compile/ engine/org/apache/derby/impl/sql/execute/ testing/org/apache/derbyTesting/functionTests/tests/lang/

Author: dag
Date: Fri Jan  2 10:02:50 2009
New Revision: 730802

URL: http://svn.apache.org/viewvc?rev=730802&view=rev
Log:
DERBY-3137 SQL roles: add catalog support

Patch DERBY-3137-setRoleAsStringFix-b: When setting a role with a
literal string argument, we should do the same checks as for dynamic
parameter on the identifier validity. Added that, plus new test cases
and some refactoring.


Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/IdUtil.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/SetRoleConstantAction.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RolesTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/IdUtil.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/IdUtil.java?rev=730802&r1=730801&r2=730802&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/IdUtil.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/util/IdUtil.java Fri Jan  2 10:02:50 2009
@@ -30,6 +30,7 @@
 import java.util.Vector;
 import java.util.HashSet;
 import java.util.Properties;
+import org.apache.derby.iapi.reference.Limits;
 
 /**
   Utility class for parsing and producing string representations of
@@ -734,4 +735,48 @@
 		else
 			return list+","+delimitedId;
 	}
+
+	/**
+	 * Parse role identifier to internal, case normal form. It should not be
+	 * NONE nor exceed Limits.MAX_IDENTIFIER_LENGTH.
+	 *
+	 * @param roleName role identifier to check (SQL form, has possible quoting)
+	 * @return the role name to use (internal, case normal form).
+	 * @exception StandardException normal error policy
+	 */
+	public static String parseRoleId(String roleName) throws StandardException
+	{
+		roleName.trim();
+		// NONE is a special case and is not allowed with its special
+		// meaning in SET ROLE <value specification>. Even if there is
+		// a role with case normal form "NONE", we require it to be
+		// delimited here, since it would have had to be delimited to
+		// get created, too. We could have chosen to be lenient here,
+		// but it seems safer to be restrictive.
+		if (StringUtil.SQLToUpperCase(roleName).equals("NONE")) {
+			throw StandardException.newException(SQLState.ID_PARSE_ERROR);
+		}
+
+		roleName = parseSQLIdentifier(roleName);
+		checkIdentifierLengthLimit(roleName, Limits.MAX_IDENTIFIER_LENGTH);
+
+		return roleName;
+	}
+
+	/**
+	 * Check that identifier is not too long
+	 * @param identifier identifier (in case normal form) to check
+	 * @param identifier_length_limit maximum legal length
+	 * @exception StandardException normal error policy
+	 */
+	public static void checkIdentifierLengthLimit(String identifier,
+												  int identifier_length_limit)
+			throws StandardException
+	{
+		if (identifier.length() > identifier_length_limit)
+			throw StandardException.newException
+				(SQLState.LANG_IDENTIFIER_TOO_LONG,
+				 identifier,
+				 String.valueOf(identifier_length_limit));
+    }
 }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj?rev=730802&r1=730801&r2=730802&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj Fri Jan  2 10:02:50 2009
@@ -153,6 +153,7 @@
 import org.apache.derby.iapi.util.ReuseFactory;
 import org.apache.derby.iapi.services.io.FormatableBitSet;
 import org.apache.derby.iapi.util.StringUtil;
+import org.apache.derby.iapi.util.IdUtil;
 
 import java.sql.Types;
 import java.util.List;
@@ -1609,13 +1610,6 @@
         initUnnamedParameterList();
     } // End of initStatement
 
-    private void checkIdentifierLengthLimit( String identifier, int identifier_length_limit) 
-        throws StandardException
-    {
-	if (identifier.length() > identifier_length_limit)
-		throw StandardException.newException(SQLState.LANG_IDENTIFIER_TOO_LONG, identifier, String.valueOf(identifier_length_limit));
-    }
-
     private ValueNode getJdbcIntervalNode( int intervalType) throws StandardException
     {
         return (ValueNode) nodeFactory.getNode( C_NodeTypes.INT_CONSTANT_NODE,
@@ -4597,9 +4591,9 @@
 		}
 
 		//limit the qualifiedId to the id length limit passed to this method
-		checkIdentifierLengthLimit(qualifiedId, id_length_limit);
+		IdUtil.checkIdentifierLengthLimit(qualifiedId, id_length_limit);
 		if (schemaName != null)
-			checkIdentifierLengthLimit(schemaName, Limits.MAX_IDENTIFIER_LENGTH);
+			IdUtil.checkIdentifierLengthLimit(schemaName, Limits.MAX_IDENTIFIER_LENGTH);
 
 		return (TableName) nodeFactory.getNode(
 								nodeType,
@@ -7860,11 +7854,11 @@
 			columnName = thirdName;
 		}
 
-		checkIdentifierLengthLimit(columnName, Limits.MAX_IDENTIFIER_LENGTH);
+		IdUtil.checkIdentifierLengthLimit(columnName, Limits.MAX_IDENTIFIER_LENGTH);
 		if (schemaName != null)
-			checkIdentifierLengthLimit(schemaName, Limits.MAX_IDENTIFIER_LENGTH);
+			IdUtil.checkIdentifierLengthLimit(schemaName, Limits.MAX_IDENTIFIER_LENGTH);
 		if (tableName != null)
-			checkIdentifierLengthLimit(tableName, Limits.MAX_IDENTIFIER_LENGTH);
+			IdUtil.checkIdentifierLengthLimit(tableName, Limits.MAX_IDENTIFIER_LENGTH);
 
 		if (tableName != null)
 		{
@@ -11645,7 +11639,8 @@
 |
 	roleName = string()
 	{
-		checkIdentifierLengthLimit(roleName, Limits.MAX_IDENTIFIER_LENGTH);
+		roleName = IdUtil.parseRoleId(roleName);
+
 		return (StatementNode) nodeFactory.getNode(
 			C_NodeTypes.SET_ROLE_NODE,
 			roleName,
@@ -11718,7 +11713,7 @@
 |	schemaName = string()
 	{
 		/* Max length for schema name is Limits.MAX_IDENTIFIER_LENGTH */
-		checkIdentifierLengthLimit(schemaName, Limits.MAX_IDENTIFIER_LENGTH);
+		IdUtil.checkIdentifierLengthLimit(schemaName, Limits.MAX_IDENTIFIER_LENGTH);
 		return (StatementNode) nodeFactory.getNode(
 								C_NodeTypes.SET_SCHEMA_NODE,
 								schemaName,
@@ -13365,7 +13360,7 @@
  
  		if (checkLength) {//if checkLength false, then calling method would do the length limit checks
 			//limit the identifier to the id length limit passed to this method
-			checkIdentifierLengthLimit(str, id_length_limit);
+			IdUtil.checkIdentifierLengthLimit(str, id_length_limit);
 		}
 		// Remember whether last token was a delimited identifier
 		nextToLastTokenDelimitedIdentifier = lastTokenDelimitedIdentifier;
@@ -13379,7 +13374,7 @@
 	{
 		if (checkLength) {//if checkLength false, then calling method would do the length limit checks
 			//limit the identifier to the id length limit passed to this method
-			checkIdentifierLengthLimit(str, id_length_limit);
+			IdUtil.checkIdentifierLengthLimit(str, id_length_limit);
 		} 
 		return str;
 	}

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/SetRoleConstantAction.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/SetRoleConstantAction.java?rev=730802&r1=730801&r2=730802&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/SetRoleConstantAction.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/SetRoleConstantAction.java Fri Jan  2 10:02:50 2009
@@ -119,25 +119,13 @@
             // SQL 2003, section 18.3, GR2: trim whitespace first, and
             // interpret as identifier, then we convert it to case normal form
             // here.
-            thisRoleName = dvs.getString();
+            String roleId = dvs.getString();
 
-            if (thisRoleName == null) {
+            if (roleId == null) {
                 throw StandardException.newException(SQLState.ID_PARSE_ERROR);
             }
 
-            thisRoleName = thisRoleName.trim();
-
-            // NONE is a special case and is not allowed with its special
-            // meaning in SET ROLE ?. Even if there is a role with case normal
-            // form "NONE", we require it to be delimited here, since it would
-            // have had to be delimited to get created, too. We could have
-            // chosen to be lenient here, but it seems safer to be restrictive.
-            if (StringUtil.SQLToUpperCase(thisRoleName).equals("NONE")) {
-                throw StandardException.newException(SQLState.ID_PARSE_ERROR);
-            }
-
-            thisRoleName = IdUtil.parseSQLIdentifier(thisRoleName);
-
+            thisRoleName = IdUtil.parseRoleId(roleId);
         }
 
         RoleGrantDescriptor rdDef = null;

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RolesTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RolesTest.java?rev=730802&r1=730801&r2=730802&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RolesTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/RolesTest.java Fri Jan  2 10:02:50 2009
@@ -492,6 +492,16 @@
 
         doDynamicSetRole(_conn);
 
+        doStmt("set role 'NONE'",
+               sqlAuthorizationRequired, idParseError, idParseError);
+        doStmt("set role 'none'",
+               sqlAuthorizationRequired, idParseError, idParseError);
+        doStmt("set role '\"NONE\"'",
+               sqlAuthorizationRequired, null, invalidRole);
+        doStmt("set role ' '",
+               sqlAuthorizationRequired, idParseError, idParseError);
+
+
         doStmt("set role bar",
                sqlAuthorizationRequired, null , null /* direct grant */);
         doStmt("set role notGranted",
@@ -1060,6 +1070,14 @@
         }
 
         try {
+            pstmt.setString(1, " ");
+            int rowcnt = pstmt.executeUpdate();
+            fail("Expected syntax error on identifier");
+        } catch (SQLException e) {
+            assertSQLState(idParseError, e);
+        }
+
+        try {
             pstmt.setString(1, null);
             int rowcnt = pstmt.executeUpdate();
             fail("Expected syntax error on identifier");
@@ -1081,6 +1099,14 @@
             }
 
             try {
+                pstmt.setString(1, "none");
+                int rowcnt = pstmt.executeUpdate();
+                fail("NONE should not be allowed as a dynamic parameter");
+            } catch (SQLException e) {
+                assertSQLState(idParseError, e);
+            }
+
+            try {
                 pstmt.setString(1, "\"NONE\"");
                 int rowcnt = pstmt.executeUpdate();
                 assertEquals("rowcount from set role ? not 0", rowcnt, 0);