Updated Branches: refs/heads/trunk 86c3afb6d -> c47f4070d
Validate that collection values are < 64K patch by slebresne; reviewed by jbellis for CASSANDRA-5355 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/9f268d16 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/9f268d16 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/9f268d16 Branch: refs/heads/trunk Commit: 9f268d1687a1c6ccd5138ad1cf88d82d23756984 Parents: 3f88b27 Author: Sylvain Lebresne <[email protected]> Authored: Thu Mar 21 13:57:01 2013 +0100 Committer: Sylvain Lebresne <[email protected]> Committed: Thu Mar 21 13:57:01 2013 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/cql3/Lists.java | 19 +++++++- src/java/org/apache/cassandra/cql3/Maps.java | 35 +++++++++++++-- src/java/org/apache/cassandra/cql3/Sets.java | 10 ++++- .../cassandra/db/marshal/CollectionType.java | 6 +++ .../org/apache/cassandra/db/marshal/ListType.java | 4 +- .../org/apache/cassandra/db/marshal/MapType.java | 6 +- .../org/apache/cassandra/db/marshal/SetType.java | 4 +- 8 files changed, 71 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 1d17e08..6827087 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -4,6 +4,7 @@ * Fix transposed arguments in AlreadyExistsException (CASSANDRA-5362) * Improve asynchronous hint delivery (CASSANDRA-5179) * Fix Guava dependency version (12.0 -> 13.0.1) for Maven (CASSANDRA-5364) + * Validate that provided CQL3 collection value are < 64K (CASSANDRA-5355) 1.2.3 http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/cql3/Lists.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Lists.java b/src/java/org/apache/cassandra/cql3/Lists.java index 76f947c..78db936 100644 --- a/src/java/org/apache/cassandra/cql3/Lists.java +++ b/src/java/org/apache/cassandra/cql3/Lists.java @@ -30,6 +30,7 @@ import org.apache.cassandra.db.marshal.ListType; import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Pair; import org.apache.cassandra.utils.UUIDGen; @@ -79,7 +80,14 @@ public abstract class Lists // We don't allow prepared marker in collections, nor nested collections assert t instanceof Constants.Value; - values.add(((Constants.Value)t).bytes); + ByteBuffer bytes = ((Constants.Value)t).bytes; + // We don't support value > 64K because the serialization format encode the length as an unsigned short. + if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("List value is too long. List values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + bytes.remaining())); + + values.add(bytes); } return new Value(values); } @@ -256,8 +264,15 @@ public abstract class Lists if (idx < 0 || idx >= existingList.size()) throw new InvalidRequestException(String.format("List index %d out of bound, list has size %d", idx, existingList.size())); + ByteBuffer bytes = ((Constants.Value)value).bytes; + // We don't support value > 64K because the serialization format encode the length as an unsigned short. + if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("List value is too long. List values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + bytes.remaining())); + ByteBuffer elementName = existingList.get(idx).right.name(); - cf.addColumn(params.makeColumn(elementName, ((Constants.Value)value).bytes)); + cf.addColumn(params.makeColumn(elementName, bytes)); } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/cql3/Maps.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Maps.java b/src/java/org/apache/cassandra/cql3/Maps.java index b49edc9..a2eb761 100644 --- a/src/java/org/apache/cassandra/cql3/Maps.java +++ b/src/java/org/apache/cassandra/cql3/Maps.java @@ -30,6 +30,7 @@ import org.apache.cassandra.db.marshal.CollectionType; import org.apache.cassandra.db.marshal.MapType; import org.apache.cassandra.db.marshal.MarshalException; import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Pair; /** @@ -78,7 +79,20 @@ public abstract class Maps throw new InvalidRequestException(String.format("Invalid map literal for %s: nested collections are not supported", receiver)); } - if (values.put(((Constants.Value)k).bytes, ((Constants.Value)v).bytes) != null) + // We don't support values > 64K because the serialization format encode the length as an unsigned short. + ByteBuffer keyBytes = ((Constants.Value)k).bytes; + if (keyBytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Map key is too long. Map keys are limited to %d bytes but %d bytes keys provided", + FBUtilities.MAX_UNSIGNED_SHORT, + keyBytes.remaining())); + + ByteBuffer valueBytes = ((Constants.Value)v).bytes; + if (valueBytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Map value is too long. Map values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + valueBytes.remaining())); + + if (values.put(keyBytes, valueBytes) != null) throw new InvalidRequestException(String.format("Invalid map literal: duplicate entry for key %s", entry.left)); } return new Value(values); @@ -225,9 +239,22 @@ public abstract class Maps assert value == null || value instanceof Constants.Value; ByteBuffer cellName = prefix.add(columnName.key).add(((Constants.Value)key).bytes).build(); - cf.addColumn(value == null - ? params.makeTombstone(cellName) - : params.makeColumn(cellName, ((Constants.Value)value).bytes)); + + if (value == null) + { + cf.addColumn(params.makeTombstone(cellName)); + } + else + { + ByteBuffer bytes = ((Constants.Value)value).bytes; + // We don't support value > 64K because the serialization format encode the length as an unsigned short. + if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Map value is too long. Map values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + bytes.remaining())); + + cf.addColumn(params.makeColumn(cellName, bytes)); + } } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/cql3/Sets.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Sets.java b/src/java/org/apache/cassandra/cql3/Sets.java index aa68714..fb57542 100644 --- a/src/java/org/apache/cassandra/cql3/Sets.java +++ b/src/java/org/apache/cassandra/cql3/Sets.java @@ -34,6 +34,7 @@ import org.apache.cassandra.db.marshal.MapType; import org.apache.cassandra.db.marshal.SetType; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.FBUtilities; /** * Static helper methods and classes for sets. @@ -79,7 +80,14 @@ public abstract class Sets throw new InvalidRequestException(String.format("Invalid set literal for %s: nested collections are not supported", receiver)); } - if (!values.add(((Constants.Value)t).bytes)) + ByteBuffer bytes = ((Constants.Value)t).bytes; + // We don't support value > 64K because the serialization format encode the length as an unsigned short. + if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Set value is too long. Set values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + bytes.remaining())); + + if (!values.add(bytes)) throw new InvalidRequestException(String.format("Invalid set literal: duplicate value %s", rt)); } return new Value(values); http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/db/marshal/CollectionType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/CollectionType.java b/src/java/org/apache/cassandra/db/marshal/CollectionType.java index d330e4c..ad2ea67 100644 --- a/src/java/org/apache/cassandra/db/marshal/CollectionType.java +++ b/src/java/org/apache/cassandra/db/marshal/CollectionType.java @@ -113,6 +113,12 @@ public abstract class CollectionType<T> extends AbstractType<T> return pack(buffers, elements, size); } + protected static int getUnsignedShort(ByteBuffer bb) + { + int length = (bb.get() & 0xFF) << 8; + return length | (bb.get() & 0xFF); + } + public CQL3Type asCQL3Type() { return new CQL3Type.Collection(this); http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/db/marshal/ListType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/ListType.java b/src/java/org/apache/cassandra/db/marshal/ListType.java index 589e29e..b6613ae 100644 --- a/src/java/org/apache/cassandra/db/marshal/ListType.java +++ b/src/java/org/apache/cassandra/db/marshal/ListType.java @@ -74,11 +74,11 @@ public class ListType<T> extends CollectionType<List<T>> try { ByteBuffer input = bytes.duplicate(); - int n = input.getShort(); + int n = getUnsignedShort(input); List<T> l = new ArrayList<T>(n); for (int i = 0; i < n; i++) { - int s = input.getShort(); + int s = getUnsignedShort(input); byte[] data = new byte[s]; input.get(data); ByteBuffer databb = ByteBuffer.wrap(data); http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/db/marshal/MapType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/MapType.java b/src/java/org/apache/cassandra/db/marshal/MapType.java index 8364ea0..19310df 100644 --- a/src/java/org/apache/cassandra/db/marshal/MapType.java +++ b/src/java/org/apache/cassandra/db/marshal/MapType.java @@ -77,17 +77,17 @@ public class MapType<K, V> extends CollectionType<Map<K, V>> try { ByteBuffer input = bytes.duplicate(); - int n = input.getShort(); + int n = getUnsignedShort(input); Map<K, V> m = new LinkedHashMap<K, V>(n); for (int i = 0; i < n; i++) { - int sk = input.getShort(); + int sk = getUnsignedShort(input); byte[] datak = new byte[sk]; input.get(datak); ByteBuffer kbb = ByteBuffer.wrap(datak); keys.validate(kbb); - int sv = input.getShort(); + int sv = getUnsignedShort(input); byte[] datav = new byte[sv]; input.get(datav); ByteBuffer vbb = ByteBuffer.wrap(datav); http://git-wip-us.apache.org/repos/asf/cassandra/blob/9f268d16/src/java/org/apache/cassandra/db/marshal/SetType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/SetType.java b/src/java/org/apache/cassandra/db/marshal/SetType.java index bbfb46f..73f25e0 100644 --- a/src/java/org/apache/cassandra/db/marshal/SetType.java +++ b/src/java/org/apache/cassandra/db/marshal/SetType.java @@ -74,11 +74,11 @@ public class SetType<T> extends CollectionType<Set<T>> try { ByteBuffer input = bytes.duplicate(); - int n = input.getShort(); + int n = getUnsignedShort(input); Set<T> l = new LinkedHashSet<T>(n); for (int i = 0; i < n; i++) { - int s = input.getShort(); + int s = getUnsignedShort(input); byte[] data = new byte[s]; input.get(data); ByteBuffer databb = ByteBuffer.wrap(data);
