cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ble...@apache.org
Subject [03/10] cassandra git commit: Fix nested Tuples/UDTs validation
Date Fri, 07 Jul 2017 11:43:23 GMT
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.11
Commit: b77e11cfd51ddb0f3ac07530399abe999df0573e
Parents: 0163b97
Author: Benjamin Lerer <b.lerer@gmail.com>
Authored: Fri Jul 7 13:29:00 2017 +0200
Committer: Benjamin Lerer <b.lerer@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


Mime
View raw message