cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [2/4] git commit: Properly reject unknown UDT fields
Date Mon, 07 Jul 2014 16:06:39 GMT
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 <sylvain@datastax.com>
Authored: Mon Jul 7 17:50:26 2014 +0200
Committer: Sylvain Lebresne <sylvain@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);
+    }
+}


Mime
View raw message