Author: jbellis
Date: Sat Nov 27 19:25:36 2010
New Revision: 1039731

URL: http://svn.apache.org/viewvc?rev=1039731&view=rev
Log:
Validate that column names in column_metadata are valid for the defined 
comparator, and decode properly in cli
patch by Pavel Yaskevich and jbellis for CASSANDRA-1773

Modified:
    cassandra/branches/cassandra-0.7/CHANGES.txt
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliClient.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliMain.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/CassandraServer.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/ThriftValidation.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/utils/FBUtilities.java

Modified: cassandra/branches/cassandra-0.7/CHANGES.txt
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/CHANGES.txt?rev=1039731&r1=1039730&r2=1039731&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.7/CHANGES.txt (original)
+++ cassandra/branches/cassandra-0.7/CHANGES.txt Sat Nov 27 19:25:36 2010
@@ -14,6 +14,8 @@ dev
    (CASSANDRA-1774)
  * improvements to Debian init script (CASSANDRA-1772)
  * use local classloader to check for version.properties (CASSANDRA-1778)
+ * Validate that column names in column_metadata are valid for the
+   defined comparator, and decode properly in cli (CASSANDRA-1773)
 
 
 0.7.0-rc1

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliClient.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliClient.java?rev=1039731&r1=1039730&r2=1039731&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliClient.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliClient.java
 Sat Nov 27 19:25:36 2010
@@ -1197,10 +1197,13 @@ public class CliClient extends CliUserHe
                 sessionState.out.println("    Replication Factor: " + 
ks_def.replication_factor);
             sessionState.out.println("  Column Families:");
 
+            boolean isSuper;
+
             Collections.sort(ks_def.cf_defs, new CfDefNamesComparator());
             for (CfDef cf_def : ks_def.cf_defs)
             {
-                sessionState.out.printf("    ColumnFamily: %s%s\n", 
cf_def.name, cf_def.column_type.equals("Super") ? " (Super)" : "");
+                isSuper = cf_def.column_type.equals("Super");
+                sessionState.out.printf("    ColumnFamily: %s%s\n", 
cf_def.name, isSuper ? " (Super)" : "");
 
                 if (cf_def.comment != null && !cf_def.comment.isEmpty())
                 {
@@ -1221,7 +1224,8 @@ public class CliClient extends CliUserHe
                     String leftSpace = "      ";
                     String columnLeftSpace = leftSpace + "    ";
 
-                    AbstractType columnNameValidator = 
getFormatTypeForColumn(cf_def.comparator_type);
+                    AbstractType columnNameValidator = 
getFormatTypeForColumn(isSuper ? cf_def.subcomparator_type
+                                                                               
       : cf_def.comparator_type);
 
                     sessionState.out.println(leftSpace + "Column Metadata:");
                     for (ColumnDef columnDef : cf_def.getColumn_metadata())

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliMain.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliMain.java?rev=1039731&r1=1039730&r2=1039731&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliMain.java 
(original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/cli/CliMain.java 
Sat Nov 27 19:25:36 2010
@@ -325,7 +325,12 @@ public class CliMain
         {
             prompt = (inCompoundStatement) ? "...\t" : getPrompt(cliClient);
 
-            line = reader.readLine(prompt).trim();
+            line = reader.readLine(prompt);
+
+            if (line == null)
+                return;
+
+            line = line.trim();
 
             if (line.isEmpty())
                 continue;

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/CassandraServer.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/CassandraServer.java?rev=1039731&r1=1039730&r2=1039731&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/CassandraServer.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/CassandraServer.java
 Sat Nov 27 19:25:36 2010
@@ -704,6 +704,7 @@ public class CassandraServer implements 
     public String system_add_column_family(CfDef cf_def) throws 
InvalidRequestException, TException
     {
         state().hasColumnFamilyListAccess(Permission.WRITE);
+        ThriftValidation.validateCfDef(cf_def);
         try
         {
             applyMigrationOnStage(new 
AddColumnFamily(convertToCFMetaData(cf_def)));

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/ThriftValidation.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/ThriftValidation.java?rev=1039731&r1=1039730&r2=1039731&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/ThriftValidation.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/thrift/ThriftValidation.java
 Sat Nov 27 19:25:36 2010
@@ -25,6 +25,7 @@ import java.util.Arrays;
 import java.util.Comparator;
 import java.util.Set;
 
+import org.apache.cassandra.config.ConfigurationException;
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.db.ColumnFamily;
 import org.apache.cassandra.db.ColumnFamilyType;
@@ -377,4 +378,39 @@ public class ThriftValidation
         }
         throw new InvalidRequestException("No indexed columns present in index 
clause with operator EQ");
     }
+
+    public static void validateCfDef(CfDef cf_def) throws 
InvalidRequestException
+    {
+        try
+        {
+            DatabaseDescriptor.getComparator(cf_def.comparator_type);
+            DatabaseDescriptor.getComparator(cf_def.subcomparator_type);
+            DatabaseDescriptor.getComparator(cf_def.default_validation_class);
+
+            if (cf_def.column_metadata == null)
+                return;
+
+            AbstractType comparator = cf_def.subcomparator_type == null
+                                    ? 
DatabaseDescriptor.getComparator(cf_def.comparator_type)
+                                    : 
DatabaseDescriptor.getComparator(cf_def.subcomparator_type);
+            for (ColumnDef c : cf_def.column_metadata)
+            {
+                DatabaseDescriptor.getComparator(c.validation_class);
+
+                try
+                {
+                    comparator.validate(c.name);
+                }
+                catch (MarshalException e)
+                {
+                    throw new InvalidRequestException(String.format("Column 
name %s is not valid for comparator %s",
+                                                                    
FBUtilities.bytesToHex(c.name), cf_def.comparator_type));
+                }
+            }
+        }
+        catch (ConfigurationException e)
+        {
+            throw new InvalidRequestException(e.getMessage());
+        }
+    }
 }

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/utils/FBUtilities.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/utils/FBUtilities.java?rev=1039731&r1=1039730&r2=1039731&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/utils/FBUtilities.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/utils/FBUtilities.java
 Sat Nov 27 19:25:36 2010
@@ -573,13 +573,13 @@ public class FBUtilities
         }
         catch (NoSuchFieldException e)
         {
-            ConfigurationException ex = new ConfigurationException("Invalid 
comparator: must define a public static instance field.");
+            ConfigurationException ex = new ConfigurationException("Invalid 
comparator " + compareWith + " : must define a public static instance field.");
             ex.initCause(e);
             throw ex;
         }
         catch (IllegalAccessException e)
         {
-            ConfigurationException ex = new ConfigurationException("Invalid 
comparator: must define a public static instance field.");
+            ConfigurationException ex = new ConfigurationException("Invalid 
comparator " + compareWith + " : must define a public static instance field.");
             ex.initCause(e);
             throw ex;
         }
@@ -599,7 +599,7 @@ public class FBUtilities
         }
         catch (ClassNotFoundException e)
         {
-            throw new ConfigurationException(String.format("Unable to find %s 
class '%s': is the CLASSPATH set correctly?", readable, classname));
+            throw new ConfigurationException(String.format("Unable to find %s 
class '%s'", readable, classname));
         }
     }
 


Reply via email to