Merge branch 'cassandra-3.0' into cassandra-3.11

CHANGES.txt

@@ -1,5 +1,6 @@
 3.11.14
 Merged from 3.0:
+ * Fix issue where frozen maps may not be serialized in the correct order (CASSANDRA-17623)
  * Suppress CVE-2022-24823 (CASSANDRA-17633)
  * fsync TOC and digest files (CASSANDRA-10709)
 

src/java/org/apache/cassandra/cql3/Maps.java

@@ -171,9 +171,9 @@ public String getText()
 
     public static class Value extends Term.Terminal
     {
-        public final Map<ByteBuffer, ByteBuffer> map;
+        public final SortedMap<ByteBuffer, ByteBuffer> map;
 
-        public Value(Map<ByteBuffer, ByteBuffer> map)
+        public Value(SortedMap<ByteBuffer, ByteBuffer> map)
         {
             this.map = map;
         }
@@ -185,7 +185,8 @@ public static Value fromSerialized(ByteBuffer value, MapType type, ProtocolVersi
                 // Collections have this small hack that validate cannot be called on a serialized object,
                 // but compose does the validation (so we're fine).
                 Map<?, ?> m = type.getSerializer().deserializeForNativeProtocol(value, version);
-                Map<ByteBuffer, ByteBuffer> map = new LinkedHashMap<>(m.size());
+                // We depend on Maps to be properly sorted by their keys, so use a sorted map implementation here.
+                SortedMap<ByteBuffer, ByteBuffer> map = new TreeMap<>(type.getKeysType());
                 for (Map.Entry<?, ?> entry : m.entrySet())
                     map.put(type.getKeysType().decompose(entry.getKey()), type.getValuesType().decompose(entry.getValue()));
                 return new Value(map);
@@ -251,7 +252,7 @@ public void collectMarkerSpecification(VariableSpecifications boundNames)
 
         public Terminal bind(QueryOptions options) throws InvalidRequestException
         {
-            Map<ByteBuffer, ByteBuffer> buffers = new TreeMap<ByteBuffer, ByteBuffer>(comparator);
+            SortedMap<ByteBuffer, ByteBuffer> buffers = new TreeMap<>(comparator);
             for (Map.Entry<Term, Term> entry : elements.entrySet())
             {
                 // We don't support values > 64K because the serialization format encode the length as an unsigned short.
@@ -466,7 +467,7 @@ static void doPut(Term.Terminal value, ColumnDefinition column, UpdateParameters
                 if (value == null)
                     return;
 
-                Map<ByteBuffer, ByteBuffer> elements = ((Value) value).map;
+                SortedMap<ByteBuffer, ByteBuffer> elements = ((Value) value).map;
                 for (Map.Entry<ByteBuffer, ByteBuffer> entry : elements.entrySet())
                     params.addCell(column, CellPath.create(entry.getKey()), entry.getValue());
             }

src/java/org/apache/cassandra/cql3/Sets.java

@@ -72,7 +72,7 @@ public Term prepare(String keyspace, ColumnSpecification receiver) throws Invali
             // We've parsed empty maps as a set literal to break the ambiguity so
             // handle that case now
             if (receiver.type instanceof MapType && elements.isEmpty())
-                return new Maps.Value(Collections.<ByteBuffer, ByteBuffer>emptyMap());
+                return new Maps.Value(Collections.emptySortedMap());
 
             ColumnSpecification valueSpec = Sets.valueSpecOf(receiver);
             Set<Term> values = new HashSet<>(elements.size());

test/unit/org/apache/cassandra/cql3/CQLTester.java

@@ -32,6 +32,8 @@
 
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
 import org.junit.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -801,6 +803,10 @@ protected CFMetaData currentTableMetadata()
         return Schema.instance.getCFMetaData(KEYSPACE, currentTable());
     }
 
+    protected com.datastax.driver.core.ResultSet executeNet(String query, Object... values) throws Throwable
+    {
+        return sessionNet().execute(formatQuery(query), values);
+    }
     protected com.datastax.driver.core.ResultSet executeNet(ProtocolVersion protocolVersion, String query, Object... values) throws Throwable
     {
         return sessionNet(protocolVersion).execute(formatQuery(query), values);
@@ -952,6 +958,13 @@ protected void assertRowsNet(ProtocolVersion protocolVersion, ResultSet result,
                                         rows.length>i ? "less" : "more", rows.length, i, protocolVersion), i == rows.length);
     }
 
+    protected void assertRowCountNet(ResultSet r1, int expectedCount)
+    {
+        Assert.assertFalse("Received a null resultset when expected count was > 0", expectedCount > 0 && r1 == null);
+        int actualRowCount = Iterables.size(r1);
+        Assert.assertEquals(String.format("expected %d rows but received %d", expectedCount, actualRowCount), expectedCount, actualRowCount);
+    }
+
     public static void assertRows(UntypedResultSet result, Object[]... rows)
     {
         if (result == null)
@@ -1599,13 +1612,27 @@ protected Object set(Object...values)
         return ImmutableSet.copyOf(values);
     }
 
+    // LinkedHashSets are iterable in insertion order, which is important for some tests
+    protected LinkedHashSet<Object> linkedHashSet(Object...values)
+    {
+        LinkedHashSet<Object> s = new LinkedHashSet<>(values.length);
+        s.addAll(Arrays.asList(values));
+        return s;
+    }
+
     protected Object map(Object...values)
+    {
+        return linkedHashMap(values);
+    }
+
+    // LinkedHashMaps are iterable in insertion order, which is important for some tests
+    protected static LinkedHashMap<Object, Object> linkedHashMap(Object...values)
     {
         if (values.length % 2 != 0)
             throw new IllegalArgumentException();
 
         int size = values.length / 2;
-        Map m = new LinkedHashMap(size);
+        LinkedHashMap<Object, Object> m = new LinkedHashMap<>(size);
         for (int i = 0; i < size; i++)
             m.put(values[2 * i], values[(2 * i) + 1]);
         return m;

test/unit/org/apache/cassandra/cql3/ColumnConditionTest.java

@@ -450,7 +450,7 @@ public void testMapCollectionBoundIsSatisfiedByValue() throws InvalidRequestExce
     {
         ColumnDefinition definition = ColumnDefinition.regularDef("ks", "cf", "c", ListType.getInstance(Int32Type.instance, true));
 
-        Map<ByteBuffer, ByteBuffer> placeholderMap = new TreeMap<>();
+        SortedMap<ByteBuffer, ByteBuffer> placeholderMap = new TreeMap<>();
         placeholderMap.put(ONE, ONE);
         Maps.Value placeholder = new Maps.Value(placeholderMap);
 

test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java

@@ -21,6 +21,7 @@
 
 import org.junit.Test;
 
+import com.datastax.driver.core.utils.UUIDs;
 import org.apache.cassandra.cql3.CQLTester;
 import org.apache.cassandra.utils.FBUtilities;
 
@@ -1071,4 +1072,77 @@ public void testInsertingCollectionsWithInvalidElements() throws Throwable
         assertInvalidMessage("Invalid map literal for m: value (1, '1', 1.0, 1) is not of type frozen<tuple<int, text, double>>",
                              "INSERT INTO %s (k, m) VALUES (0, {1 : (1, '1', 1.0, 1)})");
     }
+
+    /*
+     Tests for CASSANDRA-17623
+     Before CASSANDRA-17623, parameterized queries with maps as values would fail because frozen maps were
+     required to be sorted by the sort order of their key type, but weren't always sorted correctly.
+     Also adding tests for Sets, which did work because they always used SortedSet, to make sure this behavior is maintained.
+     We use `executeNet` in these tests because `execute` passes parameters through CqlTester#transformValues(), which calls
+     AbstractType#decompose() on the value, which "fixes" the map order, but wouldn't happen normally.
+     */
+
+    @Test
+    public void testInsertingMapDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
+    {
+        UUID uuid1 = UUIDs.timeBased();
+        UUID uuid2 = UUIDs.timeBased();
+        createTable("CREATE TABLE %s (k text, c frozen<map<timeuuid, text>>, PRIMARY KEY (k, c));");
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashMap(uuid1, "0", uuid2, "1"));
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashMap(uuid2, "3", uuid1, "4"));
+        beforeAndAfterFlush(() -> {
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ": '0', " + uuid2 + ": '1'}"), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ": '1', " + uuid1 + ": '0'}"), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ": '4', " + uuid2 + ": '3'}"), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ": '3', " + uuid1 + ": '4'}"), 1);
+        });
+    }
+
+
+    @Test
+    public void testSelectingMapDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
+    {
+        UUID uuid1 = UUIDs.timeBased();
+        UUID uuid2 = UUIDs.timeBased();
+        createTable("CREATE TABLE %s (k text, c frozen<map<timeuuid, text>>, PRIMARY KEY (k, c));");
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', {" + uuid1 + ": '0', " + uuid2 + ": '1'})");
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', {" + uuid2 + ": '3', " + uuid1 + ": '4'})");
+        beforeAndAfterFlush(() -> {
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid1, "0", uuid2, "1")), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid2, "1", uuid1, "0")), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid1, "4", uuid2, "3")), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid2, "3", uuid1, "4")), 1);
+        });
+    }
+
+    @Test
+    public void testInsertingSetDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
+    {
+        UUID uuid1 = UUIDs.timeBased();
+        UUID uuid2 = UUIDs.timeBased();
+        createTable("CREATE TABLE %s (k text, c frozen<set<timeuuid>>, PRIMARY KEY (k, c));");
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashSet(uuid1, uuid2));
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashSet(uuid2, uuid1));
+        beforeAndAfterFlush(() -> {
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ", " + uuid2 + '}'), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ", " + uuid1 + '}'), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ", " + uuid2 + '}'), 1);
+            assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ", " + uuid1 + '}'), 1);
+        });
+    }
+
+
+    @Test
+    public void testSelectingSetDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
+    {
+        UUID uuid1 = UUIDs.timeBased();
+        UUID uuid2 = UUIDs.timeBased();
+        createTable("CREATE TABLE %s (k text, c frozen<set<timeuuid>>, PRIMARY KEY (k, c));");
+        executeNet("INSERT INTO %s (k, c) VALUES ('0', {" + uuid1 + ", " + uuid2 + "})");
+        beforeAndAfterFlush(() -> {
+            assertRowsNet(executeNet("SELECT k, c from %s where k='0' and c=?", linkedHashSet(uuid1, uuid2)), row("0", list(uuid1, uuid2)));
+            assertRowsNet(executeNet("SELECT k, c from %s where k='0' and c=?", linkedHashSet(uuid2, uuid1)), row("0", list(uuid1, uuid2)));
+        });
+    }
+    // End tests for CASSANDRA-17623
 }