Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9361511F71 for ; Mon, 7 Jul 2014 16:06:39 +0000 (UTC) Received: (qmail 30507 invoked by uid 500); 7 Jul 2014 16:06:39 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 30478 invoked by uid 500); 7 Jul 2014 16:06:39 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 30466 invoked by uid 99); 7 Jul 2014 16:06:39 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 07 Jul 2014 16:06:39 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 09BFD9A7A9B; Mon, 7 Jul 2014 16:06:38 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org Date: Mon, 07 Jul 2014 16:06:39 -0000 Message-Id: <6146767ae5344ade9b04896995249643@git.apache.org> In-Reply-To: <7ee6d626d4ae4cb1a64f6e6f3afb4f68@git.apache.org> References: <7ee6d626d4ae4cb1a64f6e6f3afb4f68@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [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 Authored: Mon Jul 7 17:50:26 2014 +0200 Committer: Sylvain Lebresne 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 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 flags = EnumSet.noneOf(Frame.Header.Flag.class); Codec codec = (Codec)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 currentTypes = new HashSet<>(); private final Set 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 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); + } +}