You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/07/07 18:06:39 UTC
[2/4] git commit: Properly reject unknown UDT fields
Properly reject unknown UDT fields
patch by slebresne; reviewed by tjake for CASSANDRA-7484
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/8d87e0e2
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/8d87e0e2
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/8d87e0e2
Branch: refs/heads/trunk
Commit: 8d87e0e25e7252926bbe8b671bd86262ba7bc246
Parents: 0c6fad4
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Jul 7 17:50:26 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Mon Jul 7 17:50:26 2014 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/cql3/Constants.java | 8 ++-
.../org/apache/cassandra/cql3/UserTypes.java | 11 +++++
.../cassandra/cql3/VariableSpecifications.java | 6 +++
.../org/apache/cassandra/transport/Message.java | 51 +++++++++++---------
.../org/apache/cassandra/cql3/CQLTester.java | 30 +++++++++---
.../apache/cassandra/cql3/UserTypesTest.java | 33 +++++++++++++
7 files changed, 110 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 095624d..491ad6d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -8,6 +8,7 @@
commit log segments (CASSANDRA-7437)
* Remove left-over rows_per_partition_to_cache (CASSANDRA-7493)
* Fix error when CONTAINS is used with a bind marker (CASSANDRA-7502)
+ * Properly reject unknown UDT field (CASSANDRA-7484)
Merged from 2.0:
* Fix CC#collectTimeOrderedData() tombstone optimisations (CASSANDRA-7394)
* Support DISTINCT for static columns and fix behaviour when DISTINC is
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/src/java/org/apache/cassandra/cql3/Constants.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/Constants.java b/src/java/org/apache/cassandra/cql3/Constants.java
index 58b11dd..48f62dc 100644
--- a/src/java/org/apache/cassandra/cql3/Constants.java
+++ b/src/java/org/apache/cassandra/cql3/Constants.java
@@ -58,6 +58,12 @@ public abstract class Constants
// We return null because that makes life easier for collections
return null;
}
+
+ @Override
+ public String toString()
+ {
+ return "null";
+ }
};
public Term prepare(String keyspace, ColumnSpecification receiver) throws InvalidRequestException
@@ -76,7 +82,7 @@ public abstract class Constants
@Override
public String toString()
{
- return null;
+ return "null";
}
};
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/src/java/org/apache/cassandra/cql3/UserTypes.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/UserTypes.java b/src/java/org/apache/cassandra/cql3/UserTypes.java
index 876de2a..bb6e7d0 100644
--- a/src/java/org/apache/cassandra/cql3/UserTypes.java
+++ b/src/java/org/apache/cassandra/cql3/UserTypes.java
@@ -57,12 +57,15 @@ public abstract class UserTypes
UserType ut = (UserType)receiver.type;
boolean allTerminal = true;
List<Term> values = new ArrayList<>(entries.size());
+ int foundValues = 0;
for (int i = 0; i < ut.size(); i++)
{
ColumnIdentifier field = new ColumnIdentifier(ut.fieldName(i), UTF8Type.instance);
Term.Raw raw = entries.get(field);
if (raw == null)
raw = Constants.NULL_LITERAL;
+ else
+ ++foundValues;
Term value = raw.prepare(keyspace, fieldSpecOf(receiver, i));
if (value instanceof Term.NonTerminal)
@@ -70,6 +73,14 @@ public abstract class UserTypes
values.add(value);
}
+ if (foundValues != entries.size())
+ {
+ // We had some field that are not part of the type
+ for (ColumnIdentifier id : entries.keySet())
+ if (!ut.fieldNames().contains(id.bytes))
+ throw new InvalidRequestException(String.format("Unknown field '%s' in value of user defined type %s", id, ut.getNameAsString()));
+ }
+
DelayedValue value = new DelayedValue(((UserType)receiver.type), values);
return allTerminal ? value.bind(QueryOptions.DEFAULT) : value;
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/src/java/org/apache/cassandra/cql3/VariableSpecifications.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/VariableSpecifications.java b/src/java/org/apache/cassandra/cql3/VariableSpecifications.java
index ecdba6f..ef78619 100644
--- a/src/java/org/apache/cassandra/cql3/VariableSpecifications.java
+++ b/src/java/org/apache/cassandra/cql3/VariableSpecifications.java
@@ -49,4 +49,10 @@ public class VariableSpecifications
spec = new ColumnSpecification(spec.ksName, spec.cfName, name, spec.type);
specs[bindIndex] = spec;
}
+
+ @Override
+ public String toString()
+ {
+ return Arrays.toString(specs);
+ }
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/src/java/org/apache/cassandra/transport/Message.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/transport/Message.java b/src/java/org/apache/cassandra/transport/Message.java
index eea86e5..b02b176 100644
--- a/src/java/org/apache/cassandra/transport/Message.java
+++ b/src/java/org/apache/cassandra/transport/Message.java
@@ -270,41 +270,48 @@ public abstract class Message
EnumSet<Frame.Header.Flag> flags = EnumSet.noneOf(Frame.Header.Flag.class);
Codec<Message> codec = (Codec<Message>)message.type.codec;
- int messageSize = codec.encodedSize(message, version);
- ByteBuf body;
- if (message instanceof Response)
+ try
{
- UUID tracingId = ((Response)message).getTracingId();
- if (tracingId != null)
+ int messageSize = codec.encodedSize(message, version);
+ ByteBuf body;
+ if (message instanceof Response)
{
- body = CBUtil.allocator.buffer(CBUtil.sizeOfUUID(tracingId) + messageSize);
- CBUtil.writeUUID(tracingId, body);
- flags.add(Frame.Header.Flag.TRACING);
+ UUID tracingId = ((Response)message).getTracingId();
+ if (tracingId != null)
+ {
+ body = CBUtil.allocator.buffer(CBUtil.sizeOfUUID(tracingId) + messageSize);
+ CBUtil.writeUUID(tracingId, body);
+ flags.add(Frame.Header.Flag.TRACING);
+ }
+ else
+ {
+ body = CBUtil.allocator.buffer(messageSize);
+ }
}
else
{
+ assert message instanceof Request;
body = CBUtil.allocator.buffer(messageSize);
+ if (((Request)message).isTracingRequested())
+ flags.add(Frame.Header.Flag.TRACING);
}
- }
- else
- {
- assert message instanceof Request;
- body = CBUtil.allocator.buffer(messageSize);
- if (((Request)message).isTracingRequested())
- flags.add(Frame.Header.Flag.TRACING);
- }
- try
- {
- codec.encode(message, body, version);
+ try
+ {
+ codec.encode(message, body, version);
+ }
+ catch (Throwable e)
+ {
+ body.release();
+ throw e;
+ }
+
+ results.add(Frame.create(message.type, message.getStreamId(), version, flags, body));
}
catch (Throwable e)
{
- body.release();
throw ErrorMessage.wrap(e, message.getStreamId());
}
-
- results.add(Frame.create(message.type, message.getStreamId(), version, flags, body));
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/test/unit/org/apache/cassandra/cql3/CQLTester.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java
index b861d49..38aff2b 100644
--- a/test/unit/org/apache/cassandra/cql3/CQLTester.java
+++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java
@@ -60,6 +60,7 @@ public abstract class CQLTester
}
private String currentTable;
+ private final Set<String> currentTypes = new HashSet<>();
private final Set<String> currentTypes = new HashSet<>();
@@ -80,8 +81,10 @@ public abstract class CQLTester
if (currentTable == null)
return;
- final String toDrop = currentTable;
+ final String tableToDrop = currentTable;
+ final Set<String> typesToDrop = currentTypes.isEmpty() ? Collections.emptySet() : new HashSet(currentTypes);
currentTable = null;
+ currentTypes.clear();
// We want to clean up after the test, but dropping a table is rather long so just do that asynchronously
StorageService.tasks.execute(new Runnable()
@@ -90,7 +93,10 @@ public abstract class CQLTester
{
try
{
- schemaChange(String.format("DROP TABLE %s.%s", KEYSPACE, toDrop));
+ schemaChange(String.format("DROP TABLE %s.%s", KEYSPACE, tableToDrop));
+
+ for (String typeName : typesToDrop)
+ schemaChange(String.format("DROP TYPE %s.%s", KEYSPACE, typeName));
// Dropping doesn't delete the sstables. It's not a huge deal but it's cleaner to cleanup after us
// Thas said, we shouldn't delete blindly before the SSTableDeletingTask for the table we drop
@@ -99,15 +105,15 @@ public abstract class CQLTester
final CountDownLatch latch = new CountDownLatch(1);
StorageService.tasks.execute(new Runnable()
- {
- public void run()
{
- latch.countDown();
- }
+ public void run()
+ {
+ latch.countDown();
+ }
});
latch.await(2, TimeUnit.SECONDS);
- removeAllSSTables(KEYSPACE, toDrop);
+ removeAllSSTables(KEYSPACE, tableToDrop);
}
catch (Exception e)
{
@@ -127,6 +133,16 @@ public abstract class CQLTester
}
}
+ protected String createType(String query)
+ {
+ String typeName = "type_" + seqNumber.getAndIncrement();
+ String fullQuery = String.format(query, KEYSPACE + "." + typeName);
+ currentTypes.add(typeName);
+ logger.info(fullQuery);
+ schemaChange(fullQuery);
+ return typeName;
+ }
+
protected void createTable(String query)
{
currentTable = "table_" + seqNumber.getAndIncrement();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8d87e0e2/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/UserTypesTest.java b/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
new file mode 100644
index 0000000..a8fd7a4
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.cql3;
+
+import org.junit.Test;
+
+public class UserTypesTest extends CQLTester
+{
+ @Test
+ public void testInvalidField() throws Throwable
+ {
+ String myType = createType("CREATE TYPE %s (f int)");
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, v " + myType + ")");
+
+ // 's' is not a field of myType
+ assertInvalid("INSERT INTO %s (k, v) VALUES (?, {s : ?})", 0, 1);
+ }
+}