You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2010/11/27 20:25:37 UTC
svn commit: r1039731 - in /cassandra/branches/cassandra-0.7: ./
src/java/org/apache/cassandra/cli/ src/java/org/apache/cassandra/thrift/
src/java/org/apache/cassandra/utils/
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));
}
}