You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bl...@apache.org on 2017/07/07 11:43:22 UTC
[02/10] cassandra git commit: Fix nested Tuples/UDTs validation
Fix nested Tuples/UDTs validation
patch by Benjamin Lerer; reviewed by Andrés de la Peña for CASSANDRA-13646
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/b77e11cf
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/b77e11cf
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/b77e11cf
Branch: refs/heads/cassandra-3.0
Commit: b77e11cfd51ddb0f3ac07530399abe999df0573e
Parents: 0163b97
Author: Benjamin Lerer <b....@gmail.com>
Authored: Fri Jul 7 13:29:00 2017 +0200
Committer: Benjamin Lerer <b....@gmail.com>
Committed: Fri Jul 7 13:29:00 2017 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/cql3/UserTypes.java | 4 +-
.../apache/cassandra/db/marshal/TupleType.java | 45 ++++---------
.../apache/cassandra/db/marshal/UserType.java | 50 +++++----------
.../cassandra/serializers/TupleSerializer.java | 66 +++++++++++++++++++
.../serializers/UserTypeSerializer.java | 67 ++++++++++++++++++++
.../validation/entities/CollectionsTest.java | 36 +++++++++++
.../cql3/validation/entities/TupleTypeTest.java | 10 ++-
.../cql3/validation/entities/UserTypesTest.java | 9 +++
9 files changed, 219 insertions(+), 69 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index c966c92..08c6e48 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
2.2.11
+ * Fix nested Tuples/UDTs validation (CASSANDRA-13646)
* Remove unused max_value_size_in_mb config setting from yaml (CASSANDRA-13625
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/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 de3f545..6766d07 100644
--- a/src/java/org/apache/cassandra/cql3/UserTypes.java
+++ b/src/java/org/apache/cassandra/cql3/UserTypes.java
@@ -91,7 +91,7 @@ public abstract class UserTypes
private void validateAssignableTo(String keyspace, ColumnSpecification receiver) throws InvalidRequestException
{
if (!(receiver.type instanceof UserType))
- throw new InvalidRequestException(String.format("Invalid user type literal for %s of type %s", receiver, receiver.type.asCQL3Type()));
+ throw new InvalidRequestException(String.format("Invalid user type literal for %s of type %s", receiver.name, receiver.type.asCQL3Type()));
UserType ut = (UserType)receiver.type;
for (int i = 0; i < ut.size(); i++)
@@ -103,7 +103,7 @@ public abstract class UserTypes
ColumnSpecification fieldSpec = fieldSpecOf(receiver, i);
if (!value.testAssignment(keyspace, fieldSpec).isAssignable())
- throw new InvalidRequestException(String.format("Invalid user type literal for %s: field %s is not of type %s", receiver, field, fieldSpec.type.asCQL3Type()));
+ throw new InvalidRequestException(String.format("Invalid user type literal for %s: field %s is not of type %s", receiver.name, field, fieldSpec.type.asCQL3Type()));
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/src/java/org/apache/cassandra/db/marshal/TupleType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/TupleType.java b/src/java/org/apache/cassandra/db/marshal/TupleType.java
index 0d08a52..bf7eae4 100644
--- a/src/java/org/apache/cassandra/db/marshal/TupleType.java
+++ b/src/java/org/apache/cassandra/db/marshal/TupleType.java
@@ -39,11 +39,23 @@ public class TupleType extends AbstractType<ByteBuffer>
{
protected final List<AbstractType<?>> types;
+ private final TupleSerializer serializer;
+
public TupleType(List<AbstractType<?>> types)
{
for (int i = 0; i < types.size(); i++)
types.set(i, types.get(i).freeze());
this.types = types;
+ this.serializer = new TupleSerializer(fieldSerializers(types));
+ }
+
+ private static List<TypeSerializer<?>> fieldSerializers(List<AbstractType<?>> types)
+ {
+ int size = types.size();
+ List<TypeSerializer<?>> serializers = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
+ serializers.add(types.get(i).getSerializer());
+ return serializers;
}
public static TupleType getInstance(TypeParser parser) throws ConfigurationException, SyntaxException
@@ -119,37 +131,6 @@ public class TupleType extends AbstractType<ByteBuffer>
return 1;
}
- @Override
- public void validate(ByteBuffer bytes) throws MarshalException
- {
- ByteBuffer input = bytes.duplicate();
- for (int i = 0; i < size(); i++)
- {
- // we allow the input to have less fields than declared so as to support field addition.
- if (!input.hasRemaining())
- return;
-
- if (input.remaining() < 4)
- throw new MarshalException(String.format("Not enough bytes to read size of %dth component", i));
-
- int size = input.getInt();
-
- // size < 0 means null value
- if (size < 0)
- continue;
-
- if (input.remaining() < size)
- throw new MarshalException(String.format("Not enough bytes to read %dth component", i));
-
- ByteBuffer field = ByteBufferUtil.readBytes(input, size);
- types.get(i).validate(field);
- }
-
- // We're allowed to get less fields than declared, but not more
- if (input.hasRemaining())
- throw new MarshalException("Invalid remaining data after end of tuple value");
- }
-
/**
* Split a tuple value into its component values.
*/
@@ -297,7 +278,7 @@ public class TupleType extends AbstractType<ByteBuffer>
public TypeSerializer<ByteBuffer> getSerializer()
{
- return BytesSerializer.instance;
+ return serializer;
}
@Override
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/src/java/org/apache/cassandra/db/marshal/UserType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/UserType.java b/src/java/org/apache/cassandra/db/marshal/UserType.java
index 187deeb..93059cd 100644
--- a/src/java/org/apache/cassandra/db/marshal/UserType.java
+++ b/src/java/org/apache/cassandra/db/marshal/UserType.java
@@ -28,6 +28,8 @@ import org.apache.cassandra.cql3.*;
import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.cassandra.exceptions.SyntaxException;
import org.apache.cassandra.serializers.MarshalException;
+import org.apache.cassandra.serializers.TypeSerializer;
+import org.apache.cassandra.serializers.UserTypeSerializer;
import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.Pair;
@@ -42,6 +44,7 @@ public class UserType extends TupleType
public final ByteBuffer name;
private final List<ByteBuffer> fieldNames;
private final List<String> stringFieldNames;
+ private final UserTypeSerializer serializer;
public UserType(String keyspace, ByteBuffer name, List<ByteBuffer> fieldNames, List<AbstractType<?>> fieldTypes)
{
@@ -51,17 +54,22 @@ public class UserType extends TupleType
this.name = name;
this.fieldNames = fieldNames;
this.stringFieldNames = new ArrayList<>(fieldNames.size());
- for (ByteBuffer fieldName : fieldNames)
+ LinkedHashMap<String , TypeSerializer<?>> fieldSerializers = new LinkedHashMap<>(fieldTypes.size());
+ for (int i = 0, m = fieldNames.size(); i < m; i++)
{
+ ByteBuffer fieldName = fieldNames.get(i);
try
{
- stringFieldNames.add(ByteBufferUtil.string(fieldName, StandardCharsets.UTF_8));
+ String stringFieldName = ByteBufferUtil.string(fieldName, StandardCharsets.UTF_8);
+ stringFieldNames.add(stringFieldName);
+ fieldSerializers.put(stringFieldName, fieldTypes.get(i).getSerializer());
}
catch (CharacterCodingException ex)
{
throw new AssertionError("Got non-UTF8 field name for user-defined type: " + ByteBufferUtil.bytesToHex(fieldName), ex);
}
}
+ this.serializer = new UserTypeSerializer(fieldSerializers);
}
public static UserType getInstance(TypeParser parser) throws ConfigurationException, SyntaxException
@@ -109,38 +117,6 @@ public class UserType extends TupleType
return UTF8Type.instance.compose(name);
}
- // Note: the only reason we override this is to provide nicer error message, but since that's not that much code...
- @Override
- public void validate(ByteBuffer bytes) throws MarshalException
- {
- ByteBuffer input = bytes.duplicate();
- for (int i = 0; i < size(); i++)
- {
- // we allow the input to have less fields than declared so as to support field addition.
- if (!input.hasRemaining())
- return;
-
- if (input.remaining() < 4)
- throw new MarshalException(String.format("Not enough bytes to read size of %dth field %s", i, fieldNameAsString(i)));
-
- int size = input.getInt();
-
- // size < 0 means null value
- if (size < 0)
- continue;
-
- if (input.remaining() < size)
- throw new MarshalException(String.format("Not enough bytes to read %dth field %s", i, fieldNameAsString(i)));
-
- ByteBuffer field = ByteBufferUtil.readBytes(input, size);
- types.get(i).validate(field);
- }
-
- // We're allowed to get less fields than declared, but not more
- if (input.hasRemaining())
- throw new MarshalException("Invalid remaining data after end of UDT value");
- }
-
@Override
public Term fromJSONObject(Object parsed) throws MarshalException
{
@@ -243,4 +219,10 @@ public class UserType extends TupleType
{
return getClass().getName() + TypeParser.stringifyUserTypeParameters(keyspace, name, fieldNames, types);
}
+
+ @Override
+ public TypeSerializer<ByteBuffer> getSerializer()
+ {
+ return serializer;
+ }
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/src/java/org/apache/cassandra/serializers/TupleSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/TupleSerializer.java b/src/java/org/apache/cassandra/serializers/TupleSerializer.java
new file mode 100644
index 0000000..7cf71c6
--- /dev/null
+++ b/src/java/org/apache/cassandra/serializers/TupleSerializer.java
@@ -0,0 +1,66 @@
+/*
+ * 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.serializers;
+
+import java.nio.ByteBuffer;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+public class TupleSerializer extends BytesSerializer
+{
+ public final List<TypeSerializer<?>> fields;
+
+ public TupleSerializer(List<TypeSerializer<?>> fields)
+ {
+ this.fields = fields;
+ }
+
+ @Override
+ public void validate(ByteBuffer bytes) throws MarshalException
+ {
+ ByteBuffer input = bytes.duplicate();
+ for (int i = 0; i < fields.size(); i++)
+ {
+ // we allow the input to have less fields than declared so as to support field addition.
+ if (!input.hasRemaining())
+ return;
+
+ if (input.remaining() < 4)
+ throw new MarshalException(String.format("Not enough bytes to read size of %dth component", i));
+
+ int size = input.getInt();
+
+ // size < 0 means null value
+ if (size < 0)
+ continue;
+
+ if (input.remaining() < size)
+ throw new MarshalException(String.format("Not enough bytes to read %dth component", i));
+
+ ByteBuffer field = ByteBufferUtil.readBytes(input, size);
+ fields.get(i).validate(field);
+ }
+
+ // We're allowed to get less fields than declared, but not more
+ if (input.hasRemaining())
+ throw new MarshalException("Invalid remaining data after end of tuple value");
+ }
+}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/src/java/org/apache/cassandra/serializers/UserTypeSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/UserTypeSerializer.java b/src/java/org/apache/cassandra/serializers/UserTypeSerializer.java
new file mode 100644
index 0000000..472e39b
--- /dev/null
+++ b/src/java/org/apache/cassandra/serializers/UserTypeSerializer.java
@@ -0,0 +1,67 @@
+/*
+ * 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.serializers;
+
+import java.nio.ByteBuffer;
+import java.util.LinkedHashMap;
+import java.util.Map.Entry;
+
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+public class UserTypeSerializer extends BytesSerializer
+{
+ public final LinkedHashMap<String, TypeSerializer<?>> fields;
+
+ public UserTypeSerializer(LinkedHashMap<String, TypeSerializer<?>> fields)
+ {
+ this.fields = fields;
+ }
+
+ @Override
+ public void validate(ByteBuffer bytes) throws MarshalException
+ {
+ ByteBuffer input = bytes.duplicate();
+ int i = 0;
+ for (Entry<String, TypeSerializer<?>> entry : fields.entrySet())
+ {
+ // we allow the input to have less fields than declared so as to support field addition.
+ if (!input.hasRemaining())
+ return;
+
+ if (input.remaining() < 4)
+ throw new MarshalException(String.format("Not enough bytes to read size of %dth field %s", i, entry.getKey()));
+
+ int size = input.getInt();
+
+ // size < 0 means null value
+ if (size < 0)
+ continue;
+
+ if (input.remaining() < size)
+ throw new MarshalException(String.format("Not enough bytes to read %dth field %s", i, entry.getKey()));
+
+ ByteBuffer field = ByteBufferUtil.readBytes(input, size);
+ entry.getValue().validate(field);
+ i++;
+ }
+
+ // We're allowed to get less fields than declared, but not more
+ if (input.hasRemaining())
+ throw new MarshalException("Invalid remaining data after end of UDT value");
+ }
+}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
index 99d9695..69d5a5c 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
@@ -748,4 +748,40 @@ public class CollectionsTest extends CQLTester
execute("UPDATE %s SET s = s - ? , s = s + ? WHERE pk = ?", set(3), set(3, 4), 1);
assertRows(execute("SELECT * FROM %s WHERE pk = 1") , row(1, set(0, 1, 2, 4)));
}
+
+ @Test
+ public void testInsertingCollectionsWithInvalidElements() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, s frozen<set<tuple<int, text, double>>>)");
+ assertInvalidMessage("Invalid remaining data after end of tuple value",
+ "INSERT INTO %s (k, s) VALUES (0, ?)",
+ set(tuple(1, "1", 1.0, 1), tuple(2, "2", 2.0, 2)));
+
+ assertInvalidMessage("Invalid set literal for s: value (1, '1', 1.0, 1) is not of type tuple<int, text, double>",
+ "INSERT INTO %s (k, s) VALUES (0, {(1, '1', 1.0, 1)})");
+
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, l frozen<list<tuple<int, text, double>>>)");
+ assertInvalidMessage("Invalid remaining data after end of tuple value",
+ "INSERT INTO %s (k, l) VALUES (0, ?)",
+ list(tuple(1, "1", 1.0, 1), tuple(2, "2", 2.0, 2)));
+
+ assertInvalidMessage("Invalid list literal for l: value (1, '1', 1.0, 1) is not of type tuple<int, text, double>",
+ "INSERT INTO %s (k, l) VALUES (0, [(1, '1', 1.0, 1)])");
+
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, m frozen<map<tuple<int, text, double>, int>>)");
+ assertInvalidMessage("Invalid remaining data after end of tuple value",
+ "INSERT INTO %s (k, m) VALUES (0, ?)",
+ map(tuple(1, "1", 1.0, 1), 1, tuple(2, "2", 2.0, 2), 2));
+
+ assertInvalidMessage("Invalid map literal for m: key (1, '1', 1.0, 1) is not of type tuple<int, text, double>",
+ "INSERT INTO %s (k, m) VALUES (0, {(1, '1', 1.0, 1) : 1})");
+
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, m frozen<map<int, tuple<int, text, double>>>)");
+ assertInvalidMessage("Invalid remaining data after end of tuple value",
+ "INSERT INTO %s (k, m) VALUES (0, ?)",
+ map(1, tuple(1, "1", 1.0, 1), 2, tuple(2, "2", 2.0, 2)));
+
+ assertInvalidMessage("Invalid map literal for m: value (1, '1', 1.0, 1) is not of type tuple<int, text, double>",
+ "INSERT INTO %s (k, m) VALUES (0, {1 : (1, '1', 1.0, 1)})");
+ }
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java
index 0783dd1..3b4fb40 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/TupleTypeTest.java
@@ -123,6 +123,14 @@ public class TupleTypeTest extends CQLTester
assertInvalidMessage("Invalid tuple literal for t: too many elements. Type tuple<int, text, double> expects 3 but got 4",
"INSERT INTO %s (k, t) VALUES (0, (2, 'foo', 3.1, 'bar'))");
+
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, t frozen<tuple<int, tuple<int, text, double>>>)");
+ assertInvalidMessage("Invalid remaining data after end of tuple value",
+ "INSERT INTO %s (k, t) VALUES (0, ?)",
+ tuple(1, tuple(1, "1", 1.0, 1)));
+
+ assertInvalidMessage("Invalid tuple literal for t: component 1 is not of type tuple<int, text, double>",
+ "INSERT INTO %s (k, t) VALUES (0, (1, (1, '1', 1.0, 1)))");
}
@Test
@@ -137,7 +145,7 @@ public class TupleTypeTest extends CQLTester
// select using unset
assertInvalidMessage("Invalid unset value for tuple field number 0", "SELECT * FROM %s WHERE k = ? and t = (?,?,?)", unset(), unset(), unset(), unset());
}
-
+
/**
* Test the syntax introduced by #4851,
* migrated from cql_tests.py:TestCQL.tuple_notation_test()
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b77e11cf/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
index 3803e5c..9bafe4a 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
@@ -53,6 +53,15 @@ public class UserTypesTest extends CQLTester
"INSERT INTO %s (pk, t) VALUES (?, ?)", 1, "test");
assertInvalidMessage("Not enough bytes to read 0th field f",
"INSERT INTO %s (pk, t) VALUES (?, ?)", 1, Long.MAX_VALUE);
+
+ String type = createType("CREATE TYPE %s (a int, b tuple<int, text, double>)");
+ createTable("CREATE TABLE %s (k int PRIMARY KEY, t frozen<" + type + ">)");
+ assertInvalidMessage("Invalid remaining data after end of tuple value",
+ "INSERT INTO %s (k, t) VALUES (0, ?)",
+ userType(1, tuple(1, "1", 1.0, 1)));
+
+ assertInvalidMessage("Invalid user type literal for t: field b is not of type tuple<int, text, double>",
+ "INSERT INTO %s (k, t) VALUES (0, {a: 1, b: (1, '1', 1.0, 1)})");
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org