Repository: cassandra Updated Branches: refs/heads/trunk b2abcb7fc -> 4a5c282f7
Follow-up for 7523: prevent old clients from getting new type codes. Patch by jmckenzie; reviewed by Aleksey Yeschenko for CASSANDRA-9219 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/4a5c282f Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/4a5c282f Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/4a5c282f Branch: refs/heads/trunk Commit: 4a5c282f7615cc97929d76f36fe82e190fecbb89 Parents: b2abcb7 Author: Josh McKenzie <[email protected]> Authored: Thu May 14 15:33:29 2015 -0500 Committer: Josh McKenzie <[email protected]> Committed: Thu May 14 15:33:29 2015 -0500 ---------------------------------------------------------------------- .../cassandra/db/marshal/SimpleDateType.java | 2 +- .../apache/cassandra/db/marshal/TimeType.java | 3 +- .../apache/cassandra/transport/DataType.java | 82 +++++++++----- .../apache/cassandra/transport/OptionCodec.java | 14 +-- .../cassandra/transport/DataTypeTest.java | 108 +++++++++++++++++++ 5 files changed, 172 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a5c282f/src/java/org/apache/cassandra/db/marshal/SimpleDateType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/SimpleDateType.java b/src/java/org/apache/cassandra/db/marshal/SimpleDateType.java index a34646f..225b9cc 100644 --- a/src/java/org/apache/cassandra/db/marshal/SimpleDateType.java +++ b/src/java/org/apache/cassandra/db/marshal/SimpleDateType.java @@ -39,6 +39,7 @@ public class SimpleDateType extends AbstractType<Integer> return ByteBufferUtil.compareUnsigned(o1, o2); } + @Override public boolean isByteOrderComparable() { return true; @@ -61,7 +62,6 @@ public class SimpleDateType extends AbstractType<Integer> return this == otherType || otherType == IntegerType.instance; } - @Override public Term fromJSONObject(Object parsed) throws MarshalException { try http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a5c282f/src/java/org/apache/cassandra/db/marshal/TimeType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/TimeType.java b/src/java/org/apache/cassandra/db/marshal/TimeType.java index c5c7b98..c241a38 100644 --- a/src/java/org/apache/cassandra/db/marshal/TimeType.java +++ b/src/java/org/apache/cassandra/db/marshal/TimeType.java @@ -45,6 +45,7 @@ public class TimeType extends AbstractType<Long> return decompose(TimeSerializer.timeStringToLong(source)); } + @Override public boolean isByteOrderComparable() { return true; @@ -62,7 +63,6 @@ public class TimeType extends AbstractType<Long> return this == otherType || otherType == LongType.instance; } - @Override public Term fromJSONObject(Object parsed) throws MarshalException { try @@ -82,6 +82,7 @@ public class TimeType extends AbstractType<Long> return '"' + TimeSerializer.instance.toString(TimeSerializer.instance.deserialize(buffer)) + '"'; } + @Override public CQL3Type asCQL3Type() { return CQL3Type.Native.TIME; http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a5c282f/src/java/org/apache/cassandra/transport/DataType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/transport/DataType.java b/src/java/org/apache/cassandra/transport/DataType.java index e13194d..a78b740 100644 --- a/src/java/org/apache/cassandra/transport/DataType.java +++ b/src/java/org/apache/cassandra/transport/DataType.java @@ -24,6 +24,8 @@ import java.util.HashMap; import java.util.Map; import java.util.List; +import com.google.common.annotations.VisibleForTesting; + import io.netty.buffer.ByteBuf; import org.apache.cassandra.exceptions.RequestValidationException; @@ -32,35 +34,36 @@ import org.apache.cassandra.utils.Pair; public enum DataType implements OptionCodec.Codecable<DataType> { - CUSTOM (0, null), - ASCII (1, AsciiType.instance), - BIGINT (2, LongType.instance), - BLOB (3, BytesType.instance), - BOOLEAN (4, BooleanType.instance), - COUNTER (5, CounterColumnType.instance), - DECIMAL (6, DecimalType.instance), - DOUBLE (7, DoubleType.instance), - FLOAT (8, FloatType.instance), - INT (9, Int32Type.instance), - TEXT (10, UTF8Type.instance), - TIMESTAMP(11, TimestampType.instance), - UUID (12, UUIDType.instance), - VARCHAR (13, UTF8Type.instance), - VARINT (14, IntegerType.instance), - TIMEUUID (15, TimeUUIDType.instance), - INET (16, InetAddressType.instance), - DATE (17, DateType.instance), - TIME (18, TimeType.instance), - LIST (32, null), - MAP (33, null), - SET (34, null), - UDT (48, null), - TUPLE (49, null); + CUSTOM (0, null, 1), + ASCII (1, AsciiType.instance, 1), + BIGINT (2, LongType.instance, 1), + BLOB (3, BytesType.instance, 1), + BOOLEAN (4, BooleanType.instance, 1), + COUNTER (5, CounterColumnType.instance, 1), + DECIMAL (6, DecimalType.instance, 1), + DOUBLE (7, DoubleType.instance, 1), + FLOAT (8, FloatType.instance, 1), + INT (9, Int32Type.instance, 1), + TEXT (10, UTF8Type.instance, 1), + TIMESTAMP(11, TimestampType.instance, 1), + UUID (12, UUIDType.instance, 1), + VARCHAR (13, UTF8Type.instance, 1), + VARINT (14, IntegerType.instance, 1), + TIMEUUID (15, TimeUUIDType.instance, 1), + INET (16, InetAddressType.instance, 1), + DATE (17, SimpleDateType.instance, 4), + TIME (18, TimeType.instance, 4), + LIST (32, null, 1), + MAP (33, null, 1), + SET (34, null, 1), + UDT (48, null, 3), + TUPLE (49, null, 3); public static final OptionCodec<DataType> codec = new OptionCodec<DataType>(DataType.class); private final int id; + private final int protocolVersion; private final AbstractType type; private static final Map<AbstractType, DataType> dataTypeMap = new HashMap<AbstractType, DataType>(); static @@ -72,14 +75,17 @@ public enum DataType implements OptionCodec.Codecable<DataType> } } - private DataType(int id, AbstractType type) + DataType(int id, AbstractType type, int protocolVersion) { this.id = id; this.type = type; + this.protocolVersion = protocolVersion; } - public int getId() + public int getId(int version) { + if (version < protocolVersion) + return DataType.CUSTOM.getId(version); return id; } @@ -123,6 +129,13 @@ public enum DataType implements OptionCodec.Codecable<DataType> public void writeValue(Object value, ByteBuf cb, int version) { + // Serialize as CUSTOM if client on the other side's version is < required for type + if (version < protocolVersion) + { + CBUtil.writeString(value.toString(), cb); + return; + } + switch (this) { case CUSTOM: @@ -162,6 +175,10 @@ public enum DataType implements OptionCodec.Codecable<DataType> public int serializedValueSize(Object value, int version) { + // Serialize as CUSTOM if client on the other side's version is < required for type + if (version < protocolVersion) + return CBUtil.sizeOfString(value.toString()); + switch (this) { case CUSTOM: @@ -230,16 +247,19 @@ public enum DataType implements OptionCodec.Codecable<DataType> throw new AssertionError(); } - if (type instanceof UserType && version >= 3) + if (type instanceof UserType && version >= UDT.protocolVersion) return Pair.<DataType, Object>create(UDT, type); - if (type instanceof TupleType && version >= 3) + if (type instanceof TupleType && version >= TUPLE.protocolVersion) return Pair.<DataType, Object>create(TUPLE, type); return Pair.<DataType, Object>create(CUSTOM, type.toString()); } else { + // Fall back to CUSTOM if target doesn't know this data type + if (version < dt.protocolVersion) + return Pair.<DataType, Object>create(CUSTOM, type.toString()); return Pair.create(dt, null); } } @@ -272,4 +292,10 @@ public enum DataType implements OptionCodec.Codecable<DataType> throw new ProtocolException(e.getMessage()); } } + + @VisibleForTesting + public int getProtocolVersion() + { + return protocolVersion; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a5c282f/src/java/org/apache/cassandra/transport/OptionCodec.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/transport/OptionCodec.java b/src/java/org/apache/cassandra/transport/OptionCodec.java index ec2a1fa..3a8b813 100644 --- a/src/java/org/apache/cassandra/transport/OptionCodec.java +++ b/src/java/org/apache/cassandra/transport/OptionCodec.java @@ -30,7 +30,7 @@ public class OptionCodec<T extends Enum<T> & OptionCodec.Codecable<T>> { public interface Codecable<T extends Enum<T>> { - public int getId(); + public int getId(int version); public Object readValue(ByteBuf cb, int version); public void writeValue(Object value, ByteBuf cb, int version); @@ -48,13 +48,13 @@ public class OptionCodec<T extends Enum<T> & OptionCodec.Codecable<T>> T[] values = klass.getEnumConstants(); int maxId = -1; for (T opt : values) - maxId = Math.max(maxId, opt.getId()); + maxId = Math.max(maxId, opt.getId(Server.CURRENT_VERSION)); ids = (T[])Array.newInstance(klass, maxId + 1); for (T opt : values) { - if (ids[opt.getId()] != null) - throw new IllegalStateException(String.format("Duplicate option id %d", opt.getId())); - ids[opt.getId()] = opt; + if (ids[opt.getId(Server.CURRENT_VERSION)] != null) + throw new IllegalStateException(String.format("Duplicate option id %d", opt.getId(Server.CURRENT_VERSION))); + ids[opt.getId(Server.CURRENT_VERSION)] = opt; } } @@ -91,7 +91,7 @@ public class OptionCodec<T extends Enum<T> & OptionCodec.Codecable<T>> for (Map.Entry<T, Object> entry : options.entrySet()) { T opt = entry.getKey(); - cb.writeShort(opt.getId()); + cb.writeShort(opt.getId(version)); opt.writeValue(entry.getValue(), cb, version); } return cb; @@ -108,7 +108,7 @@ public class OptionCodec<T extends Enum<T> & OptionCodec.Codecable<T>> { T opt = option.left; Object obj = option.right; - dest.writeShort(opt.getId()); + dest.writeShort(opt.getId(version)); opt.writeValue(obj, dest, version); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a5c282f/test/unit/org/apache/cassandra/transport/DataTypeTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/transport/DataTypeTest.java b/test/unit/org/apache/cassandra/transport/DataTypeTest.java new file mode 100644 index 0000000..dc2c4e2 --- /dev/null +++ b/test/unit/org/apache/cassandra/transport/DataTypeTest.java @@ -0,0 +1,108 @@ +/* + * 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.transport; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import org.junit.Test; + +import io.netty.buffer.ByteBuf; +import org.apache.cassandra.db.TypeSizes; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.AsciiType; +import org.apache.cassandra.db.marshal.LongType; + +import static org.junit.Assert.assertEquals; + +public class DataTypeTest +{ + @Test + public void TestSimpleDataTypeSerialization() + { + for (DataType type : DataType.values()) + { + if (isComplexType(type)) + continue; + + Map<DataType, Object> options = Collections.singletonMap(type, (Object)type.toString()); + for (int version = 1; version < 5; version++) + testEncodeDecode(type, options, version); + } + } + + @Test + public void TestListDataTypeSerialization() + { + DataType type = DataType.LIST; + Map<DataType, Object> options = Collections.singletonMap(type, (Object)LongType.instance); + for (int version = 1; version < 5; version++) + testEncodeDecode(type, options, version); + } + + @Test + public void TestMapDataTypeSerialization() + { + DataType type = DataType.MAP; + List<AbstractType> value = new ArrayList<>(); + value.add(LongType.instance); + value.add(AsciiType.instance); + Map<DataType, Object> options = Collections.singletonMap(type, (Object)value); + for (int version = 1; version < 5; version++) + testEncodeDecode(type, options, version); + } + + private void testEncodeDecode(DataType type, Map<DataType, Object> options, int version) + { + ByteBuf dest = type.codec.encode(options, version); + Map<DataType, Object> results = type.codec.decode(dest, version); + + for (DataType key : results.keySet()) + { + int ssize = type.serializedValueSize(results.get(key), version); + int esize = version < type.getProtocolVersion() ? 2 + TypeSizes.encodedUTF8Length(results.get(key).toString()) : 0; + switch (type) + { + case LIST: + case SET: + esize += 2; + break; + case MAP: + esize += 4; + break; + case CUSTOM: + esize = 8; + break; + } + assertEquals(esize, ssize); + + DataType expected = version < type.getProtocolVersion() + ? DataType.CUSTOM + : type; + assertEquals(expected, key); + } + } + + private boolean isComplexType(DataType type) + { + return type.getId(Server.CURRENT_VERSION) >= 32; + } +}
